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

[CI] Fix the build on 3.0 #46

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

sfgeorge
Copy link
Member

@sfgeorge sfgeorge commented Aug 25, 2019

Up-port the CI fixes from #45 for RubyAMI 2.4 to 3.0:

  • Make the build green ✅
  • Test modern Rubies

sfgeorge and others added 23 commits August 24, 2019 09:59
Should get things passing on:
- ruby-2.5.3
- jruby-9.1.17.0
Makes it easier to ensure that the build stays green from the RSpec 2.99 -> RSpec 3.x transition.
This conversion is done by Transpec 3.4.0 with the following command:
    transpec -f

* 65 conversions
    from: obj.should
      to: expect(obj).to

* 51 conversions
    from: == expected
      to: eq(expected)

* 8 conversions
    from: it { should ... }
      to: it { is_expected.to ... }

* 6 conversions
    from: obj.should_not
      to: expect(obj).not_to

* 5 conversions
    from: =~ /pattern/
      to: match(/pattern/)

* 5 conversions
    from: it { should_not ... }
      to: it { is_expected.not_to ... }

* 3 conversions
    from: obj.should_receive(:message)
      to: expect(obj).to receive(:message)

* 2 conversions
    from: be_false
      to: be_falsey

* 2 conversions
    from: its(:attr) { }
      to: describe '#attr' do subject { super().attr }; it { } end

* 1 conversion
    from: lambda { }.should
      to: expect { }.to

* 1 conversion
    from: mock('something')
      to: double('something')

* 1 conversion
    from: obj.stub(:message)
      to: allow(obj).to receive(:message)

* 1 conversion
    from: pending
      to: skip

For more details: https://github.com/yujinakayama/transpec#supported-conversions
...with a documented, supported argument matcher block to #receive.

Unsupported in #with:
rspec/rspec-mocks#377 (comment)

Supported in #receive:
https://github.com/rspec/rspec-mocks#arbitrary-handling
Convert specs to RSpec 3.8.2 syntax with Transpec

This conversion is done by Transpec 3.4.0 with the following command:
    transpec -f

* 2 conversions
    from: obj.stub(:message => value)
      to: allow(obj).to receive_messages(:message => value)

For more details: https://github.com/yujinakayama/transpec#supported-conversions
@sfgeorge
Copy link
Member Author

@bklang @lpradovera @gfaza Can you review this for me?

Copy link
Member

@bklang bklang left a comment

Choose a reason for hiding this comment

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

LGTM, other than the question about Cucumber/Ruby 1.9.3

s.add_development_dependency %q<cucumber>, [">= 0"]
s.add_development_dependency %q<bundler>, [">= 1.0"]
s.add_development_dependency %q<rspec>, [">= 3.0"]
s.add_development_dependency %q<cucumber>, ["< 3.0"]
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this was done to make Ruby 1.9.3 happy, but we no longer need to support that ancient version. Should this restriction be lifted?

Copy link
Member Author

Choose a reason for hiding this comment

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

@bklang I was thinking about that too (actually, I completed the cuke upgrade locally and confirmed it's a small effort).

Reason for possibly not doing it.. a handful of cucumber features fail unless we fully embrace cucumber >= 3.0n syntax. But since the syntax is not backwards compatible, it means that the same features will have to remain different on support/2.x vs. develop.

I'd lean towards having the two branches have a common denominator, unless it drags down develop in some way.

But I can be swayed - I can update this branch to embrace cuke 3.0 if you like it. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Your answer makes sense to me. There's no urgency to upgrade cucumber, so let's leave it for now. As you say, we can do this work whenever we have a better reason to do it, and in the meantime we benefit from keeping backports simple. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Meant to reply back earlier... ... thanks man.

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.

3 participants