Skip to content
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

Add case sensitivity on spec type strings. #86

Closed
wants to merge 1 commit into from

Conversation

jippeholwerda
Copy link

Ran into an issue where unit testing a model named NewsOverview resulted in the test being a subclass of MiniTest::Rails::ActionView::TestCase instead of MiniTest::Spec.
We propose to make the spec types case sensitive again.

@blowmage
Copy link
Collaborator

blowmage commented Dec 5, 2012

Did your tests use the constant?

describe NewsOverview do

Or do they use a string?

describe "NewsOverview" do

@jippeholwerda
Copy link
Author

I tried both, with the same result.

@blowmage
Copy link
Collaborator

blowmage commented Dec 5, 2012

Okay. sigh

We need to rethink the naming convention then. I don't think just making it case sensitive will be enough.

@jippeholwerda
Copy link
Author

I agree that a better solution should be found. In the mean time, however, I would like to propose to revert the case insensitivity change.

On Dec 5, 2012, at 19:05 , blowmage notifications@github.com wrote:

Okay. sigh

We need to rethink the naming convention then. I don't think just making it case sensitive will be enough.


Reply to this email directly or view it on GitHub.

@blowmage
Copy link
Collaborator

I made some changes to the rpec_type registration in the 0.5 release, but I totally forgot to write any tests around this specific situation. Can you check the latest release and see if describe NewsOverview do or describe "NewsOverview" do work for you now? I think they should, but I don't have any tests that prove it right now.

@blowmage blowmage closed this in a02a7f1 Jan 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants