Update DnDCharacterTest.java

[vonabirgea]

Just have added all the other abilities to be checked on being calculated just once (lines 178 - 182). Before it was only strength to be checked.

pull request

---

Reviewer Resources:

Track Policies

[sanderploegsma]

Thanks! I think this is a good addition to the test cases.

One thing that I would like to address is that the tests in this exercise are based on canonical data that is shared across Exercism tracks. Here the test case explicitly mentions to check only the strength property, so we would be deviating from the canonical data.

In this case I think the deviation is warranted, and we might even consider opening an issue in the problem specifications to address this in the canonical data itself. Until then though, I think we should mention somewhere that our test deviates from the canonical data and why.

Each practice exercise in this repository that has canonical data has a tests.toml file in the exercise's .meta directory that keeps track of which tests have been implemented. I suggest adding a comment in there to explain the deviation, like so:

  [2ca77b9b-c099-46c3-a02c-0d0f68ffa0fe]
  description = "each ability is only calculated once"
+ comment = "The canonical data only verify the strength ability, we chose to verify every ability instead."

@vonabirgea would you be willing to commit and push this change as well?

[vonabirgea]

I think I've just done what you proposed, @sanderploegsma. I'm totally new to contributing, pull-requesting and all that stuff, so if I did something wrong - please let me know, I will try my best to make it right then. Thanks for your help!

[sanderploegsma]

@vonabirgea no problem, Exercism is all about learning so you're in the right place 😉

I see that you made your change in another Pull Request instead of this one. Probably you forgot to commit the changes for the tests.toml file to the brach containing the changes for this PR (it's called patch-1 and exists on your fork of this repository), and committed them to another branch instead (the main branch on your fork).

You could try to push the commit to the patch-1 instead, so that both changes end up in this same PR. If you're not sure how to do that it's fine, we can merge both PRs individually as they do not conflict with each other so it shouldn't be a problem.

[sanderploegsma]

@vonabirgea Since the problem-specifications were updated to reflect this change in the canonical data, I took the liberty of syncing the tests.toml on your branch. This also means that #2550 is no longer required.

[sanderploegsma]

Thanks for your contribution! 👍