Replace minitest.cr in favor of std-lib spec

crystal-lang / shards

Dependency manager for the Crystal language

Other

765 stars 102 forks source link

Replace minitest.cr in favor of std-lib spec #334

Closed bcardiff closed 4 years ago

bcardiff commented 4 years ago

This pr removes minitest to keep dependencies within crystal-lang org.

Since the integration spec use data initialized once arranche the spec to be spec/unit and spen/integration.

The Makefile targets remains the same.
I left as comments the custome failure messages.
I left the helper assertion methods used in integration specs that have no matching in std-lib spec.
To make typing happier, I introduced run! to be the alternative of run that captures the output.
The MockResolver was dead code so it is removed.

Nothing against minitest.cr (sorry @ysbaddaden!) but is important to keep dependencies within the org.

bcardiff commented 4 years ago

Yeah, there some advantage of minitest in some scenarios.

Another thing I miss in the current spec that is present in rspec is the expect(expr).to ... because it gives a cleaner prefix of an assertion. Something that minitest have.

Was there a reason to not capture all the output in run commands or was more an evolution of the code? Because we would make the output to always be captured.

ysbaddaden commented 4 years ago

Yes, we should just always capture, it shouldn't be noticeably slower.

bcardiff commented 4 years ago

@straight-shoota some feedback addressed. I didn't want to change/refactor the specs a lot. I think that can be handled later. The goal is to drop the non org dependency and allow compilation on master easily.

Improvements in the specs can come later IMO.

We need to fix build on master and promptly tag a molinillo / shards release so Crystal nightlies can build again.

bcardiff commented 4 years ago

Regarding the commit history, I've done some rebases along the way and that bothers github ui. But we can squash all these commits if preferred. I commit it that way in case other PRs were merged in between so a rebase would be easier for me.

jkthorne commented 4 years ago

I still would like to see a test unit style added to the std-lib. I think it is a little weird that a project under the Crystal Org cannot have a test unit style.

asterite commented 4 years ago

I agree. We should eventually enhance spec to provide test-unit like tests. This is not only a cosmetic matter: with test-unit you can define classes, inheritance, initializers (so better setup), share variables, etc.

But we can do it later, even after 1.0

jkthorne commented 4 years ago

@asterite I am sure more people have asked but I made a post about it a while ago. https://github.com/crystal-lang/crystal/issues/7042

asterite commented 4 years ago

On second though, there's minitest.cr which can be used by any project. I guess it's find that crystal-lang org projects only use specs. That way any core contributor only needs to know spec.

straight-shoota commented 4 years ago

This is definitely not the topic of this discussion.

bcardiff commented 4 years ago

@straight-shoota are we good on your side to merge this as is?

ysbaddaden commented 4 years ago

Yes, please squash to keep history nice. Also, as @RX14 pointed out, there seems to be Spec.before_all after all.