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

Restore compatibility with database_cleaner < 1.8.0.beta #473

Conversation

cgriego
Copy link
Member

@cgriego cgriego commented Jun 26, 2020

Summary

DatabaseCleaner::VERSION didn't exist until 1.8.0.beta, so having an older version causes an uninitialized constant error instead of using the old behavior.

Details

When upgrading the cucumber-rails 2.1, my @javascript scenarios failed with uninitialized constant DatabaseCleaner::VERSION (NameError) errors. cucumber-rails has conditional logic checking if DatabaseCleaner::VERSION is greater than or equal to 1.8.0.beta, but DatabaseCleaner::VERSION didn't exist until 1.8.0.beta.

Motivation and Context

The code change that added support for newer versions of Database Cleaner was written with the intention of maintaining support for older versions of Database Cleaner, but it failed in practice. This change restores compatibility with older versions.

How Has This Been Tested?

I tested with my app still using Database Cleaner 1.7.0. It failed when using the release version of cucumber-rails 2.1, but succeeded using the versions of cucumber-rails in this pull request.

I did not add an automated test because, outside of Rails itself, the project doesn't seem to take on the overhead of testing with older versions of libraries and an automated test for backwards compatibility wasn't added with the original change. If I'm wrong in that assessment, I'm happy to add an automated test however the project would like.

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

DatabaseCleaner::VERSION didn't exist until 1.8.0.beta, so having an
older version causes an uninitialized constant error instead of using
the old behavior.
Copy link
Contributor

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

Looks reasonable, and the PR description tells the whole story. LGTM!

Thanks, @cgriego!

@olleolleolle olleolleolle merged commit 9b8f155 into cucumber:master Jun 27, 2020
@aslakhellesoy
Copy link
Contributor

Hi @cgriego,

Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾

In return for this generous offer we hope you will:

  • ✅ Continue to use branches and pull requests. When someone on the core team approves a pull request (yours or someone else's), you're welcome to merge it yourself.
  • 💚 Commit to setting a good example by following and upholding our code of conduct in your interactions with other collaborators and users.
  • 💬 Join the community Slack channel to meet the rest of the team and make yourself at home.
  • ℹ️ Don't feel obliged to help, just do what you can if you have the time and the energy.
  • 🙋 Ask if you need anything. We're looking for feedback about how to make the project more welcoming, so please tell us!

On behalf of the Cucumber core team,
Aslak Hellesøy
Creator of Cucumber

cgriego added a commit to cgriego/cucumber-rails that referenced this pull request Jul 15, 2020
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