gnomad-browser
gnomad-browser copied to clipboard
Add .md transformer and static page tests
Implements a simple .md
transformer that renders the raw stringified .md
content as the __html
attribute in a react element, for the purposes of snapshot testing.
Includes includes a .pdf
transformer that uses the name of the source .pdf
file
Includes snapshot tests for the static pages:
- Home
- About
- Team
- Downloads
- Policies
- Publications
- Feedback
- Help
These represent the mostly static pages that are unlikely to change. Snapshot tests for these unchanging pages will help in catching unintentional regressions going forward.
While this transformer doesn't render the .md
content exactly as it would be on the browser (it doesn't get passed through the markdownLoader
), I'd argue it still represents a very useful set of tests.
-
@gnomad/markdownLoader
is tested in the gnomad-browser-toolkit repository - Snapshotting the raw
.md
files as strings catches regressions of the content itself.
This seems like the correct division of responsibility to me. Testing takes place in the relevant repository, and as such this repo does not test some elses code (though in this case someone else is us, but in a different repo).
Candidly, implementing an Async Transformer with Jests docs was proving very difficult for me. If there is some simple thing I was missing I would be excited at this point, but after trying many things and scouring many stack overflow and github threads, I believe the juice of passing the .md
content through the markdownLoader
in snapshot tests is not worth the squeeze, and that testing the raw .md
content is almost an identical test.
Thoughts / opinions?
Edit: inline snapshots are no longer being used, and the pre-commit hook for trailing whitespace
is not removed.
The Jest testing function toMatchInlineSnapshot()
seems like a good fit for many tests that don't rely on tests that use some variation of a loop syntax (they don't support multiple inline snapshots for one looped line of code). However, the pre-commit hook trailing-whitespace
won't accept the string literals the Jest function outputs, as it seems to output whitespace on lines as part of the inline snapshots.
I removed the trailing whitespace
pre-commit hook as part of this PR, but there may be a better way to do this, and this possibly warrants discussion. It does seem that prettier already removes trailing whitespace as part of its reformatting, with a bit more indiscretion than the pre-commit hook does.
Also - there seem to be some strange configuration differences happening, Phil and I were unable to replicate the same error on our local machines on a Zoom call when pulling from this topic branch.
I have re-created a snapshot for the DatasetSelector.spec.tsx
test, as my machine was mirroring the errors that CI was outputting. Doing so is causing CI to succeed now, however there still seems to be possibly something weird going on, and may warrant discussion in the near future. I opted to replace the snapshot as both CI and my machine were in agreement with what errors were being output.
Re: discussion Phil and I had about differing newline characters possibly causing snapshot test issues, I have checked and both the previous and new snapshot for DatasetSelector.spec.tsx
, one that was causing particular issues, used LF
as the newline character, per
git ls-files --eol
Still stumped on what actually changed, and what is causing the different development environments on CI, my machine, and Phil's machine.
The Jest testing function
toMatchInlineSnapshot()
seems like a good fit for many tests that don't rely on tests that use some variation of a loop ...
Re: a discussion with Phil some time back, this PR has moved away from doing inline snapshots, and kept the pre-commit hooks intact. Writing a comment here to keep a logical flow in the thread, and also added in edit to the initial message just so it's clear that that is no longer the case.
This PR also includes the resulting updated snapshots after running yarn jest
with the -u
flag on my machine. Without doing this, the CI was failing for certain snapshot tests.