rna-transcription 1.0.0: reject invalid inputs#292
Merged
petertseng merged 2 commits intoexercism:masterfrom May 30, 2017
petertseng:rna
Merged
rna-transcription 1.0.0: reject invalid inputs#292petertseng merged 2 commits intoexercism:masterfrom petertseng:rna
petertseng merged 2 commits intoexercism:masterfrom
petertseng:rna
Conversation
The invalid input cases originally came from exercism/problem-specifications#119 Today's JSON file can be found at https://github.com/exercism/x-common/blob/master/exercises/rna-transcription/canonical-data.json This track was already using all the valid inputs and simply needed to add these invalid inputs.
petertseng
commented
May 19, 2017
| #[ignore] | ||
| fn test_transcribes_cytosine_guanine() { | ||
| assert_eq!(dna::RibonucleicAcid::new("G"), dna::DeoxyribonucleicAcid::new("C").to_rna()); | ||
| assert_eq!(Ok(dna::RibonucleicAcid::new("G")), dna::DeoxyribonucleicAcid::new("C").to_rna()); |
Member
Author
There was a problem hiding this comment.
Well, this is a different approach from, say, #238 which used unwrap. Note that https://github.com/exercism/xrust/blob/master/exercises/nucleotide-codons/tests/codons.rs does use assert_eq with Ok though. I think I like this better. The difference is what happens when I return an Err when I should return Ok. With assert_eq!(Ok(...), ...):
---- test_transcribes_cytosine_guanine stdout ----
thread 'test_transcribes_cytosine_guanine' panicked at 'assertion failed: `(left == right)` (left: `Ok(RibonucleicAcid { nucleotides: "G" })`, right: `Err(())`)', tests/rna-transcription.rs:12
With assert_eq!(..., ....unwrap());
---- test_transcribes_cytosine_guanine stdout ----
thread 'test_transcribes_cytosine_guanine' panicked at 'called `Result::unwrap()` on an `Err` value: ()', src/libcore/result.rs:859
So why don't I go ahead and say now that I like assert_eq(Ok(...), ...) better since it shows me both sides.
ijanos
approved these changes
May 30, 2017
Contributor
ijanos
left a comment
There was a problem hiding this comment.
I think using unwrap in an assert is an antipattern and we should avoid it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The invalid input cases originally came from
exercism/problem-specifications#119
Today's JSON file can be found at
https://github.com/exercism/x-common/blob/master/exercises/rna-transcription/canonical-data.json
This track was already using all the valid inputs and simply needed to
add these invalid inputs.