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

bit checking order is reversed in tests #578

Closed
wants to merge 1 commit into from
Closed

bit checking order is reversed in tests #578

wants to merge 1 commit into from

Conversation

BaseCase
Copy link

I believe the tests for this exercise are written expecting we check the bits from right to left, not left to right as indicated here. testShuffle1 and testShuffle3 both pass regardless of which direction we go, but testShuffle2 fails when going left to right.

If starting with leftmost is the correct behavior, then I believe testShuffle2 should expect to get ("Purple", "Marigold", "Cyan") instead of the ("Marigold", "Cyan", "Purple") it currently wants (I could do a different PR with that change instead of this if that's the right thing).

I might also just be confused and it's correct as is!!

I believe the tests for this exercise are written expecting we check the bits from right to left, not left to right as indicated here. `testShuffle1` and `testShuffle3` both pass regardless of which direction we go, but `testShuffle2` fails when going left to right.

If starting with leftmost is the correct behavior, then I believe `testShuffle2` should expect to get `("Purple", "Marigold", "Cyan")` instead of the `("Marigold", "Cyan", "Purple")` it currently wants (I could do a different PR with that change instead of this if that's the right thing).

I might also just be confused and it's correct as is!!
@ErikSchierboom
Copy link
Member

Pinging @exercism/reviewers

@kotp kotp requested a review from a team October 14, 2022 17:30
@BaseCase
Copy link
Author

Ah! My mistake, I failed to check existing PRs before opening this one. Apologies for the noise. @paiv, would you like me to close this?

@kotp
Copy link
Member

kotp commented Oct 17, 2022

This is an identical proposal as #497 and so I am closing as a duplicate.
A discussion is availble in issue #496.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants