Skip to content
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

Use electron in place of phantomjs #4409

Closed
alexose opened this issue Dec 15, 2016 · 11 comments
Closed

Use electron in place of phantomjs #4409

alexose opened this issue Dec 15, 2016 · 11 comments
Assignees

Comments

@alexose
Copy link
Contributor

alexose commented Dec 15, 2016

Not only is PhantomJS not being super actively maintained, it is also notably slower than Electron.

It should be pretty straightforward to swap PhantomJS with Electron without having to change the tests much. However, due to subtle differences, we will have to repair some failing tests.

@alexose
Copy link
Contributor Author

alexose commented Dec 15, 2016

New implementation is up and being tested on the e2e-electron branch.

@alexose
Copy link
Contributor Author

alexose commented Dec 15, 2016

I've noticed two things that cause Electron to fail in places where Phantom didn't:

  1. Click events require that the clicked element isn't overlapped by any other element. This is good to fix anyway.

  2. Our smooth scrolling plugin causes some timing issues. I think this is because Electron is so much quicker to load pages. Anyway, I've implemented a way to override smooth scrolling for our E2E tests.

@alexose
Copy link
Contributor Author

alexose commented Dec 15, 2016

All tests are passing locally but continue to have issues in the Travis environment. The good news is that it's at least 25% faster with a lot more room for improvement.

@alexose
Copy link
Contributor Author

alexose commented Dec 16, 2016

Travis appears to be tripping over selectors for certain form elements. This isn't happening locally. I'm going to try to debug this in docker.

@alexose
Copy link
Contributor Author

alexose commented Dec 19, 2016

Successfully replicated it in docker. Definitely looks like some extra complexity introduced by xvfb.

Edit: Yep. The select go off the screen, meaning that they can't actually be clicked. Gonna fix this by verifying that the correct option element exists, then just setting the value with .setValue

@alexose
Copy link
Contributor Author

alexose commented Dec 21, 2016

Turns out that using .setValue doesn't trigger the change event, and it's not really possible to trigger change programmatically in React.

So, I wrote a snippet that uses the arrow keys instead of the mouse. This works fine in xvfb, but not in OSX. 🤦

There are still a couple of other issues I need to tackle, most importantly figuring out what is causing HCA not to validate: https://travis-ci.org/alexose/vets-website/jobs/185638483#L2492-L2524

@alexose
Copy link
Contributor Author

alexose commented Dec 28, 2016

Only one set of problems left at this point, which is the edit screen test in est/hca/06-edit-submission.e2e.spec.js. It's almost certainly a timing issue.

https://travis-ci.org/alexose/vets-website/jobs/187298791

@alexose
Copy link
Contributor Author

alexose commented Jan 5, 2017

Almost there, but I'm still seeing an issue with the accessibility tests. There's also some new work to be done now that #4481 has been merged.

@jcmeloni-zz
Copy link

Not moving over to vets.gov-team as this is captured in master ticket over there and will close it when that closes https://github.com/department-of-veterans-affairs/vets.gov-team/issues/578

@alexose-gov alexose-gov assigned alexose-gov and unassigned alexose Feb 1, 2017
@alexose-gov
Copy link
Contributor

Done!

@jcmeloni-usds
Copy link
Contributor

Yaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay!!!!! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants