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

Add JavaScript functional test harness #122

Closed
biancadanforth opened this issue Jul 29, 2019 · 8 comments
Closed

Add JavaScript functional test harness #122

biancadanforth opened this issue Jul 29, 2019 · 8 comments
Assignees

Comments

@biancadanforth
Copy link
Collaborator

As discussed here, there is no test coverage for functions exported by Fathom's JavaScript library that require rendering a page in a browser.

Given that we've already had issues with some of these error-prone functions (like isVisible, see #118 ), it would be wise to set up some automated testing for them in our CI.

As Erik mentioned, we may be able to achieve this using Firefox's Headless mode.

As part of closing this issue, a test for isVisible should be written.

If we prioritize this issue, I am very interested in working on it. For now, I will leave it unassigned.

@biancadanforth
Copy link
Collaborator Author

I spoke with jlast who is on the DevTools team and in part works on their CI.

For a web development test harness (no compatibility with landing code in Firefox), he recommends Cypress, which is Electron based. DevTools used this themselves until they switched to Mochitest (Firefox only).

There is ongoing work to make significant improvements to Puppeteer for Firefox, which he thinks will be complete around December 2019. At that point, he would recommend Puppeteer.

@erikrose
Copy link
Contributor

I watched Cypress's intro movie, and my takeaways (mostly for my own reference) were…

  1. It looks amazing UI-wise for building tests of complicated, multi-step interactions with JS apps. Our use case, however, is going to be dominated by unit-like tests of utilsForFrontEnd.js helpers for the forseeable future.
  2. They're pushing a subscription-based payment plan for their own Travis-like web-based frontend. It's not immediately obvious how to use it on our existing infra, but I don't know that it's impossible.
  3. It's monolithic, including (and requiring?) its own assertion lib and test runner.

@erikrose
Copy link
Contributor

A couple more links to follow (sorry, I got interested and couldn't help it):

@biancadanforth
Copy link
Collaborator Author

  1. They're pushing a subscription-based payment plan for their own Travis-like web-based frontend. It's not immediately obvious how to use it on our existing infra, but I don't know that it's impossible.

It looks like they have a free plan (limit: 3 users, 500 test recordings) an OSS plan (limit: 5 users, unlimited test recordings), but one of the requirements for the OSS plan is that the team or their organization is a non-commercial entity; that sounds like it would disqualify us.

A couple more links to follow (sorry, I got interested and couldn't help it):

I was reading the TravisCI stuff, and they don't support Docker on macOS, and all of the instructions either require having your own web server to serve the pages or making an account (possibly free?) through SauceLabs. I have used Taskcluster before (Mozilla's CI tool), so another option is that we could use Taskcluster and then have access to their docker workers.

That second link has some good guidance on how we might use Selenium Webdriver. It has a good article on doing so using NodeJS. Perhaps that's a good first route to go?

In general, do we want cross-browser testing? Perhaps that can be a follow-up issue?

@erikrose
Copy link
Contributor

erikrose commented Aug 3, 2019

I was reading the TravisCI stuff, and they don't support Docker on macOS,

I think what they mean is that they don't support Docker on their virtualized Mac CI boxes.

@biancadanforth
Copy link
Collaborator Author

biancadanforth commented Aug 3, 2019

I think what they mean is that they don't support Docker on their virtualized Mac CI boxes.

D'oh. Yeah. That makes sense. TravisCI runs in a VM.

Since part of this issue is to add a test for isVisible, here are test cases that I manually checked in PR #116 which should return false:

  • display: none
  • Ancestor is display: none
  • Visibility: hidden
  • Ancestor is visibility: hidden
  • Opacity: 0
  • Ancestor opacity: 0
  • getBoundingClientRect().width: 0 && overflow:hidden
  • getBoundingClientRect().height: 0 && overflow: hidden
  • Ancestor getBoundingClientRect().width(): 0 and itself overflow: hidden
  • Ancestor getBoundingClientRect().height(): 0 and itself overflow: hidden
  • Element is off-screen above page
  • Element is off-screen below page
  • Element is off-screen to the left of the page
  • Element is off-screen to the right of the page

@biancadanforth biancadanforth self-assigned this Aug 3, 2019
@pdehaan
Copy link
Contributor

pdehaan commented Aug 6, 2019

Ah cool. I've used Cypress and Puppeteer [locally, not in any CI] in the past for various website testings and screenshot generation and they're both pretty neat; although I think both are still mostly Chrome based (although I understand Puppeteer has an experimental puppeteer-firefox project[1]). Cypress.io still has cross-browser on their roadmap: cypress-io/cypress#310

I also watched the "Complete Code Coverage with Cypress" webcast the other week and it was very interesting [if you have ~60 minutes].

[1] https://aslushnikov.github.io/ispuppeteerfirefoxready/

@biancadanforth
Copy link
Collaborator Author

Thanks @pdehaan for the information. Cypress looks really polished, but as Erik mentioned, they are monolithic (we'd have to throw out our current CI, Travis).

What's more, I was able to quickly get a working solution for Travis on Monday. It spins up the latest release version of Firefox in headless mode and tests isVisible on MDN with Selenium Webdriver using Mocha.

What's left to do there is for PR #116 to merge, so that we have the right isVisible to add a test for, add our own custom test page and update the test cases accordingly.

biancadanforth added a commit that referenced this issue Aug 10, 2019
* Add a new eslint rule, 'operator-linebreak: [error, after]' in light of style-related feedback and fixed violations
* Ensure comments are complete sentences and are consistent
* Remove extra parens from conditional statements
* Update the first check in 'isVisible' to account for an extra case I found while working on #122
biancadanforth added a commit to biancadanforth/fathom that referenced this issue Aug 14, 2019
Now that we are actually linting the isVisible.js functional test, eslint reports an error:
```
/home/travis/build/mozilla/fathom/test/functional/isVisible.js

  4:68  error  "../../utilsForFrontend" is not found  node/no-missing-require
```
I believe this is because 'require' is relative to the directory in which the script is found, while eslint is checking relative to the current working directory.

This could be resolved by:
* Ignoring the error (easiest)
* Updating 'eslint-plugin-node' to >= v5.1.0 in which we can apply the 'resolvePaths' setting.
  * This requires also updating 'eslint', as otherwise 'make lint' fails
  * Updating 'eslint' updates default rules, for which there are then dozens of new violations throughout the source code to address
  * I vote that considering updating eslint be a follow-up issue, rather than part of fixing mozilla#122

In light of these options, I chose the best, easiest one, since it doesn't blow up everything else.
biancadanforth added a commit that referenced this issue Aug 20, 2019
#122: Add JavaScript functional test harness
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

3 participants