Fix Ctrl-C on Ruby v1.8.7. #114

Merged
merged 2 commits into from Nov 12, 2012

Conversation

Projects
None yet
2 participants
Contributor

moll commented Nov 7, 2012

Ruby v1.8.7's Module.constants returns strings, not symbols.

Fix Ctrl-C on Ruby v1.8.7.
Ruby v1.8.7's Module.constants returns strings, not symbols.
Contributor

moll commented Nov 7, 2012

I'm guessing it's okay that the build failed with Rails 3, because you've set up "Allowed Failures" in Travis for that?

Contributor

ragaskar commented Nov 7, 2012

The allowed fails are jruby and rbx (and only because I haven't flipped them back, I think those are mostly passing now, but it's always a crapshoot). The rails3 runs across the 'standard' rubies should pass.

So the dependency tests are funny (and a little bad) in that they only run when rspec2 is present. It actually looks like this spec is failing generally -- ie, everywhere it gets run (when rspec2 is present), it fails. This test probably needs to be updated to pass:

  1. Jasmine::Dependencies legacy_rack? should return true if Rack::Server does not exist
    143 Failure/Error: Jasmine::Dependencies.legacy_rack?.should be_true
    144 expected: true value
    145 got: false
    146 # ./spec/dependencies_spec.rb:322
Contributor

moll commented Nov 7, 2012

Oh, sorry for missing that test.

However, I think the approach for legacy_rack? test is wrong. It's mocking and testing a single implementation based on how we think it works, and as we saw, actually causes a false positive. It's meaningless and would be as well if we were to stub now Rack::Server.
To be a proper test it'd actually need to load an old version of Rack. Could you help with that? :)

Contributor

ragaskar commented Nov 7, 2012

That's fair; I'm happy to have this test deleted, given that I think we integration test this functionality by verifying that Jasmine works with rails 2.3.5.

Remove legacy_rack? tests.
It was mocking and testing a single implementation based on how we think it works, and as we saw, actually caused a false positive.
Contributor

moll commented Nov 11, 2012

Lights seem to say go. :)

ragaskar added a commit that referenced this pull request Nov 12, 2012

Merge pull request #114 from moll/patch-1
Fix Ctrl-C on Ruby v1.8.7.

@ragaskar ragaskar merged commit 54b37cf into jasmine:master Nov 12, 2012

1 check passed

default The Travis build passed
Details
Contributor

ragaskar commented Nov 12, 2012

Merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment