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

🪟 🔧 Ignore classnames during jest snapshot comparison #17773

Merged
merged 3 commits into from
Oct 10, 2022

Conversation

timroes
Copy link
Collaborator

@timroes timroes commented Oct 9, 2022

What

This changes the jest tests to ignore class names during snapshot comparison.

Previously when using jest's toMatchSnapshot() method on a @testing-library/react output, we'd compare the whole DOM tree, which included CSS classnames which where minified from SASS modules. Changing CSS could lead to classnames in completely unrelated files getting a new hash and thus those file's tests would fail on completely unrelated PRs. See https://github.com/airbytehq/airbyte/pull/17582/files for an example where FrequentlyUsedDestinations and StartWithDestination tests failed despite those component had no changes at all.

How

There is unfortunately no built-in way to Jest or testing-library to ignore specific properties in snapshot testing. Thus I added a custom plugin for snapshot serialization, that will be used whenever a RenderResult (the result of testing-library/react render method) is tried to be snapshotted.

That plugin will then clone the whole DOM tree that should be snapshotted and modify each class name in it to be redacted to just the count of classnames that the element had (e.g. <2 classnames>).

Also it's now enough to just pass in the RenderResult directly to the expect and no longer required to call the asFragment() on it.

Why the many changes in the snapshots?

As a result of no longer using asFragment(), but the RenderResult directly, the wrapping elements of the snapshot are now <body><div> instead of <DocumentFragement> which caused all snapshots to change (besides the change in class that this PR intended to do).

@timroes timroes added area/frontend Related to the Airbyte webapp ui/tests labels Oct 9, 2022
@timroes timroes requested a review from a team as a code owner October 9, 2022 20:18
@github-actions github-actions bot added the area/platform issues related to the platform label Oct 9, 2022
@timroes timroes changed the title Ignore classnames during jest snapshot comparison 🪟 🔧 Ignore classnames during jest snapshot comparison Oct 9, 2022
Copy link
Contributor

@josephkmh josephkmh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍 just left one thought

airbyte-webapp/scripts/classname-serializer.js Outdated Show resolved Hide resolved
@timroes timroes merged commit 6c5a150 into master Oct 10, 2022
@timroes timroes deleted the tim/ignore-classnames-jest branch October 10, 2022 10:44
letiescanciano added a commit that referenced this pull request Oct 10, 2022
…vation

* master: (22 commits)
  Update full-refresh-append.md (#17784)
  Update full-refresh-overwrite.md (#17783)
  Update incremental-append.md (#17785)
  Update incremental-deduped-history.md (#17786)
  Update cdc.md (#17787)
  🪟 🔧 Ignore classnames during jest snapshot comparison (#17773)
  feat: replace openjdk with amazoncorretto:17.0.4 on connectors for seсurity compliance (#17511)
  Start testing buildpulse. (#17712)
  Add missing types to the registry (#17763)
  jobs db descriptions (#16543)
  config db data catalog (#16427)
  Update lowcode docs (#17752)
  db migrations to support new webhook operations (#17671)
  Bump Airbyte version from 0.40.13 to 0.40.14 (#17762)
  September Release Notes (#17754)
  Revert "Use java-datadog-tracer-base image (#17625)" (#17759)
  Add connection migrations for schema changes (#17651)
  Connection Form Refactor - Part Two (#16748)
  Improve E2E testing around the Connection Form (#17577)
  Bump strict encrypt version (#17747)
  ...
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
* Ignore classnames during jest snapshot comparison

* Replace by simple placeholder

* Update snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants