-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
try creating a system test and running in Travis #4888
Conversation
Selenium::WebDriver::Error::WebDriverError: Selenium::WebDriver::Error::WebDriverError: Unable to find chromedriver. Please download the server from http://chromedriver.storage.googleapis.com/index.html and place it somewhere on your PATH. More info at https://github.com/SeleniumHQ/selenium/wiki/ChromeDriver. |
Hmm, @siaw23 have you ever done this? I think this could dramatically stabilize our testing of front-end features! |
fcdfd41
to
150cdc4
Compare
I took a shot at this. System tests have been fixed but only locally(on development). I think we can't run system tests on Travis if we're using Passenger because Passenger runs multiple process which won't work with Capybara. So we can either change from Puma to Passenger on production (which I recommend) or run system tests only on development with Puma 😬... |
Well, system tests should run in test mode, not development mode, right? Sorry, I'm new to this! Right now tests don't run on passenger at all, they are lower level. And we don't run them in production mode at all. So don't we not lose anything in running system tests on Puma? Thanks!!! |
@publiclab/plots2-reviewers anyone want to take a crack at getting these to run in Travis? The above posts seem to show it's possible. |
Oh I'll have to check this again! I left this like this thinking the usual tests (non system tests) are running. |
OK so I had to re-read what you wrote:
They do.
We run the normal tests like usual. The only test we don't run now are system tests and that's because of Passenger. So in this PR we're running normal tests + system tests in development and test modes. Then on production, we're just running the normal tests. Though ideally we'd like to run system tests on production too.
I'm not sure what this means but I don't see how we'd lose anything on running system tests on puma, it's only to test UI right? |
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.
Hmm, maybe I am misunderstanding as well; it's my understanding that prior to system tests in Rails, all tests run in "test" mode, not development or production. And that this is true in Travis as well.
.travis.yml
Outdated
- docker-compose exec web bash -c "CI=TRUE GENERATE_REPORT=true rails test:system" | ||
# Capybara can't work with Passenger on Travis, hence disabling system test on | ||
# production until we change over to a Rack server. | ||
# - docker-compose exec web bash -c "CI=TRUE GENERATE_REPORT=true rails test:system" |
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.
So I think we really need to re-enable this line, and to follow the guidance in the links I provided to try to get system tests to run in Travis.
@@ -93,7 +93,7 @@ For information on how to install for use with the cloud environment, please see | |||
9. By default, start rails with `passenger start` from the Rails root and open http://localhost:3000 in a web browser. | |||
(for local SSL work, see [SSL](#ssl+in+development) below) | |||
10. Wheeeee! You're up and running! Log in with test usernames "user", "moderator", or "admin", and password "password". | |||
11. Run `rails test -d` to confirm that your install is working properly. | |||
11. Run `rails test` to confirm that your install is working properly. Or `rails test:system` for system tests. |
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.
what was the -d
originally?
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.
-d
= Output test failures and errors after the test run
Anyways, my biggest interest in this PR is to get system tests running in Travis, so we can use them as part of our CI. Where would we set it to use Puma? https://dev.to/aergonaut/running-rails-5-system-tests-on-travis-ci-with-chromedriver-4nm7 seems to show it working in Travis. Actually the errors I had seen were about not being able to find ChromeDriver, not related to Capybara. But maybe you got past those errors and saw a Capybara/Passenger error? If so, could you point me at the commit where you saw that? |
Yes you can @sashadev-sky |
Yes, i think that's right - and in Travis it has to be using the configured chromedriver and chrome from either My guess is that if there's a way we can configure Travis to run them either as they are or with Puma (if that's the missing part) then that would be best. This will hopefully be really great for JS testing. The hidden treat is also that it may be possible to get it to generate a set of screenshots of different pages automatically 💥 |
@jywarren I have seen those posts you referred to. I know system tests are possible on Travis. What the articles you referred to don't mention is what server they are using. They only showed how it works on Travis without making mention of which server they are using. With Thin, Webrick, Puma that article is relevant, with Passenger (which is not a Rack server...I think, it doesn't work). |
Hi @siaw23 -- so sorry, i know you know! Sincere apologies, didn't mean to run through the same info for you, just trying to catch Sasha up a bit. Thank you, that's an excellent point on their omission of the server in those posts. What your observation makes me wonder is whether we can override the Passenger server config with a Rack server solely in the Travis context, for the purposes of the Travis CI. Do you think that'd be a reasonable way to achieve system tests in Travis, or do you think we should consider another path? Thank you! ❤️ |
@jywarren yes I have been playing with the screenshots, it's really great. teaspoon mocha sinon with rails fixtures had my head spinning so i'm excited about getting this to work. https://www.rubydoc.info/github/jnicklas/capybara#The_DSL this page is the capybara docs and it has every possible way you can configure it and pros and cons, etc. Like for ex., I got it to run with So instead I tried So yeah I agree out of this list of options there could definitely be a solution that doesn't involve infrastructure changes but we would need to dig a bit. |
I have no experience with running system tests + Passenger + Travis. I saw a comment on Passenger or Travis' repo once stating how this may not be possible. Couldn't find it unfortunately when I tried to search for it. If we used something like Puma we could avoid a lot of headaches for contributors. So for this particular issue and with my commits, we can run system tests locally until we switch to a Rack server to have them run on Travis. It'll be ideal to have system test running on CI. I honestly don't know how to do it with Passenger + Travis. |
@siaw23 Yeah sorry me too I didn't see your comment until after I had posted my really long one 🙈I will be so happy the day travis CI makes perfect sense to me |
Ooh, a more descriptive error:
|
test/application_system_test_case.rb
Outdated
require "test_helper" | ||
|
||
class ApplicationSystemTestCase < ActionDispatch::SystemTestCase | ||
caps = Selenium::WebDriver::Remote::Capabilities.chrome("chromeOptions" => {"args" => %w(--headless)}) |
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.
Maybe we need to set chrome flags here instead of in .travis.yml?
uhhhhhhhh....... |
🎉 🎉 🎉 ✔️
|
Now I guess we should see what we can remove and still let this run... to simplify! |
Anything else we can remove? |
.travis.yml
Outdated
- docker-compose exec web bash -c "wget https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb" | ||
- docker-compose exec web bash -c "dpkg -i google-chrome-stable_current_amd64.deb; apt-get -fy install" | ||
- docker-compose exec web bash -c "nohup Xvfb -ac :9 -screen 0 1280x1024x16 &" | ||
- docker-compose exec web bash -c "export DISPLAY=:99.0" |
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.
Let's try removing these and also add links for where to update dependencies
Gemfile
Outdated
@@ -36,7 +36,6 @@ gem 'omniauth-github', '~> 1.1', '>= 1.1.2' | |||
gem 'omniauth-google-oauth2' | |||
gem 'omniauth-twitter' | |||
gem "paperclip", "~> 6.1.0" | |||
gem 'passenger' |
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.
Oh let's try re adding this?
OK this is pretty awesome. I've refined it down to what I believe is the minimum changes needed and cleaned up comments. This should be ready for a merge and then we can start building:
|
This is ready to merge if it passes tests. Opening next steps here: #5316 |
🎉 🎉 🎉 |
* try creating a system test * Create application_system_test_case.rb * Update .travis.yml * Update Gemfile * Update Gemfile * Update .travis.yml * Update application_system_test_case.rb * Update .travis.yml * Update .travis.yml * Update .travis.yml * Fix system test * Disable system test on production for now * Comment system test related configs from .travis.yml * Update Gemfile * Update Gemfile.lock * Update application_system_test_case.rb * Update .travis.yml * update gemfile.lock * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update application_system_test_case.rb * apt-get install google-chrome-stable chromium-chromedriver * wget/dpkg install of chromium and chromedriver * Update .travis.yml * Update .travis.yml * gemfile.lock update * Gemfile.lock update * change curl to wget * Chromedriver 2.35 and removed finds missing * Turn off binary option * Add command to start Chrome * update selenium-webdriver to 3.141.0 * --no-sandbox * export DISPLAY=:99.0 * Xvfb : nohup Xvfb -ac :9 -screen 0 1280x1024x16 & * Remove prefix * Chromedriver 73.0.3683.68 * Add chrome flags to ruby initializer * comment out first start of chrome, remove extraneous comments * commenting out `xvfb` and `DISPLAY` * move passenger to general gems; move back if it fails * cleaning up commented lines * Update Gemfile * Update Gemfile * Update Gemfile * Update application_system_test_case.rb
* try creating a system test * Create application_system_test_case.rb * Update .travis.yml * Update Gemfile * Update Gemfile * Update .travis.yml * Update application_system_test_case.rb * Update .travis.yml * Update .travis.yml * Update .travis.yml * Fix system test * Disable system test on production for now * Comment system test related configs from .travis.yml * Update Gemfile * Update Gemfile.lock * Update application_system_test_case.rb * Update .travis.yml * update gemfile.lock * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update .travis.yml * Update application_system_test_case.rb * apt-get install google-chrome-stable chromium-chromedriver * wget/dpkg install of chromium and chromedriver * Update .travis.yml * Update .travis.yml * gemfile.lock update * Gemfile.lock update * change curl to wget * Chromedriver 2.35 and removed finds missing * Turn off binary option * Add command to start Chrome * update selenium-webdriver to 3.141.0 * --no-sandbox * export DISPLAY=:99.0 * Xvfb : nohup Xvfb -ac :9 -screen 0 1280x1024x16 & * Remove prefix * Chromedriver 73.0.3683.68 * Add chrome flags to ruby initializer * comment out first start of chrome, remove extraneous comments * commenting out `xvfb` and `DISPLAY` * move passenger to general gems; move back if it fails * cleaning up commented lines * Update Gemfile * Update Gemfile * Update Gemfile * Update application_system_test_case.rb
Closes #3683
https://guides.rubyonrails.org/testing.html#implementing-a-system-test
https://chriskottom.com/blog/2017/04/full-stack-testing-with-rails-system-tests/