Browser testing

[idoros]

This PR brings back browser testing using Cypress as requested in #289.

Fixes #289

• add cypress
• add cypress to test scripts
• add CI for cypress (chrome/firefox)
  • test failing spec
  • test passing spec
• test coverage (move to e2e) - try merging with unit if possible
• move tests to e2e
• remove node 12 from unit tests (code isn't suppose to runs on node anyway)
• add e2e tests when running test script

Not done in this PR:

• would be helpful in keeping Tabbable correct - auto browser tabbable list to check against tests expectation.
• merge test coverage between unit and E2E

PR Checklist

• Issue being fixed is referenced - Restore actual browser testing with Cypress #289.
• Unit test coverage added/updated.
• E2E test coverage added/updated.

[changeset-bot]

⚠️ No Changeset found

Latest commit: f392dde

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[codecov]

Codecov Report

Merging #291 (ef81675) into master (e1b59bb) will increase coverage by 6.46%.
The diff coverage is n/a.

❗ Current head ef81675 differs from pull request most recent head f392dde. Consider uploading reports for the commit f392dde to get more accurate results

@@             Coverage Diff             @@
##           master      #291      +/-   ##
===========================================
+ Coverage   93.53%   100.00%   +6.46%     
===========================================
  Files           1         1              
  Lines         263       108     -155     
  Branches       70        33      -37     
===========================================
- Hits          246       108     -138     
+ Misses         16         0      -16     
+ Partials        1         0       -1     

Impacted Files	Coverage Δ
src/index.js	100.00% <0.00%> (+6.46%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1b59bb...f392dde. Read the comment docs.

[idoros]

@stefcameron, e2e cypress tests fail because of missing permission that I don't have access to permit. Once fixed I can continue.

Edit: unrelated I would like to know why Codecov thinks there is a change to the test coverage as I haven't touched any source or test code that it knows about.

[stefcameron]

@stefcameron, e2e cypress tests fail because of missing permission that I don't have access to permit. Once fixed I can continue.

This is a strange issue. I'm not aware of any special permissions given in focus-trap and focus-trap-react as comparison, and yet Cypress works fine in CI in those repos. Maybe you need github-action@v2.6.1 instead of just github-action@v2?

Edit: unrelated I would like to know why Codecov thinks there is a change to the test coverage as I haven't touched any source or test code that it knows about.

Another mystery, I'm afraid. You also asked somewhere else how to merge code coverage from Cypress into code coverage from Jest. I don't know how to do that either.

Not much help... 😞

I did find this article: https://docs.cypress.io/guides/tooling/code-coverage But I don't think it's talking about how to merge coverage from two test systems into one for a single combined report.

Honestly, integration with codecov is nice, but not to the extent of spending days trying to figure this out. I say either we leave it as is and it reports on Jest-based tests only (and there won't be much to report because we're moving pretty much everything over to Cypress), we switch code coverage reporting to be Cypress-based only, or we just take it out because even without codecov, both jest --coverage and equivalent for Cypress should still produce nice text-based reports in the CI output/console.

As for what to have as unit tests in Jest (since I'd prefer to keep Jest "hooked up" than to rip it out), if everything moves over because everything is technically an integration/e2e test, then we can add 2 simple tests that verify that isTabbable() and isFocusable() both throw an error when node is falsy. That's a unit test, and I think Cypress would be overkill for testing something like that.

Ideally, more inner parts of the source code would be exported so that they could be individually unit tested, but I'm not asking to do that. I would simply prefer to keep Jest in place and ready for unit tests in the future for new work in future PRs (so that adding a unit test then doesn't also mean re-adding Jest to the repo and configuring it; that will be too much effort to ask, and the test just won't get written, or it'll get written with Cypress, which, at least IMO, isn't really the tool to test such things).

[idoros]

Maybe you need github-action@v2.6.1 instead of just github-action@v2

👍 this was the issue

I'd prefer to keep Jest "hooked up"

I don't mind having tests for node and browser. I believe it isn't an overkill to test any flow that should run in the browser if we already run other tests in a browser, the overhead is non existing (I will expand in the issue). We can keep the node testing for unit tests and set the env to node, so it will not mock the dom and it can actually check that the lib can load in node and do other non-dom related tests (although I would still pick a setup with common test libs with the browser tests - mocha/chai/sinon - not that I have any particular love for them :/ ).

integration with codecov is nice, but not to the extent of spending days trying to figure this out

I think I will continue and see where this leads. Getting 2 separate reports is not helpful.
If you can give me access to the Codecov of Tabbable then maybe I can try to understand what changed.

[idoros]

@stefcameron, I think I'm done, Notice that I changed the CI tests and you need to reset the required jobs.

[stefcameron]

@stefcameron, I think I'm done, Notice that I changed the CI tests and you need to reset the required jobs.

Thank you for your work here! 🎉 I will review as soon as I can.

I updated the required CI tests to be the new Node 14, e2e Chrome and e2e Firefox jobs.

[stefcameron]

Nice work! This looks great to me. I'm glad the existing Jest/Testing-Library tests moved over to Cypress fairly easily (at least that's what it looks like to me 😄 maybe it wasn't so easy for you when you were figuring it out 😉 ).

I'm approving ahead of updating/syncing the version of Cypress used for testing. Let's see if that works, and we'll merge this!

[idoros]

Could you update the dependency here, as well as the container used in CI

I would say no problem, but I'm stuck in a remote place until Saturday and for some weird reason installing Cypress from my Wifi/mobile just won't finish :/

I will try again tomorrow.

[stefcameron]

Could you update the dependency here, as well as the container used in CI

I would say no problem, but I'm stuck in a remote place until Saturday and for some weird reason installing Cypress from my Wifi/mobile just won't finish :/

I will try again tomorrow.

Thanks for trying. I found a few minutes this morning and made the change myself. It was a lot more than just one new commit because there had been conflicting package.json updates made on master since, and those had to be carried all the way through your dozen or so commits... 😄

LMK if it looks good to you. If so, and if it succeeds, I'll merge it.

[idoros]

Thank you. Looks fine to me.

[stefcameron]

@allcontributors add @idoros for tests

[allcontributors]

@stefcameron

I've put up a pull request to add @idoros! 🎉