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

Restrict pending the spec to only Ruby versions 2.2.0, 2.2.1, 2.2.2 #313

Merged

Conversation

pboling
Copy link
Contributor

@pboling pboling commented Oct 18, 2015

Fixes #312

  • Restrict pending the spec to only Ruby versions 2.2.0, 2.2.1, 2.2.2
  • Better paradigm for pending specs due to bugs in interpreter
    • new gem rspec-pending_for, written by yours truly.
    • Gem is based on ruby_version and ruby_engine gems. Just using them directly did not actually reduce the complexity much.

@pboling pboling force-pushed the issue/312-ruby-2.2.3-fixes-pended-mash-spec branch from e822f48 to 3cc9955 Compare October 18, 2015 19:18
# For things Broken in Ruby <= 2.2.2, Fixed in 2.2.3+
def pending_for_mri220_to_mri222
version = ruby_version
return unless version.start_with?('ruby_2.2') && version.end_with?('0', '1', '2')
Copy link
Member

Choose a reason for hiding this comment

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

There's a finate list of these, can we just check for something like ['ruby_2.2....].include?.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Changing.

@pboling pboling force-pushed the issue/312-ruby-2.2.3-fixes-pended-mash-spec branch from 3cc9955 to 52476cb Compare October 19, 2015 04:35
if mri22?
pending 'Bug in MRI 2.2.x means this behavior is broken in those versions'
end
pending_for(engine: 'ruby', versions: %w(2.2.0 2.2.1 2.2.2))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dblock How do you like me now. 😸

@pboling pboling force-pushed the issue/312-ruby-2.2.3-fixes-pended-mash-spec branch from 52476cb to e79659e Compare October 19, 2015 04:40
@pboling
Copy link
Contributor Author

pboling commented Oct 19, 2015

@dblock this is ready, and output, running specs on Ruby 2.2.2 looks like:

Pending:
  Hashie::Mash#respond_to? is able to access an unknown suffixed key as a method
    # This behavior is broken in Ruby versions ["2.2.0", "2.2.1", "2.2.2"] due to a bug in the Ruby engine (MRI >= 1.9 or REE)
    # ./spec/hashie/mash_spec.rb:365

Finished in 0.13778 seconds (files took 0.41187 seconds to load)
560 examples, 0 failures, 1 pending

@pboling pboling force-pushed the issue/312-ruby-2.2.3-fixes-pended-mash-spec branch from 453101c to feb9472 Compare October 19, 2015 05:00
- Better paradigm for pending specs due to bugs in interpreter
@pboling pboling force-pushed the issue/312-ruby-2.2.3-fixes-pended-mash-spec branch from feb9472 to a44dd9a Compare October 19, 2015 05:16
@pboling pboling mentioned this pull request Oct 19, 2015
1 task
@dblock
Copy link
Member

dblock commented Oct 19, 2015

It's an amazing effort. However am I wrong that most of this is implemented in https://github.com/janlelis/ruby_version or https://github.com/martinkozak/ruby-version?

RubyVersion.between? '2.2.0', '2.2.2'

@michaelherold
Copy link
Member

You're right about that, dB. I made the conscious decision not to pull in one of those back when I added the simple check.

If we need the more complicated logic, it might make sense to pull one of them in.

@dblock
Copy link
Member

dblock commented Oct 19, 2015

Well what's here is more complicated logic. This entire thing should be a 1-liner, but it should be robust, Hashie is not in the business of parsing ruby versions. Would you like to try with one of those gems?

@dblock
Copy link
Member

dblock commented Oct 19, 2015

And sorry to be a pest ;)

@pboling
Copy link
Contributor Author

pboling commented Oct 19, 2015

I'll switch to a maintained gem. My StrictKeyAccess / Dictionary PR has some specs that are pended for Rubinius, and utilized this complication.

@dblock
Copy link
Member

dblock commented Oct 19, 2015

Thanks @pboling - hang in there! Lets get this PR done first. Add a note when you amended it.

@pboling
Copy link
Contributor Author

pboling commented Oct 19, 2015

@dblock Having reviewed the options I propose we use:

Because they appear maintained, and the syntax is good.

@dblock
Copy link
Member

dblock commented Oct 19, 2015

I don't care which gem personally as long as we have something like a 1-liner in hashie ;)

- a one liner with no complexity for pending specs by Ruby Engine / Version
- removes all complexity from Hashie
@pboling
Copy link
Contributor Author

pboling commented Oct 22, 2015

@dblock I used those two gems, but the complexity remained in Hashie, so I rolled it out into a separate gem, as it seems like a common, generic, basic need of many projects, that I think I will want to use... everywhere.

https://github.com/pboling/rspec-pending_for

@pboling
Copy link
Contributor Author

pboling commented Oct 22, 2015

If this is acceptable, then I will rebase and squash commits. Let me know.

@dblock
Copy link
Member

dblock commented Oct 22, 2015

👍 on rspec-pending-for !

dblock added a commit that referenced this pull request Oct 22, 2015
…d-mash-spec

Restrict pending the spec to only Ruby versions 2.2.0, 2.2.1, 2.2.2
@dblock dblock merged commit 5c276eb into hashie:master Oct 22, 2015
@dblock
Copy link
Member

dblock commented Oct 22, 2015

Awesome, merged.

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Dec 12, 2015
## 3.4.3 (10/25/2015)

* [#314](hashie/hashie#314): Added a
  `StrictKeyAccess` extension that will raise an error whenever a key is
  accessed that does not exist in the hash -
  [@pboling](https://github.com/pboling).

* [#304](hashie/hashie#304): Ensured compatibility
  of `Hash` extensions with singleton objects -
  [@regexident](https://github.com/regexident).

* [#306](hashie/hashie#306): Added
  `Hashie::Extensions::Dash::Coercion` -
  [@marshall-lee](https://github.com/marshall-lee).

* [#310](hashie/hashie#310): Fixed
  `Hashie::Extensions::SafeAssignment` bug with private methods -
  [@marshall-lee](https://github.com/marshall-lee).

* [#313](hashie/hashie#313): Restrict pending spec
  to only Ruby versions 2.2.0-2.2.2 - [@pboling](https://github.com/pboling).

* [#315](hashie/hashie#315): Default `bin/` scripts:
  `console` and `setup` - [@pboling](https://github.com/pboling).
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

3 participants