Chore/testing infrastructure #78
Conversation
…sage for unit testing media chrome custom elements. Update gitignore to ignore coverage reports
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/mux/media-chrome/31fsaJJtgsmAwSiQRXx36EWuJ7de |
| @@ -0,0 +1,145 @@ | |||
| import { spy } from 'sinon'; | |||
There was a problem hiding this comment.
Why all the underscores in __tests__?
There was a problem hiding this comment.
__tests__ is a default for Jest, pretty common: jestjs/jest#637
| describe('<media-play-button/>', () => { | ||
| it('passes the a11y audit', async () => { | ||
| const el = await fixture(`<media-play-button></media-play-button>`); | ||
| await expect(el).shadowDom.to.be.accessible(); |
There was a problem hiding this comment.
wtf, you can just do to.be.accessible()? Wow.
There was a problem hiding this comment.
yeah. it's not perfect, but it's a good audit to have. It's built on axe core (https://www.npmjs.com/package/chai-a11y-axe)
|
Hack example of Testing Library with shadow dom/custom elements support. Adding for documentation/reference in case we decide to revisit: https://github.com/MatthiasKainer/dom-testing-tools-web-components-todo-list |
There was a problem hiding this comment.
Some small suggestions, but overall really happy with this 🙌🏼
I generally default to Jest, and I see you tried that first, but we're hamstrung by your comment in the notes, so this all looks like the right setup.
If Jest does play nicer with web components world in the future, then it would be nice to transition at some point down the road, only because Jest + testing-library feels like the more "modern" setup. Practically speaking though, this all feels like a great way to have unit test coverage.
| @@ -0,0 +1,145 @@ | |||
| import { spy } from 'sinon'; | |||
There was a problem hiding this comment.
__tests__ is a default for Jest, pretty common: jestjs/jest#637
| /** @TODO Implement me (CJP) */ | ||
| expect(false).to.be.true; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Maybe just remove the ones that are skipped rn? I generally feel like having @TODO littered around source + test files is an anti-pattern because they're not very visible and often times never get done.
If it's truly a TODO -- file an issue, if not, then just remove the clutter.
That's my opinion, anyway.
There was a problem hiding this comment.
I'm cool with removing the @TODOs but I'd recommend we leave the skips, as that's a common practice for tests for visibility and "red"/"green" testing (whether or not we're doing TDD/BDD).
| { label: 'English', kind: 'subtitles', language: 'en-US' }, | ||
| ]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Having these tests makes understanding the source wayyyy easier, and now I'd feel much more comfortable updating/refactoring any of that source code 👍🏼
| @@ -13,7 +13,7 @@ | |||
| "build": "snowpack build --config snowpack.prod.config.js", | |||
| "dev": "snowpack dev --config snowpack.dev.config.js", | |||
| "start": "yarn dev", | |||
| "test": "echo \"Error: no test specified\" && exit 1", | |||
| "test": "web-test-runner --coverage", | |||
There was a problem hiding this comment.
Can we add a .github/workflows/ci.yml file with something like
name: CI
on: [push, pull_request]
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node-version }}
- run: yarn install --frozen-lockfile
- run: yarn test
- run: yarn build
Something like that -- goal would just be to make sure tests pass and the build succeeds on every push
There was a problem hiding this comment.
I'd recommend we only require passing tests for pull_requests to avoid adding friction with pairing/sharing scenarios (or just "save your work" best practices).
💯 agree. |
This PR adds the basic infrastructure for unit/integration testing of web components/custom elements as well as JS modules, including:
NOTES:
jestplustesting-libraryfor better assertion & querying semantics, especially for the DOM & web components/custom elements. However, jest currently has some node/JSDOM assumptions that require some hack solutions to work with Web Testing Library, andtesting-librarycurrently has no plans for shadowDOM support. If we prefer these semantics, we can iterate to come up with a viable solution.