Apply fix for jenkins tests #4267
Conversation
|
Hehe, apologies for making you have to go through that RPN code 😅 although you seem to have come out on top pretty quickly, well done 👍 |
|
success!
Haha, yeh its not too bad once you get your head round it. You should have presented it at the tutorial last week, that would have really blown their minds lol |
|
Just for reference, as said in the issue, the documentation is using the |
Well I guess its generally best practice anyway, to add outputs where possible before returning exit codes: so that it is easier to debug |
Codecov Report
@@ Coverage Diff @@
## develop #4267 +/- ##
===========================================
- Coverage 79.18% 79.18% -0.00%
===========================================
Files 468 468
Lines 34476 34476
===========================================
- Hits 27297 27295 -2
- Misses 7179 7181 +2
Continue to review full report at Codecov.
|
…ue (#4267) For the recent documentation revamp, the `ArithmeticAddCalculation` and `ArithmeticAddParser` were simplified significantly, by getting rid off as much as the unnecessary code, because they are being literally included as example for the basic how-to create a code plugin. A part that was removed was the `settings` input node that allowed to change the behavior of the parser and allow negative sums instead of it returning an exit code. This, however, in turn cause the Reverse Polish Notation tests on Jenkins to fail. Since these tests are not required to pass, the changes were merged without a fix. In the scope of the RPN tests, negative sums are fine, which anyway is just a mechanism to introduce some kind of failure mode for demonstration purposes. To fix this, without making the logic of the parser more complex, we simply change the order of attaching the output node and performing the final check. Since the RPN workchains only check if the output node is there and do not care about the exit status of the calculation, they will happily continue and the code of the parser keeps the same complexity.
this did the job locally thanks @sphuber, so hopefully will also parse on Jenkins 🤞
fixes #4264