Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct unit tests in hyper-optimised-telemetry exercise #1796

Merged
merged 2 commits into from
Nov 3, 2021
Merged

fix: correct unit tests in hyper-optimised-telemetry exercise #1796

merged 2 commits into from
Nov 3, 2021

Conversation

kaelonR
Copy link
Contributor

@kaelonR kaelonR commented Oct 29, 2021

This PR fixes two unit tests on the hyper-optimised-telemetry exercise that wrongly assume 0 and Int16.MaxValue (32767) should be encoded into signed shorts, rather than unsigned shorts as specified in the exercise instructions.

image

Directly targets issue #1795 and #1621 (The latter having been closed, though it reports the same problem)

This PR is incompatible with PR #1756, as the author of that PR aims to change the instructions to be in line with how the unit tests have been set up, while this PR aims to do the exact inverse by changing the unit tests to match the instructions instead.

@ErikSchierboom
Copy link
Member

@j141 Could you also update the exemplar implementation (.meta/Exemplar.cs)? That should fix CI.

@kaelonR
Copy link
Contributor Author

kaelonR commented Nov 2, 2021

@ErikSchierboom Whoops, i somehow missed the existence of that Exemplar.cs. I've updated it to match the changes to the unit tests now.

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes much more sense now!

@ErikSchierboom ErikSchierboom merged commit 6df7023 into exercism:main Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants