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

extend permitted rack-test version #268

Closed
wants to merge 1 commit into from

Conversation

lazylester
Copy link

rack-test 2.0.2 is now released. I ran all specs using rspec spec and everything passed. Should be sufficient, no?

@bethesque
Copy link
Member

Thanks for looking into this. The tests are necessary but not sufficient 😆 Before we upgrade, we need to be confident that there are no breaking changes that will affect the pact framework that might not be covered in the tests. The ruby codebase is used by Pact in many other languages via the pact-ruby-standalone package, so there's a big impact if there are unexpected changes. I'll have a look at the changelog and see if anything stands out to me.

@bethesque
Copy link
Member

I've just updated ruby 2.2 to ruby 2.4 in the test matrix. I know both versions are unsupported, however, we still need ruby 2.4 for the pact-ruby-standalone package that we make with Travelling Ruby. Can you merge in the change and see if that makes the first test in the test matrix pass?

@lazylester
Copy link
Author

What triggers running the tests against the test matrix? Looks as though it might be related to pushing to github? maybe? Any tips on testing PR's would be nice to see in the "contributing" page. It just says "run the tests". Or is is common practice just to make it the default rake task?

@bethesque
Copy link
Member

bundle exec rake is the standard. You're right, the docs should have more info in them.

@bethesque
Copy link
Member

What triggers running the tests against the test matrix?

A git push. See the github workflow config file here https://github.com/pact-foundation/pact-ruby/blob/master/.github/workflows/test.yml#L3

@bethesque
Copy link
Member

I've updated the testing matrix again, because there were issues with ruby 2.4. Can you merge from master please?

@bethesque
Copy link
Member

Each of these items needs to be checked against the code in both pact-ruby, pact-support, and the pact-mock_service gem.

https://github.com/rack/rack-test/blob/main/History.md#200--2022-06-24

@lazylester
Copy link
Author

Beth, I had no idea. I thought I was just proposing a simple change, that didn't degrade the test suite. But it became much bigger than my skill set or my ability to follow up! I can't do this justice, thanks for being responsive and thanks for the work you and the team do.

@lazylester lazylester closed this Aug 19, 2022
@bethesque
Copy link
Member

No worries. You're absolutely right that it should be updated. We just have to make sure it's done carefully. I'll put it on my list of things to do.

@stanhu
Copy link
Contributor

stanhu commented Aug 23, 2022

I don't think there's much to worry about here. I don't think any of these modules depend on the deprecated methods.

  1. pack-support doesn't depend on rack-test anymore since pact-foundation/pact-support@80bbdcc
  2. pack-mock_services just runs include Rack::Test and already allows for rack-test 2: https://github.com/pact-foundation/pact-mock_service/blob/f33a00710cf3b9e32adb9521c86663332ef2c05d/pact-mock_service.gemspec#L33:
% git grep Rack::Test
spec/features/administration_endpoints_cors_spec.rb:  include Rack::Test::Methods
spec/features/missing_interactions_spec.rb:  include Rack::Test::Methods
spec/features/mock_interactions_using_put_interactions_spec.rb:  include Rack::Test::Methods
spec/features/mock_interactions_with_cors_spec.rb:  include Rack::Test::Methods
spec/features/mock_interactions_with_same_description_and_provider_state_spec.rb:  include Rack::Test::Methods
spec/features/mock_multiple_responses_spec.rb:  include Rack::Test::Methods
spec/features/mock_one_response_spec.rb:  include Rack::Test::Methods
spec/features/write_pact_file_spec.rb:  include Rack::Test::Methods
spec/lib/pact/consumer/mock_service_spec.rb:      include Rack::Test::Methods
  1. pact-ruby only uses include Rack::Test::Methods:
% git grep Rack::Test
documentation/configuration.md:Pact uses RSpec and Rack::Test to create dynamic specs based on the pact files. RSpec configuration can be used to modify test behaviour if there is not an appropriate Pact feature. If you wish to use the same spec_helper.rb file as your unit tests, require it in the pact_helper.rb, but remember that the RSpec configurations for your unit tests may or may not be what you want for your pact verification tests.
lib/pact/provider/test_methods.rb:      include Rack::Test::Methods

@bethesque
Copy link
Member

New version of pact gem released with this change.

@bethesque
Copy link
Member

Thanks for looking into it @stanhu. I've opened the rack-test version for the ruby gem, but locked it for the pact-ruby-standalone and pact-cli for now, until the Ruby gem users give it a good shake out.

@stanhu
Copy link
Contributor

stanhu commented Sep 29, 2022

@bethesque Thanks! For some reason, I'm not seeing the new gem? https://rubygems.org/gems/pact

@bethesque
Copy link
Member

Sorry, the release failed. Try now.

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