Technical design discussion

trotzig commented 4 years ago

We started a discussion around this module on Twitter and I wanted to summarize a few things and outline my thoughts on how this library could work, and then we can collaborate on the design in this issue. https://twitter.com/CraigCav/status/1278100205779419137?s=20

Background

This project started when @emilsjolander reached out to me asking to run Happo via Jest. I had previously made attempts that failed for various reasons (mostly related to getting styles and images/assets working with Jest). I started a new attempt and found that some of the earlier headaches were mostly gone. Jest now has all the hooks and events that we need and I think we could make a really good Happo runner with the help of it.

Usage in CI

So far, I've mostly been concerned with getting this library to work in a CI workflow. That means generating screenshots and comparing them with a previous version (baseline). Other Happo integrations (Happo Examples, Storybook, Cypress all work in similar ways:

Run a CI script that takes screenshots for a PR, put those together in a Happo report
Resolve the baseline sha (usually a commit on the master branch) and compare the two versions (tip of PR report with baseline report)

My idea for the Jest integration is to make the test runner quite "dumb" -- it will simply take screenshots (remotely via the Happo API). Comparing screenshots with previous versions will be handled separately, just like the happo-cypress integration does.

Dev usage

Jest is excellent at rapid local development. You start up in watch mode and can change code around until the tests pass. It would be great if the happo integration could make use of this as well, and help make local development smooth. Ideally it would

Allow running a single test (or a few test files) and produce subset reports
Compare those subset reports with a known baseline, but ignore the missing screenshots For that last item, we'd likely need to make some changes on the server. We can use the compare reports endpoint and pass in a known sha1 from the master branch, plus the (currently undocumented isAsync param, see happo-cypress for usage. If Happo has a report for sha1, it will compare against it. If no report is found, Happo will traverse the git history to find a report that we can compare with.

Recording HTML + CSS and collecting assets

The actual screenshooting happens on Happo's workers (different ones per browser target). We need something in Jest that simply snapshots the DOM. E.g.

import { render } from '@testing-library/react';
import Link from '../src/Link';
import { screenshot } from 'happo-jest';

it('has different variants', () => {
  const { container, rerender } = render(<Link href="#">Facebook</Link>);
  screenshot(container, { variant: 'default' });
  rerender(
    <Link href="#" inverted>
      Facebook
    </Link>,
  );
  screenshot({ variant: 'inverted' });
});

When those screenshot calls are made, we need to collect the HTML, the CSS and any assets currently rendered. This is basically what happo-cypress does 1, 2, 3, and it's possible we could extract some generic DOM snapshot library that we could use in both places (although I'm fine keeping things separated/duplicated to begin with).

Generating screenshots remotely

Then with all those snapshots collected, we can send them to happo.io for remote screenshooting. For that, we use the snap-requests API endpoint (undocumented!), via the RemoteBrowserTarget class. See happo-cypress for an example of how we send these payloads. I think we can do this in a custom HappoEnvironment for jest, in the teardown method.

We also have to put all those snap-requests together in a Happo report. This will "finalize" the full run and give us something to run comparisons with. I'm not 100% sure where this code goes. In happo-cypress, it's in two places: in an after hook when running locally and in a wrapping command when running in CI.

Comparing reports and generating diffs

When we have a baseline sha and a report that we just created, we can run a comparison between the two. In happo-cypress, this happens in a wrapping command (which was the only way to get some code to run after the entire test suite). We need to do something similar in happo-jest. Resolving the baseline and some other useful env variables happens in a resolveEnvironment module for happo-cypress and we should be able to do the same. It knows things about different CI environment.

Notes

The reason I'm referencing the happo-cypress project and not the happo.io library is that we've decided to move away from a few things we did wrong in the happo.io library:

We don't want to lazily check out a baseline commit (like we do in happo-ci)
We don't want to maintain a custom test runner, better to tag along with ones that are already good (like cypress and jest)

trotzig commented 4 years ago

I thought I'd mention here as well that I won't be working actively on this for a while. I'm taking some time off in July and we'll mostly be doing support and maintenance in the coming month. I'll make sure to keep up-to-date with this thread though!

emilsjolander commented 4 years ago

This all sounds good to me :)

CraigCav commented 4 years ago

My idea for the Jest integration is to make the test runner quite "dumb" -- it will simply take screenshots (remotely via the Happo API)

This sounds ideal for CI - and is a solid place to start.

Allow running a single test (or a few test files) and produce subset reports

Compare those subset reports with a known baseline, but ignore the missing screenshots. For that last item, we'd likely need to make some changes on the server.

Is the server code OSS? I'd assumed it isn't in order to protect happo's service model.

Happo will traverse the git history to find a report that we can compare with.

This sounds potentially challenging, since presumably happo-jest would be traversing the git history and happo's server would know about the potential report history; finding the most recent intersection seems like it could be chatty over the network? I assume happo's server doesn't have direct access to git history?

If we can get this right, happo-jest could have a really compelling local development story. The visual regression testing tool I use today commits the snapshot baseline alongside the code (basically like jest snapshot but with images). This allows for easy evaluation of whether the current code matches the baseline, but it isn't easy to scale with multiple device profiles. Getting almost immediate feedback on visual changes while developing locally is a huge productivity win that i'm really excited about.

trotzig commented 4 years ago

Is the server code OSS? I'd assumed it isn't in order to protect happo's service model.

Yeah, it's not.

I assume happo's server doesn't have direct access to git history?

It does if you have [the Happo GitHub app}(https://github.com/apps/happo) installed and connected your github repo. We store a simple sha > commitedAt combo in our db, so traversing the git history is a simple query if we have a sha from the default/master branch to begin with.

Getting almost immediate feedback on visual changes while developing locally is a huge productivity win that i'm really excited about.

Yeah this would be fantastic! I think with the power of Jest and a smart way to put together the happo.io integration, we could get there.

One thing we'll have to be smart about is performance. Waiting for a screenshot for each screenshot invocation might not perform well (there will be occasional wait times on the happo.io server, html+css+asset payloads might be large causing network delay). I'd argue that we should find a solution that spends as little time as possible when screenshot is invoked, and then do as much background work as possible. My initial strategy would be to let screenshot collect DOM snapshots, but don't send those snapshots to happo.io until we've finished the entire test file. We can then bundle together all snapshots together with an assets package (images, fonts, etc) and send that as one snap-request to happo servers. This is what happo-cypress does, and it's working well. But in doing so, we're introducing some complexity in terms of summarizing the results, since we're no longer in the context of the test itself when we get the results.

Here's how a test result could look like:

 HAPPO  src/__happo__/Button-happo-test.js (using c0d11dcd as baseline)
  ✓ _default_ 
  ✓ _inverted_
  ! _disabled_ has diff
  ✓ with icon 
! Button _inverted_ looks different from the baseline (c0d11dcd)
  See diff: https://happo.io/diffs/fbcdfdcfacbd

I don't know enough about controlling the Jest output to tell if this output is even possible though. :)

lencioni commented 4 years ago

Is the idea here for making a jest runner that runs all of the Happo tests (e.g. like jest-runner-eslint for running ESLint), or is this for adding Happo tests to existing jest runs?

trotzig commented 4 years ago

Good question @lencioni -- I don't know to be honest. Ideally you'd be able to mix in some happo screenshots with your regular tests, but I don't know if that's feasible. I do think that we should allow people to write jest tests just like they're used to (with describe, before blocks, etc), so it won't be like the eslint runner in that sense.

lencioni commented 4 years ago

jest-playwright might be worth a look: https://github.com/playwright-community/jest-playwright

happo / happo-jest