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

Make `driven_by` overridable #28586

Merged
merged 1 commit into from Mar 29, 2017

Conversation

Projects
None yet
5 participants
@mtsmfm
Contributor

mtsmfm commented Mar 28, 2017

Sometimes we want to use rack_test partially instead of selenium for test speed:

class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
  driven_by :selenium, using: :chrome, screen_size: [1400, 1400], options: {url: "http://chrome:4444/wd/hub"}
end

class WithJavascriptTest < ApplicationSystemTestCase
end

class WithoutJavascriptTest < ApplicationSystemTestCase
  driven_by :rack_test
end

In the abobe case, WithoutJavascriptTest uses selenium because
SystemTestCase calls superclass' driver on #initialize (self.class.superclass.driver.use).

Using class_attribute can handle inherited driven_by.

@rails-bot

This comment has been minimized.

rails-bot commented Mar 28, 2017

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

@rafaelfranca rafaelfranca added this to the 5.1.0 milestone Mar 28, 2017

actionpack/test/dispatch/system_testing/system_test_case_test.rb Outdated
@@ -6,6 +6,14 @@ class SetDriverToRackTestTest < DrivenByRackTest
end
end
class OverrideDriverToRackTestTest < DrivenBySeleniumWithChrome

This comment has been minimized.

@eileencodes

eileencodes Mar 28, 2017

Member

I was confused why this test was necessary when the test before works with rack_test. Can you rename it to OverrideSeleniumSubclassToRackTestTest so it's clearer?

@mtsmfm mtsmfm force-pushed the mtsmfm:override-driver branch Mar 28, 2017

Make `driven_by` overridable
Sometimes we want to use rack_test partially instead of selenium for test speed:

```ruby
class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
  driven_by :selenium, using: :chrome, screen_size: [1400, 1400], options: {url: "http://chrome:4444/wd/hub"}
end

class WithJavaScriptTest < ApplicationSystemTestCase
end

class WithoutJavaScriptTest < ApplicationSystemTestCase
  driven_by :rack_test
end
```

In the abobe case, `WithoutJavaScriptTest` uses selenium because
`SystemTestCase` calls superclass' driver on `#initialize` (`self.class.superclass.driver.use`).

Using `class_attribute` can handle inherited `driven_by`.

@mtsmfm mtsmfm force-pushed the mtsmfm:override-driver branch to 24e0fa7 Mar 29, 2017

@mtsmfm

This comment has been minimized.

Contributor

mtsmfm commented Mar 29, 2017

@eileencodes I renamed the test class!

@eileencodes eileencodes merged commit 56f3af4 into rails:master Mar 29, 2017

2 checks passed

codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

eileencodes added a commit that referenced this pull request Mar 29, 2017

Merge pull request #28586 from mtsmfm/override-driver
Make `driven_by` overridable
@eileencodes

This comment has been minimized.

Member

eileencodes commented Mar 29, 2017

Backported to 5-1-stable in 41412ef

@mtsmfm mtsmfm deleted the mtsmfm:override-driver branch Mar 29, 2017

@maclover7 maclover7 removed the needs work label Mar 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment