search

thoughtbot / shoulda-context

Shoulda Context makes it easy to write understandable and maintainable tests under Minitest and Test::Unit within Rails projects or plain Ruby projects.
MIT License
239 stars 58 forks source link

Raise on duplicate test name #26

Closed grosser closed 11 years ago

grosser commented 11 years ago

Just found a bunch of duplicate test names, would be nice if it would raise instead of just warn since it basically means the test is not getting executed which is pretty bad ...

https://github.com/thoughtbot/shoulda-context/blob/master/lib/shoulda/context/context.rb#L388

rmm5t commented 11 years ago

Agreed, that is pretty bad, but I'm not sure a full error is warranted. What's causing most of your concern? Is it that the potential warnings are getting ignored by the team?

For comparison, redefining a constant is pretty bad too, but Ruby just prints a warning in that scenario.

drapergeek commented 11 years ago

I have to agree that I don't think this should raise. I think the warning is sufficient.

grosser commented 11 years ago

There is a good use case for overwriting a constant and it can happen when some require goes wrong, but there is no good use case for overwriting a test in shoulda-land -> you lost a test-case -> false sense of security.

What is the reason to not raise? Is there any good use case for "silently overwriting a test" ? <-> just raise since nobody needs this "feature".

On Wed, Jun 19, 2013 at 8:15 AM, Jason Draper notifications@github.comwrote:

I have to agree that I don't think this should raise. I think the warning is sufficient.

— Reply to this email directly or view it on GitHubhttps://github.com/thoughtbot/shoulda-context/issues/26#issuecomment-19690547 .

travisjeffery commented 11 years ago

I'm in favor of raising. I think Ruby's loose idea of a constant is bad anyway, and here we're talking about test suites which you want to be honest with but may be lying here.

Lots of times people have warnings turned off too, like if you're running a big Rails app and you're using deprecated methods then you may turn warnings off until the day you go to remove the deprecations and that would be the day you discover these shoulda-context warnings and possibly discover that your test suite's incorrect.