exercism / abap

Exercism exercises in ABAP.
https://exercism.org/tracks/abap
MIT License
71 stars 37 forks source link

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

Open BaerbelW opened 2 years ago

BaerbelW commented 2 years ago

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 commented 2 years ago

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

BaerbelW commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

BaerbelW commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

...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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

cl_abap_char_utilities added in https://github.com/exercism/abap-test-runner/pull/74

larshp commented 2 years ago

so, whats the status for this issue?

pokrakam commented 2 years ago

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

pokrakam commented 2 years ago

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

BaerbelW commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@pokrakam can you reiterate what the problem is?

pokrakam commented 2 years ago

@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 commented 1 year ago

@pokrakam / @BaerbelW try again, I think this issue was fixed with https://github.com/open-abap/open-abap/pull/459, also see https://github.com/exercism/abap/issues/192