Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom signs - throws generic error when running tests on initial code #119

Open
BaerbelW opened this issue Apr 2, 2022 · 23 comments
Open
Assignees

Comments

@BaerbelW
Copy link

BaerbelW commented Apr 2, 2022

When running tests on "Custom signs" before adding code, I get this error:

"An error occurred while running your tests. This might mean that there was an issue in our infrastructure, or it might mean that you have something in your code that's causing our systems to break.

Please check your code, and if nothing seems to be wrong, try running the tests again."

When this happened for other exercises yesterday, it usually worked with a 2nd (or 3rd) try but no such luck right now.

Shouldn't running the tests initially always provide results - i.e. not throw technical errors - even if all fail?

@larshp
Copy link
Member

larshp commented Apr 2, 2022

yea, can you paste the code here? then we can reproduce the error

@BaerbelW
Copy link
Author

BaerbelW commented Apr 2, 2022

Here is the code:

CLASS zcl_custom_signs DEFINITION
  PUBLIC
  FINAL
  CREATE PUBLIC.

  PUBLIC SECTION.

    "! Build a sign that includes both of the parameters.
    METHODS build_sign IMPORTING occasion      TYPE string
                                 name          TYPE string
                       RETURNING VALUE(result) TYPE string.

    "! Build a birthday sign that conditionally formats the return string.
    METHODS build_birthday_sign IMPORTING age           TYPE i
                                RETURNING VALUE(result) TYPE string.

    "! Build a graduation sign that includes multiple lines
    METHODS graduation_for IMPORTING name          TYPE string
                                     year          TYPE i
                           RETURNING VALUE(result) TYPE string.

    "! Determine cost based on each character of sign parameter that builds
    "! the template string that includes the currency parameter.
    METHODS cost_of IMPORTING sign          TYPE string
                              currency      TYPE string
                    RETURNING VALUE(result) TYPE string.

ENDCLASS.



CLASS zcl_custom_signs IMPLEMENTATION.


  METHOD build_sign.
    "Implement solution here
  ENDMETHOD.


  METHOD build_birthday_sign.
    "Implement solution here
  ENDMETHOD.


  METHOD graduation_for.
    "Implement solution here
  ENDMETHOD.


  METHOD cost_of.
    "Implement solution here
  ENDMETHOD.


ENDCLASS.

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 2, 2022

Hmm, my solution still tests OK on the site, so the issue is not with the tests. If I introduce a syntax error then I get a message

We received the following error when we ran your code:
  ● Test suite failed to run
    ReferenceError: Foo is not defined

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 2, 2022

D'Oh! Ignore last comment, I was experimenting on my JavaScript solution. Ich habe Languages mixed up.
You are right, the error happens on the template code.

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 2, 2022

@larshp I think there is an issue with the test runner or with the way one of the unit tests fail. I got the same error with the template but once I pasted the solution it runs in exercism-test and on the web.

@BaerbelW Can you go ahead and implement the solution anyway? Once the tests pass you should get meaningful feedback.

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 2, 2022

Update: I did a process by elimination by deleting the code inside my methods one by one. As expected, the unit tests started to fail, but when I deleted the graduation_for code, the test failure crashed the test run. So @BaerbelW if you implement that one you should get meaningful errors back again 🙂

@larshp can you please take a look?

If I do the same experiment of alternately deleting the code for build_birthday_sign and graduation_for, I get the same output running in exercism-test.

Wonder if it could have something to do with a \n linefeed in the output? Here's the test class:

CLASS ltc_graduation DEFINITION FINAL FOR TESTING
  DURATION SHORT
  RISK LEVEL HARMLESS.

  PRIVATE SECTION.

    DATA cut TYPE REF TO zcl_custom_signs.

    METHODS setup.

    METHODS rob_2021 FOR TESTING RAISING cx_static_check.
    METHODS jill_1999 FOR TESTING RAISING cx_static_check.

ENDCLASS.

CLASS ltc_graduation IMPLEMENTATION.


  METHOD setup.
    cut = NEW zcl_custom_signs( ).
  ENDMETHOD.


  METHOD rob_2021.
    cl_abap_unit_assert=>assert_equals(
        act = cut->graduation_for(
          name = 'Rob'
          year = '2021' )
        exp = |Congratulations Rob!\nClass of 2021| ).
  ENDMETHOD.


  METHOD jill_1999.
    cl_abap_unit_assert=>assert_equals(
      act = cut->graduation_for(
        name = 'Jill'
        year = '1999' )
      exp = |Congratulations Jill!\nClass of 1999| ).
  ENDMETHOD.

ENDCLASS.

If it's that and it's a messy fix, we can also update the exercise to something different.

@BaerbelW
Copy link
Author

BaerbelW commented Apr 2, 2022

@pokrakam - I'm trying to implement it but don't really know how to properly tackle the line-break. When I try with a simple CONCATENATE or with a for me new String expression with the "|", I still get the above error as I don't really know how the syntax needs to look like (even after checking ABAPDoku). So, I'm a bit mystified how to actually pass that test case!

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 3, 2022

@BaerbelW you are one link away from the answer in your documentation link, look under Control Characters 🙂

@BaerbelW
Copy link
Author

BaerbelW commented Apr 3, 2022

Sorry, Mike, but I don't get what I have to provide to make that particular testcase not end in a technical error. Reading the documentation doesn't make it any clearer for me, i.e. I cannot make heads or tails of it. I also don't think that this will be a helpful example for newbies to learn ABAP and would be in favor to just ditch the control character example & testcase from the exercise.

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 3, 2022

