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

Hamming: remove tests that don't make sense per 1761 #1762

Merged
merged 2 commits into from
Jan 2, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions exercises/hamming/canonical-data.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,50 @@
"strand2": "AGTG"
},
"expected": {"error": "left and right strands must be of equal length"}
},
{
"uuid": "5dce058b-28d4-4ca7-aa64-adfe4e17784c",
"description": "disallow left empty strand",
"property": "distance",
"input": {
"strand1": "",
"strand2": "G"
},
"expected": {"error": "left strand must not be empty"}
Copy link
Member

Choose a reason for hiding this comment

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

Why can't it be empty? As long as the right strand is also empty, representing that it is not there, the distance could still be said to be 0. And if there is a length of not 0 for the right hand strand, the distance would equal the length of the second strand? (I will have to go look at the discussions to find out if this is coming from there, though)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The case for both strands being empty is allowed as a distance of 0, as you mention, per the case specified on ln 14, but the ambiguity that prompts your question is partly why this change is proposed 😃

},
{
"uuid": "db92e77e-7c72-499d-8fe6-9354d2bfd504",
"description": "disallow left empty strand",
"comments": ["Normalises error messages"],
"reimplements": "5dce058b-28d4-4ca7-aa64-adfe4e17784c",
"property": "distance",
"input": {
"strand1": "",
"strand2": "G"
},
"expected": {"error": "left and right strands must be of equal length"}
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be a test that is identical to the test above, just different words for the same situation. And the reliance that a strand can not be 0 length.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is a reimplementation (see ln 97 reimplement property) of the test on ln 84 that normalises the error message to demonstrate that the logical result is actually the same as non-empty strands of unequal length (such as on ln 81).

Copy link
Member

Choose a reason for hiding this comment

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

I see in #97 but it doesn't seem to be related.

Copy link
Member

Choose a reason for hiding this comment

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

I see #1762 though. I think this is what you meant be "In 97"?

Copy link
Member

Choose a reason for hiding this comment

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

Nope, that doesn't make sense. Sorry very confused at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

In ln Sorry, apparently the font on my browser doesn't allow the visual distinction between what I thought was a capital "I" (eye) and a lower case "l" (ell) and though it was a typographical error, wrong case for "in".

Copy link
Member

Choose a reason for hiding this comment

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

We really should use "line" when we mean to say "line" to avoid confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, my bad! Will use "line" going forward

},
{
"uuid": "38826d4b-16fb-4639-ac3e-ba027dec8b5f",
"description": "disallow right empty strand",
"property": "distance",
"input": {
"strand1": "G",
"strand2": ""
},
"expected": {"error": "right strand must not be empty"}
},
{
"uuid": "920cd6e3-18f4-4143-b6b8-74270bb8f8a3",
"description": "disallow right empty strand",
"comments": ["Normalises error messages"],
"reimplements": "38826d4b-16fb-4639-ac3e-ba027dec8b5f",
"property": "distance",
"input": {
"strand1": "G",
"strand2": ""
},
"expected": {"error": "left and right strands must be of equal length"}
Copy link
Member

Choose a reason for hiding this comment

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

See prior comments, it is just mirrored here.

}
]
}