problem-specifications
problem-specifications copied to clipboard
exercism
resistor-color-trio: Add missing test case from description
This test case is already given as example in the description, but not covered by tests. I'd argue it's a bug fix and not covered by #1560.
I'd argue that it is not a bug and that this PR is blocked by #1560. As stated by @iHiD in the body of that Issue a test would have to be wrong, not merely missing, to be considered a bug:
We will still accept bugfixes (if tests are wrong) and improvements to the copy around exercises, as long as they do not alter the substance of an exercise
Emphasis added.
Though it would take some effort to survey them all, I believe there are a number of description.md files mention examples that do not literally exist in the tests. Certainly there are many descriptions that make assumptions / implications not actually addressed in tests. If this addition isn't actually testing anything new and otherwise uncovered in the tests -- which it isn't obviously doing, as all three colors appear in the same orientation in other tests -- then I'd argue it's not actually necessary, on top of not really being a bug fix.
@miguelraz thanks for the making the change I requested. I have to agree with @yawpitch though. I recommend following the suggestion outlined in #1560 that suggests that changes be added at the local (track) level. This test case could be added to the track where you came across this issue in the canonical data. @exercism/julia was it?
Personally I think the suggested change is a good one for the canonical data. I am going to close this PR for now.
This PR should be reconsidered whenever the restrictions of #1560 are reversed.
@SaschaMann Nice idea. I believe y'all can also now convert PRs to drafts (I've just done that here)
It needs to be rebased, I think. The version can be dropped and the test case itself needs a UUID now.
Oh, lol, should we get into locale specific decimal separators too? :)
On Mon, 14 Mar 2022, 16:29 Angelika Tyborska, @.***> wrote:
@.**** requested changes on this pull request.
I'm not sure if 3300 ohms is the correct return value for this case. The description says:
When we get more than a thousand ohms, we say "kiloohms".
3300 is more than a thousand, so my interpretation of the description would be that the expected value is 3,3 kiloohms.
— Reply to this email directly, view it on GitHub https://github.com/exercism/problem-specifications/pull/1652#pullrequestreview-909073965, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNZA6NZ4JUWAD4DXPK3QXTU75SODANCNFSM4MTC5KEQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you are on a team that was mentioned.Message ID: @.*** com>
@cmcaine Huh? I'm not sure where that suggestion comes from. The return values are described as objects with value and unit keys, not strings, so it's just about using a float for the value. AFAIK you don't need to deal with locale specific separators to use a float. Unless we're coding in Excel 😁
The return values are described as objects with value and unit keys, not strings, so it's just about using a float for the value.
Some implementations of this exercise use strings as expected value, e.g. C#'s. Doesn't change your argument because the language would deal with it, but it's worth keeping that in mind for these tests.
https://neurophysics.ucsd.edu/courses/physics_120/resistorcharts.pdf switches at exactly 1000 ohms. I find that compelling enough to fix the description to 3.3 k