Hmm, will have a think on it.
Newline = \n, as explained in https://help.sap.com/doc/abapdocu_752_index_htm/7.52/en-US/abenstring_templates_separators.htm so you can just put that right in the middle - the answer is actually in the test case :)

As to newbies, the majority of Exercism users are folks who are already proficient in one language and want to learn another. The \n is quite common, used in Linux and part of regex and in many languages, so that's why it's part of the string-oriented exercise. So it's useful to know that what someone may be used to works the same in ABAP.

There is also a hints feature which we haven't used yet, maybe I'll try implementing it.

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 3, 2022

...and I'm pleased to say that the way we used before string templates also works:

    result = `Congratulations ` && name && `!` && cl_abap_char_utilities=>newline &&
             `Class of ` && year.

Or a mixture:

    result = |Congratulations { name }!| && cl_abap_char_utilities=>newline &&
             |Class of { year }|.

@BaerbelW
Copy link
Author

BaerbelW commented Apr 3, 2022

Yeah, I thought I could "just" put it within a concatenation but neither option I had tried did away with the error message! I was fairly certain that it must have been a stupid mistake on my end but didn't see it until I checked your example above. The error wasn't caused by anything to do with "\n" but that I had forgotten to add the "!" in the first part of the concatenation.

I may have overlooked it while briefly poking around Exercism for ABAP but is there a mention somewhere that classes & methods like cl_abap_char_utilities=>newline are available on the platform? I wouldn't have expected that.

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 3, 2022

As a rough guide, the implementation tries to cover what's in the ABAP docs, including some of the documented system classes if they are also part of the ABAP cloud syntax. No RTTI stuff though.
https://help.sap.com/doc/abapdocu_752_index_htm/7.52/en-US/abencl_abap_char_utilities.htm?file=abencl_abap_char_utilities.htm

@larshp is there a list of available system classes?

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 3, 2022

Hmm, I take that back, cl_abap_char_utilities is not available on the web version.

@larshp it works if I run it in exercism-test CLI, but throws
./zcl_custom_signs.clas.abap[52, 5] - Unknown class cl_abap_char_utilities (check_syntax) [E] abaplint: 1 issue(s) found
on the web version.

@BaerbelW re.

I had forgotten to add the "!" in the first part of the concatenation.
Yes this particular test is a special case, you could normally see this type thing as part of the test feedback. e.g. the same change in the Birthday task gives a meaningful feedback:
Expected 'Happy Birthday! What a young fellow you are.', got 'Happy Birthday What a young fellow you are.'

And always feel free to click on the unit test tab to see what's being tested. Personally I do these this with a Test Driven Development approach and code against each test. So I have the test open to the side and write code for each test until it passes then move onto the next. It's a good way to practice TDD, something like Minesweeper is a nice example (though that may be best done on a SAP/Trial system) as the tests start with simple functions and get progressively more intensive.

@larshp
Copy link
Member

larshp commented Apr 4, 2022

cl_abap_char_utilities added in exercism/abap-test-runner#74

@larshp
Copy link
Member

larshp commented Apr 5, 2022

so, whats the status for this issue?

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 5, 2022

Nice, works with
result = |Congratulations { name }!| && cl_abap_char_utilities=>newline && |Class of { year }|.
Thanks.

@pokrakam pokrakam closed this as completed Apr 5, 2022
@pokrakam
Copy link
Collaborator

pokrakam commented Apr 5, 2022

Sorry I closed this, thought I had raised it.

@BaerbelW is this resolved or are there other problems with the exercise? I am also trying to add some hints but that's proving a bit problematic - #123

Edit: Hints work, you should see a bar saying the exercise has been updated. Follow the link and click on update. Then you should see a hint icon appear in the top right:
image

This is the only exercise with hints so far, let me know what you think

@pokrakam pokrakam reopened this Apr 5, 2022
@pokrakam pokrakam assigned BaerbelW and unassigned pokrakam Apr 5, 2022
@BaerbelW
Copy link
Author

BaerbelW commented Apr 5, 2022

@pokrakam
Hi Mike - thanks for adding the tips which come up nicely when I click the light bulb. I'm sure that these will be quite helpful for others but me.

The initial issue with the not very helpful error message still ocurs though, when I delete the "!" to make that particular test with the line break fail. So that's not yet resolved as far as I can tell. Something for @larshp to look at?

Cheers
Bärbel

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 5, 2022

You are right, the \n (suspected) still breaks the test runner. @larshp could you please take a look?

Change the text to make build_birthday_sign fail and we get a meaningful
Expected 'Happy Birthday! What a young fellow you are.', got 'Happy Birthday What a young fellow you are.'

But do the same with graduation_for and we get
An error occurred while running your tests. This might mean that there was an issue in our infrastructure, or it might mean that you have something in your code that's causing our systems to break.

Same message even if I remove the newline from the code, so I'm guessing the newline in the test might be causing an issue?

METHOD rob_2021.
  cl_abap_unit_assert=>assert_equals(
      act = cut->graduation_for(
        name = 'Rob'
        year = '2021' )
      exp = |Congratulations Rob!\nClass of 2021| ).
ENDMETHOD.

@larshp
Copy link
Member

larshp commented Apr 11, 2022

@pokrakam can you reiterate what the problem is?

@pokrakam
Copy link
Collaborator

pokrakam commented Apr 11, 2022

@larshp if a unit test expects a newline in text and fails, it seems to crash the test. Try out Custom Signs with graduation_for.

@larshp
Copy link
Member

larshp commented Oct 31, 2022

@pokrakam / @BaerbelW try again, I think this issue was fixed with open-abap/open-abap-core#459, also see #192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants