In Allergies, test results show “Expected” and “Actual” switched. Having made the same mistake recently on the Ruby track, I figured the order of arguments in the assertion functions is the wrong way around; and indeed, Microsoft documentation has “expected” first, so that’s it. Shall I make a PR to fix it? (I’m still not sure which tracks accept community contributions.)
1 Like
A change like this should probably be done by a maintainer and applied across all exercises. That track doesn’t have a maintainer at the moment.
How confident are you in your ability to audit all the tests, change all the places that need changing, and verifying that your changes work correctly, without any maintainer to help you along?
For this specific change, I reckon the pipeline should be enough, as long as it checks that the example solution passes. I mean, the only thing I can imagine going wrong here is misplacing a parenthesis or a comma while copy-pasting, in which case we would have a syntax error, and the test wouldn’t pass.
If someone better-suited wants to do it, that’s obviously fine by me. It just looks like a simple thing.
The VB track is currently unmaintained so a cross-track reviewer would be the one reviewing any incoming PRs. I think we should just fix Allergies for now so it’s easier for review. Someone later can make a sweep through the repo now that it’s known to be a possible issue.
2 Likes
Add missing `allergies` tests by BNAndras · Pull Request #405 · exercism/vbnet · GitHub should take care of this issue once merged. I switched the expected/actual order for all assertions and then used Assert.True(actual)/Assert.False(actual) for the allergicTo tests to make it even clearer for students.
Accumulate and Binary also have the expected and actual values switched, but I’m planning on deprecating both exercises first before fixing that. Then they’re not available to students who haven’t already started the exercise.
Edit - Binary is already deprecated, but I’ll still fix the order of arguments for the asserts.