-
-
Notifications
You must be signed in to change notification settings - Fork 395
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
Rewrite tests and example for CustomSet #226
Conversation
eb6bf94
to
1539db4
Compare
@devonestes excellent work as usual! I think I have a couple small comments. This diff is a little challenging to review. I'm mostly looking at it as a new exercise. |
test "sets with elements are not empty" do | ||
custom_set = CustomSet.new([1]) | ||
assert CustomSet.empty?(custom_set) == false | ||
end | ||
end | ||
|
||
@tag :pending |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the describe
blocks. Are you at all worried that @pending
these blocks will make too many test go red at one time? It seems ok since each is focused on one function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frankly, I didn't really think about it too much since that was a refactor. But now that you mention it, I don't think it's a problem. When I was doing the implementation for these, the examples in the tests pretty much all went green at the same time since there aren't really many edge cases to handle in these operations.
1539db4
to
dfbe2ae
Compare
We have a lot of issues open around the `CustomSet` example. This PR rewrites that to remove the deprecated `Set` module, and also updates it to use the current tests listed in the `exercism/x-common` repo. This should address Issues exercism#121, exercism#180, exercism#189, exercism#198 and exercism#217.
dfbe2ae
to
b81dbdd
Compare
When we ran `mix test` we were getting several compiler warnings. Here's the ouput that we got before this PR: ``` warning: behaviour Set undefined exercises/custom-set/example.exs:1 warning: the Inspect protocol has already been consolidated, an implementation for CustomSet has no effect exercises/custom-set/example.exs:126 warning: function digits/1 is private, @doc's are always discarded for private functions exercises/largest-series-product/example.exs:3 warning: function slices/2 is private, @doc's are always discarded for private functions exercises/largest-series-product/example.exs:17 warning: redefining module DNA (current version defined in memory) exercises/nucleotide-count/example.exs:1 warning: redefining module DNA (current version defined in memory) exercises/rna-transcription/example.exs:1 Including tags: [:pending] warning: redefining module ChangeTest (current version defined in memory) exercises/flatten-array/flatten_array_test.exs:8 warning: redefining module ChangeTest (current version defined in memory) exercises/hexadecimal/hexadecimal_test.exs:8 warning: redefining module DNATest (current version defined in memory) exercises/nucleotide-count/nucleotide_count_test.exs:8 warning: redefining module DNATest (current version defined in memory) exercises/rna-transcription/rna_transcription_test.exs:8 warning: the Inspect protocol has already been consolidated, an implementation for BinTree has no effect exercises/zipper/zipper_test.exs:16 ``` I addressed all of the warnings except those for `CustomSet` since those warnings are taken care of in exercism#226. Many of them were naming issues for modules, and funny enough this fix of the compiler warnings pointed out that we had a typo in a test that should have failed but wasn't failing! I've also fixed that typo.
When we ran `mix test` we were getting several compiler warnings. Here's the ouput that we got before this PR: ``` warning: behaviour Set undefined exercises/custom-set/example.exs:1 warning: the Inspect protocol has already been consolidated, an implementation for CustomSet has no effect exercises/custom-set/example.exs:126 warning: function digits/1 is private, @doc's are always discarded for private functions exercises/largest-series-product/example.exs:3 warning: function slices/2 is private, @doc's are always discarded for private functions exercises/largest-series-product/example.exs:17 warning: redefining module DNA (current version defined in memory) exercises/nucleotide-count/example.exs:1 warning: redefining module DNA (current version defined in memory) exercises/rna-transcription/example.exs:1 Including tags: [:pending] warning: redefining module ChangeTest (current version defined in memory) exercises/flatten-array/flatten_array_test.exs:8 warning: redefining module ChangeTest (current version defined in memory) exercises/hexadecimal/hexadecimal_test.exs:8 warning: redefining module DNATest (current version defined in memory) exercises/nucleotide-count/nucleotide_count_test.exs:8 warning: redefining module DNATest (current version defined in memory) exercises/rna-transcription/rna_transcription_test.exs:8 warning: the Inspect protocol has already been consolidated, an implementation for BinTree has no effect exercises/zipper/zipper_test.exs:16 ``` I addressed all of the warnings except those for `CustomSet` since those warnings are taken care of in #226. Many of them were naming issues for modules, and funny enough this fix of the compiler warnings pointed out that we had a typo in a test that should have failed but wasn't failing! I've also fixed that typo.
Looks great! 🎉 |
We have a lot of issues open around the
CustomSet
example. This PR rewritesthat to remove the deprecated
Set
module, and also updates it to use thecurrent tests listed in the
exercism/x-common
repo.resolves #121
resolves #180
resolves #189
resolves #198
resolves #217