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.

Sign up for GitHub

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

Jump to bottom

Permit running jobs in system tests #36283

Merged
merged 1 commit into from
May 16, 2019
Merged

Permit running jobs in system tests #36283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions actionpack/lib/action_dispatch/railtie.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,11 @@ class Railtie < Rails::Railtie # :nodoc:

ActionDispatch.test_app = app
end

initializer "action_dispatch.system_tests" do |app|
Copy link
Member

Choose a reason for hiding this comment

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

👋Hello!

By including the app.routes.url_helpers inside the test itself, if you have a route that starts with test_, minitest will try to call it as it thinks it's a test https://github.com/seattlerb/minitest/blob/ab39d35fb4e84eb866ed9c4ecb707cbf3889de42/lib/minitest/test.rb#L66

It can be surprising to user and can break if the route expect parameters

Copy link
Member

Choose a reason for hiding this comment

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

Agree. We should include this routes in a object and use method_missing to delegate to them like we do in the integration tests. @Edouard-chin can you open a PR?

ActiveSupport.on_load(:action_dispatch_system_test_case) do
include app.routes.url_helpers
end
end
end
end
8 changes: 5 additions & 3 deletions actionpack/lib/action_dispatch/system_test_case.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
require "action_dispatch/system_testing/server"
require "action_dispatch/system_testing/test_helpers/screenshot_helper"
require "action_dispatch/system_testing/test_helpers/setup_and_teardown"
require "action_dispatch/system_testing/test_helpers/undef_methods"

module ActionDispatch
# = System Testing
Expand Down Expand Up @@ -110,12 +109,11 @@ module ActionDispatch
# Because <tt>ActionDispatch::SystemTestCase</tt> is a shim between Capybara
# and Rails, any driver that is supported by Capybara is supported by system
# tests as long as you include the required gems and files.
class SystemTestCase < IntegrationTest
class SystemTestCase < ActiveSupport::TestCase
include Capybara::DSL
include Capybara::Minitest::Assertions
include SystemTesting::TestHelpers::SetupAndTeardown
include SystemTesting::TestHelpers::ScreenshotHelper
include SystemTesting::TestHelpers::UndefMethods

def initialize(*) # :nodoc:
super
Expand Down Expand Up @@ -160,6 +158,10 @@ def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400], options: {

driven_by :selenium

def url_options # :nodoc:
default_url_options.merge(host: Capybara.app_host)
end

ActiveSupport.run_load_hooks(:action_dispatch_system_test_case, self)
end

Expand Down
1 change: 0 additions & 1 deletion actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ module SetupAndTeardown # :nodoc:
DEFAULT_HOST = "http://127.0.0.1"

def host!(host)
super
Capybara.app_host = host
end

Expand Down
26 changes: 0 additions & 26 deletions actionpack/lib/action_dispatch/system_testing/test_helpers/undef_methods.rb
View file
Open in desktop

This file was deleted.

37 changes: 0 additions & 37 deletions actionpack/test/dispatch/system_testing/system_test_case_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,40 +46,3 @@ class SetHostTest < DrivenByRackTest
assert_equal "http://example.com", Capybara.app_host
end
end

class UndefMethodsTest < DrivenBySeleniumWithChrome
test "get" do
exception = assert_raise NoMethodError do
get "http://example.com"
end
assert_equal "System tests cannot make direct requests via #get; use #visit and #click_on instead. See http://www.rubydoc.info/github/teamcapybara/capybara/master#The_DSL for more information.", exception.message
end

test "post" do
exception = assert_raise NoMethodError do
post "http://example.com"
end
assert_equal "System tests cannot make direct requests via #post; use #visit and #click_on instead. See http://www.rubydoc.info/github/teamcapybara/capybara/master#The_DSL for more information.", exception.message
end

test "put" do
exception = assert_raise NoMethodError do
put "http://example.com"
end
assert_equal "System tests cannot make direct requests via #put; use #visit and #click_on instead. See http://www.rubydoc.info/github/teamcapybara/capybara/master#The_DSL for more information.", exception.message
end

test "patch" do
exception = assert_raise NoMethodError do
patch "http://example.com"
end
assert_equal "System tests cannot make direct requests via #patch; use #visit and #click_on instead. See http://www.rubydoc.info/github/teamcapybara/capybara/master#The_DSL for more information.", exception.message
end

test "delete" do
exception = assert_raise NoMethodError do
delete "http://example.com"
end
assert_equal "System tests cannot make direct requests via #delete; use #visit and #click_on instead. See http://www.rubydoc.info/github/teamcapybara/capybara/master#The_DSL for more information.", exception.message
end
end