-
Notifications
You must be signed in to change notification settings - Fork 7
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
Switch from PhantomJS to headless Chrome #101
base: master
Are you sure you want to change the base?
Switch from PhantomJS to headless Chrome #101
Conversation
Using automatic linting of ruby code in atom throws some warnings that are not necessary in my opinion.
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.
Thanks, @gurix!
config/application.rb
Outdated
@@ -40,6 +40,9 @@ class Application < Rails::Application | |||
|
|||
# Always raise error on unpermitted parameters | |||
# config.action_controller.action_on_unpermitted_parameters = :raise | |||
|
|||
# Automatically convert boolean values to integer using sqlite3 | |||
config.active_record.sqlite3.represent_boolean_as_integer = true if Rails.version >= '5.1.0' && config.active_record.sqlite3.present? |
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 don't use SQLite at all. As far as I remember some tests use to break under SQLite, so I decided to use MySQL also in tests.
So I think we should rather try to remove all SQLite stuff, maybe then the deprecation warning would go away?
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.
Simply the deprecation warning goes away but nothing is stopping someone to use the base project in conjunction with sqlite so it is justified here.
@@ -0,0 +1,10 @@ | |||
AllCops: |
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.
I used Rubocop some time ago, but didn't remember I'm still using it.
It's good to have it, I think. But line length and stuff sometimes seems a pain in the ass, especially in specs.
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.
True but for some blocks as it
, scenario
and so on you can disable the line length. Feel free to change it as soon as it disturbs you (not tested):
LineLength:
Enabled: true
Max: 160
ExcludedMethods:
- describe
- context
- scenario
- it
- feature
Metrics/BlockLength:
ExcludedMethods:
- describe
- context
- scenario
- it
- feature
spec/features/pages/edit_spec.rb
Outdated
@@ -59,8 +59,8 @@ | |||
fill_in 'page_navigation_title', with: 'A new navigation title' | |||
fill_in 'page_content', with: "A new content with a ![existing image](@image-existing-image) and a ![new image](@image-new-image). Also an ![existing code](@code-existing-code) and a ![new code](@code-new-code). " | |||
fill_in 'page_notes', with: 'A new note' | |||
|
|||
find('#page_images_attributes_0_file', visible: false).set base64_other_image[:data] | |||
# As capybara does not finde thethe element having "display: none" in headless chrome we simulate it as js code |
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.
Nice idea.
Some improvements:
- Typos:
finde thethe
- Create a module with this method, e.g.
find_hidden
(seeFocusElement
module for inspiration) so we can reuse it.
Gemfile
Outdated
@@ -96,6 +96,8 @@ group :development, :test do | |||
gem 'rspec' | |||
gem 'rspec-rails' # RSpec for Rails | |||
|
|||
gem 'capybara' # Capybara helps you test web applications by simulating how a real user would interact with your app. |
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.
Interesting. Why does it work locally with capybara-selenium
only, but not on Travis?
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.
I guess you installed capybara before and capybara-selenium
does not depend from capybara
for its own in this case. But yeah it should as it is declared in https://github.com/dsaenztagarro/capybara-selenium/blob/868a5b6329464f0d0bcee5beda5a93c4623182d7/capybara_selenium.gemspec#L21.
Gemfile
Outdated
@@ -96,6 +96,8 @@ group :development, :test do | |||
gem 'rspec' | |||
gem 'rspec-rails' # RSpec for Rails | |||
|
|||
gem 'capybara' # Capybara helps you test web applications by simulating how a real user would interact with your app. | |||
|
|||
gem 'sqlite3' # Use SQLite as the database for Active Record |
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.
Ah, look here! Maybe this is the cause for the deprecation warning?
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.
Ohhh uhhmm I guess so. I'l try to remove it.
@@ -13,8 +13,14 @@ addons: | |||
before_install: | |||
- export TZ=Europe/Zurich | |||
- mysql -e 'CREATE DATABASE base_test' | |||
- google-chrome-stable --headless --disable-gpu --remote-debugging-port=9222 http://localhost & |
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.
Interesting stuff. Could you explain a bit more (or give a link)?
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.
By the way, Chromedriver isn't found on Travis. Any idea how to fix?
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.
Nope I had not the time atm.
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.
https://github.com/flavorjones/chromedriver-helper seems to install it properly. Hint came from http://quyetbui.info/chrome-headless-capybara-on-travis-ci/. But i am not sure if i.e google-chrome-stable --headless --disable-gpu ...
and the gem capybara-selenium
ir really needed in this setting.
At least headless Chromedriver is found somehow. But still a lot is broken 😡. |
Bootstrap 4 uses Flexbox which is not supported by PhantomJS, so some existing specs failed.
This is a good moment to replace PhantomJS with headless Chrome.
Took the chance to also update other gems.