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.

Sign up for GitHub

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

Jump to bottom

Ruby 2.0 tests #194

Merged
merged 10 commits into from
Apr 10, 2013
Merged

Ruby 2.0 tests #194

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7040a45
multiple_choices? with an s
Mar 20, 2013
66127fb
maybe a good way to test proc equality
Mar 20, 2013
3f27f99
Preserve 1.9.3 compatibility.
Mar 20, 2013
773010e
check for dup proc by comparing call value of a test proc
Mar 25, 2013
c77bfa2
remove silly VM nonsense
Mar 25, 2013
b123ec2
adding 2.0.0 to travis test
Mar 25, 2013
ed0e877
comapre exception class by is_a rather then class.name ==
Mar 25, 2013
9e778b1
comparing parent proc with call
Mar 25, 2013
494d95f
<= 1.9.3 ruby namespace lookup constant
Mar 25, 2013
a0a07c5
support both multiple_choice? and multiple_choices?
Mar 26, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ rvm:
- 1.8.7
- ree
- 1.9.3
- 2.0.0
notifications:
email: false
bundler_args: --without development
2 changes: 1 addition & 1 deletion features/steps/httparty_response_steps.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@

Then /it should raise (?:an|a) ([\w:]+) exception/ do |exception|
@exception_from_httparty.should_not be_nil
@exception_from_httparty.class.name.should eql(exception)
@exception_from_httparty.should be_a(Object.const_get("::#{exception}"))
end
2 changes: 1 addition & 1 deletion spec/httparty/response_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def response_mock(klass)
:length_required? => Net::HTTPLengthRequired,
:method_not_allowed? => Net::HTTPMethodNotAllowed,
:moved_permanently? => Net::HTTPMovedPermanently,
:multiple_choice? => Net::HTTPMultipleChoice,
:multiple_choices? => Net::HTTPMultipleChoice,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to test both, I think I would just move this one to a separate example, rather than as one in the huge list. I would then maybe just do an if RUBY_VERSION check around each of them. RUBY_VERSION >= '2.0.0' => multiple_choices? and < '2.0.0' multiple_choice?. Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the real question is should the code be changing the method name per version of ruby, this might be confusion to users. The new variable name is consistant with the proper name of the response.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, gotcha. I would think both should work. One version should alias to the other. What about that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, too bad though, the beautiful dynamic method declaration will need hacks.

:no_content? => Net::HTTPNoContent,
:non_authoritative_information? => Net::HTTPNonAuthoritativeInformation,
:not_acceptable? => Net::HTTPNotAcceptable,
Expand Down
4 changes: 2 additions & 2 deletions spec/httparty_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,11 +599,11 @@ def self.name
end

it 'should dup the proc on the child class' do
imaginary_option = lambda { "This is a new lambda" }
imaginary_option = lambda { 2 * 3.14 }
@parent.default_options[:imaginary_option] = imaginary_option
@parent.default_options[:imaginary_option].should be_equal imaginary_option
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be testing equality? Seems like it should be the same as below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure what you mean, my change tests the call value of the proc for equality, and keeps the test for the proc not being the same object.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the point of changing the block and swapping out the equality check for the invocation check was because the equality check didn't work, but the @parent is still using the equality check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see what you mean, yea that makes perfect sense.

@child1.default_options[:imaginary_option]
@child1.default_options[:imaginary_option].should == imaginary_option
@child1.default_options[:imaginary_option].call.should == imaginary_option.call
@child1.default_options[:imaginary_option].should_not be_equal imaginary_option
end

Expand Down