Conversation
jiegillet
left a comment
jiegillet
left a comment
There was a problem hiding this comment.
Nice! Only one thing I would revert.
| } | ||
|
|
||
| pub fn a_m_and_upper_a_m_are_26_different_characters_but_not_a_pangram_test() { | ||
| pub fn a_m_and_a_m_are_26_different_characters_but_not_a_pangram_test() { |
There was a problem hiding this comment.
I think the previous title was better, lost in generation I suppose
There was a problem hiding this comment.
Hm, yeah you're right, they get lost in the generation. I'm going to see if I can improve the test title upstream and regenerate.
lpil
left a comment
lpil
left a comment
There was a problem hiding this comment.
Nice! Could you revert the should.be_true be changes please? It produces a worse error that does not include the location of the assertion.
|
I took another look at this and the include=false should actually be present, because it's been reimplemented with a different description and different (but equivalent) data. I'm going to put this one on hold while I look at the upstream fix. |
3 participants
This removes an include=false from the tests.toml, as the
exclusion seems to have been unintentional (the new test
passes with no changes to the example solution).
It also switches to use the should.be_false / should.be_true style
that other tests use, rather than assert False / assert True.