Closed anandaroop closed 7 years ago
jonallured
commented
7 years ago Can you remind me - will we need more than Capybara in order to add this level of testing? We will, right? Because we need something that will execute JS. So, does that mean we'll want to use Selenium or Poltergeist or something? It's been too long since I had to set this stuff up...
anandaroop
commented
7 years ago Yeah, we've generally used Capybara + Selenium. Some projects e.g. Helix fire up a real web browser, but I think we'd want to avoid that as much as possible, maybe by starting with a headless driver like Poltergeist/PhantomJS?
jonallured
commented
7 years ago Now that #93 has landed and we're on the latest and greatest Rails, I was going to see what might make sense to move this issue forward. I took another look at the Rails guides and what they have to say about system tests.
http://guides.rubyonrails.org/testing.html#system-testing
The advice there is to use test/system directory to write these tests and Rails now bestows upon us the capybara gem.
Buuut, since we're an Rspec shop, we use rspec-rails and the guidance there is to use Feature Specs:
https://github.com/rspec/rspec-rails#feature-specs
As far as I can tell, these are the same things - use capybara to drive a browser and test all levels of your application. System tests and Feature specs are the same thing, I think.
So if that's the case, then I don't really think we need to do anything other than start writing Feature specs in the spec/features folder and decide on what type of driver to use.
Does this sound right?
anandaroop
commented
7 years ago Yeah — both approaches use Capybara to drive a browser through end to end tests, using Capybara's DSL for tests that are expressive of the UI.
With the System Test approach we get a few niceties preconfigured — a Chrome driver, screenshots for failed tests, a shared transactional view of the db between the server process and the test process, and… well I'm not sure what else, come to think of it.
So possible directions I see now:
go the Rspec spec/features route, and end up configuring this stuff from scratch
go the Rails test/system route, which I think just means uncommenting this so that we can take advantage of the above defaults, and do things like:
bin/rails generate system_test user_does_awesome
bin/rake test:system
EDIT: I think you'd also have to create a test/test_helper.rb file, which in my experiment I copied from a dummy app created with rails _5.1.1_ new dummy_app
keep an eye out for Rspec's spec/system folder which will pick up the Rails system test configuration (I think)
Whichever approach we go with, the immediate bigger hurdle will probably be configuring all the boilerplate for Rack-level session stuff and mocking the upstream API responses. So maybe not worth it to fret too much about which approach above is best. I'd vote for system tests, but feel free to experiment and make a call.
jonallured
commented
7 years ago Turns out the hardest part of this is actually at the Javascript level. I've spent quite a bit of time trying to get acceptance testing to work for Rosalind, but I have been defeated. Sometimes these computers just don't want to be programmed. I've got my work on a branch on my fork, but it boils down to this:
// go to https://rosalind.artsy.net/
// open the developer console, then copy/paste these lines:
document.querySelector('.GenericAutosuggest > div > input').dispatchEvent(new Event('focus', { bubbles: true }))
document.querySelector('.GenericAutosuggest > div > input').value = 'Pho'
document.querySelector('.GenericAutosuggest > div > input').dispatchEvent(new Event('input', { bubbles: true }))What you should see is that the suggestions for genes show up - we have programmatically triggered the Autosuggest React component. :tada:
That should mean that we can use Capybara's execute_script to cause this same behavior in our test, right? HAHAHAHAHAHA no. That is the maniacal laughter of a developer that's lost his mind trying to make something work.
Instead, it seems that the triggering of these events has no affect on the page while in the context of a feature spec, here's the relevant part of the spec I wrote:
execute_script("document.querySelector('.GenericAutosuggest > div > input').dispatchEvent(new Event('focus', { bubbles: true }))")
execute_script("document.querySelector('.GenericAutosuggest > div > input').value = 'Pho'")
execute_script("document.querySelector('.GenericAutosuggest > div > input').dispatchEvent(new Event('input', { bubbles: true }))")
`open #{save_screenshot}`That produces this screenshot:
Notice the absence of the suggestions? I sure do! But also notice that 'Pho' has been entered into the input. This means that execute_script is working as intended and that the querySelector method is finding the element on the page. So this is where I'm at - I can get that plugin to behave in Chrome's console, but not in the context of an acceptance test.
A couple more things to note before I close.
At some point in my quest to understand what was happening, I became suspicious of the calls made to the match endpoint, so I wrote another feature spec that directly hits that endpoint. What I figured out was that I could use Kinetic's built in support for mocking to avoid hitting Gravity:
Kinetic::Stub::Gravity::GravityModel.match([gene], { params: { term: 'Pho' } }, 200, Gene)This test is green and proves that if the Autosuggest plugin was behaving and hitting this endpoint, then it would return the right Javascript. Even still, as another sanity check, I sometimes edit the MatchController and throw a raise :hell in there, but it's never triggered because that request isn't made.
I'll also point out that instead of having to deal with authentication at all, I simply mocked out those things at the controller level:
allow_any_instance_of(ApplicationController).to receive(:find_current_user)
allow_any_instance_of(ApplicationController).to receive(:require_admin_or_genomer)This avoids the code used to secure the application.
Both of these choices could be seen as cheating since what I'm writing are acceptance tests, but I think you have to weigh the trade offs and my instinct is that it's worth it to skip Kinetic hitting Gravity and authentication logic. Open to other thoughts here, but it hardly seems to matter since I can't get the suggestions to work. :disappointed:
Finally, big thanks to @gnilekaw for working with me on this. Among other things, he figured out that { bubbles: true } was required in order to make Autosuggest work in the console. :heart: :heart: :heart:
anandaroop
commented
7 years ago Sometimes these computers just don't want to be programmed
LOL, true that.
I checked out your branch and I tried to build on it in my own branch. In short:
That's about all I have time for at the moment, but feel free to keep investigating or maybe pair next week!
anandaroop
commented
7 years ago And Helix might contain some hints about how to proceed from here.
Thanks for getting us this far @jonallured !
jonallured
commented
7 years ago Whoa, nice! I'm trying to reproduce your results on my machine here, but I'm getting this when I try to run the tests:
Selenium::WebDriver::Error::WebDriverError:
Unable to find Mozilla geckodriver. Please download the server from https://github.com/mozilla/geckodriver/releases and place it somewhere on your PATH. More info at https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette/WebDriver.Did you have to download that server and mess with your PATH?
anandaroop
commented
7 years ago Hmm, I did not. It seems like your setup is looking for a Firefox flavored browser instead of Chrome. If you look at the commit history that I linked to my branch above, you'll see the (only) steps that I took to get Chrome (technically Chromium) configured to work for js tests.
jonallured
commented
7 years ago Oops, you're totally right - once I add that config it works!!
So where does this leave us? Are headful acceptance tests where we want to go? Might be our only option. I wonder if there will be any issues running them on Circle.
anandaroop
commented
7 years ago Yeah, good q — I was hoping to avoid headful tests, for reasons of speed and environment fiddling.
But I'm not experienced with headless browser tests, so I'm not too sure how feasible it will be for us to go that route.
We could WIP this over to Circle and see what happens, and what it would take for these Chromium tests to go headless. Worth a shot, right?
starsirius
commented
7 years ago This is so exciting! 👍 Also want to mention capybara-chromium was meant to be a replacement for capybara-webkit. It has no activity for a while, but worth keeping an eye on it.
anandaroop
commented
7 years ago Good to know, thanks!
jonallured
commented
7 years ago @anandaroop how do you feel about this issue? Do you feel like the current feature specs satisfy the intent of this one? Wondering if it makes sense to close this sucker.
anandaroop
commented
7 years ago I've got some WIP for testing the update functionality (currently only the querying functionality is exercised by the feature specs).
Let's leave this open so that it will nag at my conscience a little longer 😄, but thanks for the reminder — I need to finish that off!
anandaroop
commented
7 years ago Thanks for bumping this last week @jonallured — I think we can close now that #123 is merged.
Currently Rosalind has no feature specs / acceptance tests.
I had been hedging my bets, hoping that the release version of Rails 5.1 would land soon enough that we could use the new, officially blessed approach to acceptance tests that in Rails lingo are called System Tests.
To that end, I briefly attempted a 5.1 upgrade after it was released, but got blocked by the following:
There is an active discussion about this, but no resolution yet.
There is a potential workaround that involves depending on another developer's fork SimpleForm, which relaxes those requirements, but I'm not sure how I feel about modifying Kinetic for a short-term fix like that.
We should decide soon, whether to:
Any other ideas? cc: @starsirius @jonallured @gnilekaw