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

Only load SystemTestCase if Puma is defined #28160

Merged

Conversation

@y-yagi
Copy link
Member

@y-yagi y-yagi commented Feb 25, 2017

SystemTestCase supports only Puma, and always load puma's file.
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/system_testing/server.rb#L1

For that reason, the case of use Capybara but do not use Puma, it will cause an error.
So we need to check about Puma is defined as well.

railties/lib/rails/test_help.rb Outdated
@@ -11,7 +11,7 @@

require "active_support/testing/autorun"

if defined?(Capbyara)
if defined?(Capbyara) && defined?(Puma)

This comment has been minimized.

@robin850

robin850 Feb 26, 2017
Member

It looks like there is a typo in the constant name ; this will never be true. And it looks like it has been replicated in the second change in your pull request.

This comment has been minimized.

@y-yagi

y-yagi Feb 26, 2017
Author Member

Oops. You are correct. I fixed. Thanks!

SystemTestCase supports only Puma, and always load puma's file.
https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/system_testing/server.rb#L1

For that reason, the case of use Capybara but do not use Puma, it will cause an error.
So we need to check about Puma is defined as well.
@y-yagi y-yagi force-pushed the y-yagi:only_load_systemtestcase_if_puma_is_defined branch to a070dfa Feb 26, 2017
@eileencodes eileencodes merged commit cbbbbcc into rails:master Feb 27, 2017
2 checks passed
2 checks passed
@rails-bot
codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@eileencodes
Copy link
Member

@eileencodes eileencodes commented Feb 27, 2017

Oops, my bad on the spelling mistake 😳

@y-yagi y-yagi deleted the y-yagi:only_load_systemtestcase_if_puma_is_defined branch Feb 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants