Upgrade from 'feature' to 'system' test #3568

gonzar11 · 2020-03-25T20:12:20Z

Description
I couldn't remove database cleaner because some specs on backends started to fail. Then I realized that it used just to truncate the database before starting run all specs. This is because some migrations are filling the database.

It closes #3564

Checklist:

I have followed Pull Request guidelines
I have added a detailed description into each commit message
I have updated Guides and README accordingly to this change (if needed)
I have added tests to cover this change (if needed)
I have attached screenshots to this PR for visual changes (if needed)

houndci-bot · 2020-03-25T20:12:56Z

frontend/spec/spec_helper.rb

@@ -94,3 +102,5 @@

  Kernel.srand config.seed
 end
+


Layout/TrailingBlankLines: 2 trailing blank lines detected.

houndci-bot · 2020-03-25T20:12:56Z

frontend/spec/spec_helper.rb

+  config.before(:each, type: :system) do
+    driven_by :rack_test
+  end
+


Layout/TrailingWhitespace: Trailing whitespace detected.

houndci-bot · 2020-03-25T20:12:56Z

backend/spec/spec_helper.rb

 ActionView::Base.raise_on_missing_translations = true

 Capybara.default_max_wait_time = ENV['DEFAULT_MAX_WAIT_TIME'].to_f if ENV['DEFAULT_MAX_WAIT_TIME'].present?

 ActiveJob::Base.queue_adapter = :test

+


Layout/EmptyLines: Extra blank line detected.

houndci-bot · 2020-03-25T20:24:47Z

frontend/spec/spec_helper.rb

@@ -93,4 +101,4 @@
  config.order = :random

  Kernel.srand config.seed
-end
+end


Layout/TrailingBlankLines: Final newline missing.

coorasse · 2020-03-26T20:42:33Z

backend/spec/spec_helper.rb

@@ -44,6 +44,7 @@
 Capybara.exact = true

 require "selenium/webdriver"
+Capybara.javascript_driver = (ENV['CAPYBARA_DRIVER'] || :selenium_chrome_headless).to_sym


why is this line necessary? isn't

config.before(:each, type: :system, js: true) do driven_by :selenium_chrome_headless end

already taking care of defining the driver?

Rails uses as default :selenium . If I don't define Capybara.javascript_driver, rails will assume that we use :selenium. In my case it fails because I didn't have geckodriver( Mozilla driver used with selenium).
Then with config.before(:each, type: :system, js: true) we redefine capybara Capybara javascript driver.

That's odd. I checked the branch locally and tried commenting this line and all the driven_by below and the result is the same. I think that for some reason it's just using the default ignoring the driven_by calls.

I've tried to print which driver is being used. Please add this line puts Capybara.current_driver in both config.before(:each, type: :system) and config.before(:each, type: :system, js: true) before and after using driven_by. You will see the difference between use and don's use driven_by

Ok thanks, I'm giving it a try

if I comment out

# Capybara.javascript_driver = (ENV['CAPYBARA_DRIVER'] || :selenium_chrome_headless).to_sym

no matter what Capybara.current_driver prints, it is opening Firefox to run specs so it's definitely not headless.
I think there's some misconfiguration happening but I couldn't get exactly where yet.

I've also tried with this syntax, which seems to be the default to make exactly the same thing that we do with our custom driver:

driven_by :selenium, using: :headless_chrome, screen_size: [1920, 1080]

but it still opens firefox to run specs. 😢

@gonzar11 did you get the chance to take a look?

@gonzar11, I worked on a similar PR in a Solidus extension (solidus_starter_frontend) and I would like to add my thoughts.

Capybara.javascript_driver = ... is not necessary; Rails testing guide (but for Minitest) says that default Capybara driver settings should be changed via driven_by;

in my PR I choose to have 2 different variables (CAPYBARA_DRIVER/CAPYBARA_JS_DRIVER), it can be useful sometimes;

I made some tests in this PR commenting out the Capybara.javascript_driver = ... line in spec_helper.rb; running a JS spec with CAPYBARA_DRIVER=selenium_chrome bin/rspec backend/spec/system/admin/configuration/payment_methods_spec.rb:36 first opens a Firefox empty window then a Chrome window with the specified test.
As a test, if I comment out the following lines, I see that it works as expected:

if RSpec.current_example.metadata[:js] && page.driver.browser.respond_to?(:url_blacklist) page.driver.browser.url_blacklist = ['http://fonts.googleapis.com'] end

houndci-bot · 2020-03-30T13:31:19Z

frontend/spec/spec_helper.rb

+  end
+
+  config.before(:each, type: :system, js: true) do
+    driven_by (ENV['CAPYBARA_DRIVER'] || :selenium_chrome_headless).to_sym


Lint/ParenthesesAsGroupedExpression: (...) interpreted as grouped expression.

houndci-bot · 2020-03-30T13:31:19Z

backend/spec/spec_helper.rb

+  end
+
+  config.before(:each, type: :system, js: true) do
+    driven_by (ENV['CAPYBARA_DRIVER'] || :selenium_chrome_headless).to_sym


Lint/ParenthesesAsGroupedExpression: (...) interpreted as grouped expression.

blocknotes · 2020-05-25T14:26:05Z

backend/spec/spec_helper.rb

@@ -44,6 +44,7 @@
 Capybara.exact = true

 require "selenium/webdriver"
+Capybara.javascript_driver = (ENV['CAPYBARA_DRIVER'] || :selenium_chrome_headless).to_sym


@gonzar11, I worked on a similar PR in a Solidus extension (solidus_starter_frontend) and I would like to add my thoughts.

Capybara.javascript_driver = ... is not necessary; Rails testing guide (but for Minitest) says that default Capybara driver settings should be changed via driven_by;

in my PR I choose to have 2 different variables (CAPYBARA_DRIVER/CAPYBARA_JS_DRIVER), it can be useful sometimes;

I made some tests in this PR commenting out the Capybara.javascript_driver = ... line in spec_helper.rb; running a JS spec with CAPYBARA_DRIVER=selenium_chrome bin/rspec backend/spec/system/admin/configuration/payment_methods_spec.rb:36 first opens a Firefox empty window then a Chrome window with the specified test.
As a test, if I comment out the following lines, I see that it works as expected:

if RSpec.current_example.metadata[:js] && page.driver.browser.respond_to?(:url_blacklist) page.driver.browser.url_blacklist = ['http://fonts.googleapis.com'] end

houndci-bot reviewed Mar 25, 2020

View changes

gonzar11 force-pushed the gonzar11:gonzar11/switch-to-system-test branch 2 times, most recently from 02af214 to 3368775 Mar 25, 2020

houndci-bot reviewed Mar 25, 2020

View changes

frontend/spec/spec_helper.rb Outdated

@@ -93,4 +101,4 @@

config.order = :random

Kernel.srand config.seed

end

end

This comment has been minimized.

Sign in to view

houndci-bot Mar 25, 2020

Layout/TrailingBlankLines: Final newline missing.

gonzar11 force-pushed the gonzar11:gonzar11/switch-to-system-test branch from 3368775 to 6f31382 Mar 25, 2020

coorasse approved these changes Mar 26, 2020

View changes

Upgrade from 'feature' to 'system' test

Loading status checks…

004b9c7

gonzar11 force-pushed the gonzar11:gonzar11/switch-to-system-test branch from 6f31382 to 004b9c7 Mar 30, 2020

houndci-bot reviewed Mar 30, 2020

View changes

kennyadsl added Needs Core Team Review Needs Work and removed Needs Core Team Review labels Apr 1, 2020

blocknotes suggested changes May 25, 2020

View changes

solidusio / solidus

Upgrade from 'feature' to 'system' test #3568

Upgrade from 'feature' to 'system' test #3568

gonzar11 commented Mar 25, 2020 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

solidusio / solidus

Sponsor solidusio/solidus

Join GitHub today

GitHub is where the world builds software

Upgrade from 'feature' to 'system' test #3568

Upgrade from 'feature' to 'system' test #3568

Conversation

gonzar11 commented Mar 25, 2020 • edited

This comment has been minimized.

houndci-bot Mar 25, 2020

This comment has been minimized.

houndci-bot Mar 25, 2020

This comment has been minimized.

houndci-bot Mar 25, 2020

This comment has been minimized.

houndci-bot Mar 25, 2020

This comment has been minimized.

coorasse Mar 26, 2020 Contributor

This comment has been minimized.

gonzar11 Mar 30, 2020 • edited Author

This comment has been minimized.

kennyadsl Apr 3, 2020 • edited Member

This comment has been minimized.

gonzar11 Apr 8, 2020 Author

This comment has been minimized.

kennyadsl Apr 20, 2020 Member

This comment has been minimized.

kennyadsl Apr 20, 2020 Member

This comment has been minimized.

kennyadsl May 1, 2020 Member

This comment has been minimized.

blocknotes May 25, 2020 • edited Contributor

This comment has been minimized.

houndci-bot Mar 30, 2020

This comment has been minimized.

houndci-bot Mar 30, 2020

This comment has been minimized.

blocknotes May 25, 2020 • edited Contributor

Essential cookies

Always active

Analytics cookies

gonzar11 commented Mar 25, 2020 •

edited

coorasse Mar 26, 2020
Contributor

gonzar11 Mar 30, 2020 •

edited

Author

kennyadsl Apr 3, 2020 •

edited

Member

gonzar11 Apr 8, 2020
Author

kennyadsl Apr 20, 2020
Member

kennyadsl Apr 20, 2020
Member

kennyadsl May 1, 2020
Member

blocknotes May 25, 2020 •

edited

Contributor

blocknotes May 25, 2020 •

edited

Contributor