Fix Ledger test so they pass regardless of system locale

[manumafe98]

pull request

Related issue: #2635

Reviewer Resources:

Track Policies

[manumafe98]

The only issue I could see with this fix is that the students think that part of the code needs to be reformatted, and then do not know how to go back to the previous working state.

Also maybe this need to be another discussion but a lot of students (checking on community solutions) send the exercise raw without changes, maybe this reformat style are not that exciting? or maybe we could make a check so at least they need to make a change to the original code?

Apart from that ledger has the same implementation in reference as well in the code given to the students, which I guess is not the idea, comparing to the Markdown reformat exercise.

[sanderploegsma]

The only issue I could see with this fix is that the students think that part of the code needs to be reformatted, and then do not know how to go back to the previous working state.

Yes, ideally you should come up with a solution that works regardless of how students change the implementation. One way I could think of is to temporarily change the JVM locale to en-US from the test suite, perhaps using an @Rule like the one mentioned here: https://stackoverflow.com/a/76430798

Also maybe this need to be another discussion but a lot of students (checking on community solutions) send the exercise raw without changes, maybe this reformat style are not that exciting? or maybe we could make a check so at least they need to make a change to the original code?

Apart from that ledger has the same implementation in reference as well in the code given to the students, which I guess is not the idea, comparing to the Markdown reformat exercise.

Yes I think this exercise could be improved a bit. If you have any ideas feel free to drop them in an issue or on the forum, perhaps there are others who want to help out 👍

[manumafe98]

The only issue I could see with this fix is that the students think that part of the code needs to be reformatted, and then do not know how to go back to the previous working state.

Yes, ideally you should come up with a solution that works regardless of how students change the implementation. One way I could think of is to temporarily change the JVM locale to en-US from the test suite, perhaps using an @Rule like the one mentioned here: https://stackoverflow.com/a/76430798

Also maybe this need to be another discussion but a lot of students (checking on community solutions) send the exercise raw without changes, maybe this reformat style are not that exciting? or maybe we could make a check so at least they need to make a change to the original code?
Apart from that ledger has the same implementation in reference as well in the code given to the students, which I guess is not the idea, comparing to the Markdown reformat exercise.

Yes I think this exercise could be improved a bit. If you have any ideas feel free to drop them in an issue or on the forum, perhaps there are others who want to help out 👍

I will try to implement that Rule, so in the meantime works as expected until we found how we can improve this kind of exercises!

[manumafe98]

@sanderploegsma I have a couple o questions, I rebased main into this branch and now seems to have a ton of changes, i should open a new one? Also how do you download the logs of the pipeline?

Cause i've done that from the image above, but I do not find them helpful.

Also the annotation locally seems to work ok, the tests now work correctly.

[sanderploegsma]

@sanderploegsma I have a couple o questions, I rebased main into this branch and now seems to have a ton of changes, i should open a new one?

Should be fixed now, I clicked the Update Branch button on GitHub.

Also how do you download the logs of the pipeline?

The logging output from the test runner isn't helpful indeed. However, when any tests fail it should also store an artifact that you can download. It contains the JSON output of the test runner for each exercise. That should contain information about what is going on.

[manumafe98]

The logging output from the test runner isn't helpful indeed. However, when any tests fail it should also store an artifact that you can download. It contains the JSON output of the test runner for each exercise. That should contain information about what is going on.

Oh! great I found it!

"message" : "./src/test/java/LedgerTest.java:4: error: package org.junitpioneer.jupiter does not exist\nimport org.junitpioneer.jupiter.DefaultLocale;

Okey so seems like, the addition of the dependency to the build.gradle file is not sufficient, should be added somewhere else?

Thanks for your help!

[sanderploegsma]

The test runner is a Docker container that runs without access to the internet for security purposes. This means that you can't add dependencies to an exercise without bundling them in the test runner.

In practice we only do this for dependencies that are really required for the exercise to work (like a JSON library in the rest-api exercise).

In this case, there is an alternative way to achieve the same without installing extra dependencies by using JUnit's extension model. For example, these extension points might work: https://junit.org/junit5/docs/current/user-guide/#extensions-lifecycle-callbacks-before-after-execution. It could work similar to the @Rule approach for JUnit 4.

[manumafe98]

The test runner is a Docker container that runs without access to the internet for security purposes. This means that you can't add dependencies to an exercise without bundling them in the test runner.

In practice we only do this for dependencies that are really required for the exercise to work (like a JSON library in the rest-api exercise).

Good to know! thanks!

In this case, there is an alternative way to achieve the same without installing extra dependencies by using JUnit's extension model. For example, these extension points might work: https://junit.org/junit5/docs/current/user-guide/#extensions-lifecycle-callbacks-before-after-execution. It could work similar to the @Rule approach for JUnit 4.

Sure, I will give it a try then, let me read that documentation and see if I can figure it out how to do it!

[manumafe98]

@sanderploegsma Done! now works locally and in the pipeline!

[sanderploegsma]

Great job! This is exactly what I had in mind. Two more things to address and then we're good to go.

[sanderploegsma]

Awesome, thanks for this!