-
-
Notifications
You must be signed in to change notification settings - Fork 328
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
Remove mime-types
, rexml
, and webrick
runtime dependencies
#559
Changes from 4 commits
dbc735f
f6c8f88
4d2373b
a1ad97d
10649a3
7928281
7f667a8
081bd2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -65,6 +65,8 @@ | |||
} | ||||
|
||||
step 'I append to "features/support/env.rb" with:', selenium_config | ||||
step 'I append to "Gemfile" with:', "gem 'webrick', group: 'test'\n" | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These references to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||||
run_command_and_stop('bundle install --jobs 4') | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Install the
This configuration is optional for downstream projects. If developers choose to use |
||||
end | ||||
|
||||
Given('I force {string} to use select boxes for dates') do |file| | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
require 'cucumber' | ||
require 'capybara' | ||
|
||
module CucumberRailsHelper | ||
module CucumberRailsHelper # rubocop:todo Layout/ModuleLength | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wherever possible we're trying to avoid doing in-line exclusions and using the top level control with a comment explaining why. Ideally longer term we should compress this down a bit. But it is test only code, so it's low prio. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fixed this by removing a superfluous line in |
||
def rails_new(options = {}) | ||
validate_rails_new_success(run_rails_new_command(options)) | ||
cd options[:name] | ||
|
@@ -16,10 +16,10 @@ def rails_new(options = {}) | |
|
||
def install_cucumber_rails(*options) | ||
add_conditional_gems(options) | ||
add_rails_specific_gems | ||
|
||
add_gem 'cucumber', Cucumber::VERSION, group: :test | ||
add_gem 'capybara', Capybara::VERSION, group: :test | ||
add_gem 'selenium-webdriver', '~> 3.11', group: :test | ||
add_gem 'rspec-expectations', '~> 3.7', group: :test | ||
add_gem 'database_cleaner', '>= 1.8.0', group: :test unless options.include?(:no_database_cleaner) | ||
add_gem 'database_cleaner-active_record', '>= 2.0.0.beta2', group: :test if options.include?(:database_cleaner_active_record) | ||
|
@@ -79,8 +79,8 @@ def run_rails_new_command(options) | |
options[:name] ||= 'test_app' | ||
flags = %w[ --skip-action-cable --skip-action-mailer --skip-active-job --skip-bootsnap --skip-bundle --skip-javascript | ||
--skip-jbuilder --skip-listen --skip-spring --skip-sprockets --skip-test-unit --skip-turbolinks ] | ||
flags += %w[--skip-active-storage] if rails_5_2_or_higher? | ||
flags += %w[--skip-action-mailbox --skip-action-text] if rails_6_0_or_higher? | ||
flags += %w[--skip-active-storage] if rails_equal_or_higher_than?('5.2') | ||
flags += %w[--skip-action-mailbox --skip-action-text] if rails_equal_or_higher_than?('6.0') | ||
run_command "bundle exec rails new #{options[:name]} #{flags.join(' ')} #{options[:args]}" | ||
end | ||
|
||
|
@@ -94,12 +94,8 @@ def clear_bundle_env_vars | |
delete_environment_variable 'BUNDLE_GEMFILE' | ||
end | ||
|
||
def rails_5_2_or_higher? | ||
Rails.gem_version >= Gem::Version.new('5.2') | ||
end | ||
|
||
def rails_6_0_or_higher? | ||
Rails.gem_version >= Gem::Version.new('6.0') | ||
def rails_equal_or_higher_than?(version) | ||
Rails.gem_version >= Gem::Version.new(version) | ||
luke-hill marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end | ||
|
||
def add_conditional_gems(options) | ||
|
@@ -108,11 +104,15 @@ def add_conditional_gems(options) | |
else | ||
add_gem 'cucumber-rails', group: :test, require: false, path: File.expand_path('.').to_s | ||
end | ||
end | ||
|
||
if rails_6_0_or_higher? | ||
def add_rails_specific_gems | ||
if rails_equal_or_higher_than?('6.0') | ||
add_gem 'sqlite3', '~> 1.4' | ||
add_gem 'selenium-webdriver', '~> 4', group: :test | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. stylistically I'm not a fan of single digits in the twiddle. I know it works, but for completeness / style can we say 4.0 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated in |
||
else | ||
add_gem 'sqlite3', '~> 1.3.13' | ||
add_gem 'selenium-webdriver', '~> 3.11', group: :test | ||
end | ||
end | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still support lower ruby versions. I'm kinda on the fence about this, as ideally people should be controlling their own version constraints if they need this. So I think I'm 50-50 about this. I don't see the harm in retaining this until we're ruby 3.0+ only (Which isn't likely to be too long at current development rates).
That being said, if you're passionate about this, I don't see the harm in making this change. It was originally made when we introduced ruby3 into our support matrix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My argument is there is no code in the
cucumber-rails
gem that usesrexml
orwebrick
. It operates identically, with or without these dependencies, on all supported versions of Ruby. There is no value in declaring them as runtime dependencies.