Introduce "Skipped" Category or Flag in Happo Reports for Unchanged stories

[pudek357]

Hey @trotzig!

We've implemented a feature in our monorepo to run Cypress tests only on affected/changed projects thanks to lerna. This has significantly reduced our GitHub Actions usage. However, this change has led to a reporting issue in Happo. Specifically, stories within the projects that are not tested (because they are unchanged) are marked as "deleted" in Happo reports, due to the current comparison method based on sha.

I propose introducing a new category in Happo reports, named "Skipped", or alternatively, adding a "(skipped)" flag to the "Unchanged" category. This would more accurately reflect the status of projects that were not tested because they were unchanged since the last sha.

I look forward to any thoughts or suggestions on this proposal.

[pudek357]

Now I'm thinking we'd need to still search within the whole monorepo for occurrences of .happoScreenshot() which was not called (as we skipped them), and then pass such array to cli:

yarn happo-e2e finalize --skippedStories="story1,story2,story3"

[trotzig]

Hi @pudek357 and thanks for the feature request! You are right that we would probably need a mechanism to tell happo about the skipped examples. Would it be possible for you to do such an analysis?

The skipped examples could be sent in the happo-e2e finalize call or a separate one, I'll have to think about that as I want to make sure this is implemented in a general way to fit other integrations too (and not just Cypress/Playwright).

Also, for this to work, you would need to create "full" reports on the main branch so that Happo always has a full list of snapshots to compare against.

[trotzig]

Let's assume we sent the skipped examples in the finalize call. It would then have to look something like this:

yarn happo-e2e finalize --skippedExamples='[{ "component": "story1", "variant": "default" }, { "component": "story2", "variant": "default" }]'

Even though they are stories in your case, the more generic Happo word for them are "examples". And each example is identified via component and variant. I know passing JSON like this to a CLI isn't ideal, but it should work in this case as long as your stories don't have quotation marks (/["']/) in their names.

[trotzig]

I realize max length for the --skippedExamples argument could come into play, but let's deal with that if it happens. The underlying implementation of happo-e2e finalize is making a regular http call to the happo API so worst case you can call the API directly and get around potential length issues.

[trotzig]

I realize there's a third property per example we need: the target name.

yarn happo-e2e finalize --skippedExamples='[{ "component": "story1", "variant": "default", "target": "chrome-large" }, { "component": "story2", "variant": "default", "target": "chrome-large" }]'

The target should correspond with the list of targets you have defined in the .happo.js configuration file, e.g.

module.exports = {
  targets: {
    "chrome-large": new RemoteBrowserTarget('chrome', { viewport: '1200x900' }),
    "chrome-small": new RemoteBrowserTarget('chrome', { viewport: '375x700' }),
  } 
}

...or if you are using Dynamic targets you'll have to pass the name of the dynamic target, or several examples each with a target name if you are using multiple dynamic targets.

[trotzig]

I thought I should also mention here that an alternative solution to using a new "skip" feature would be to generate one Happo report per project, and simply skip running the ones that are unaffected by a change. You can "orchestrate" multiple projects into a single Happo build status, see docs here: https://docs.happo.io/docs/multi-project#orchestration

But I'm still interested in exploring the skip feature as it's one that has been asked for several times.

[pudek357]

Thanks for your quick reply!

Constructing such complex array --skippedExamples='[{ "component": "story1", "variant": "default", "target": "chrome-large" }, { "component": "story2", "variant": "default", "target": "chrome-large" }]' I think it would be challenging, even if we refactor our dynamic targets to pre-defined ones. We'd need to search whole code base for code like this:

cy.get('body').happoScreenshot({
  component: 'HubPage',
  variant: 'OtherTest',
  targets: ['chrome-large'],
})

and exact the content from {..}. Such a script could be error-prone.

---

I thought I should also mention here that an alternative solution to using a new "skip" feature would be to generate one Happo report per project, and simply skip running the ones that are unaffected by a change. You can "orchestrate" multiple projects into a single Happo build status, see docs here: https://docs.happo.io/docs/multi-project#orchestration

But I'm still interested in exploring the skip feature as it's one that has been asked for several times.

We keep all Cypress Happo screenshots within one Happo project, as I understand the solution with usage of the/orchestrate endpoint will not work.

[trotzig]

I made an attempt at statically extracting calls to happoScreenshot, complete with component, variant, and targets values used. Can you try this in your repository and see if the results make sense?

Full disclosure, I wrote this with the help of chatpgt. With some guidance it produced a pretty solid script I must say. 😊

const fs = require('fs');
const path = require('path');
const esprima = require('esprima');

// Get the current directory of the script
const rootDirectory = __dirname;

// Function to recursively traverse the directory and find JavaScript files with the specified suffix
function findJavaScriptFiles(directory, suffix) {
  const files = fs.readdirSync(directory);
  let jsFiles = [];

  files.forEach((file) => {
    const filePath = path.join(directory, file);
    const stats = fs.statSync(filePath);

    if (stats.isDirectory()) {
      jsFiles = jsFiles.concat(findJavaScriptFiles(filePath, suffix));
    } else if (file.endsWith(suffix)) {
      jsFiles.push(filePath);
    }
  });

  return jsFiles;
}

// Function to extract calls to 'happoScreenshot' from a JavaScript file
function extractHappoScreenshots(filePath) {
  const content = fs.readFileSync(filePath, 'utf-8');
  const ast = esprima.parseScript(content, { tolerant: true, loc: true });

  const screenshots = [];

  // Traverse the AST to find function calls with 'happoScreenshot'
  function traverse(node) {
    if (
      node.type === 'CallExpression' &&
      node.callee.property &&
      node.callee.property.name === 'happoScreenshot'
    ) {
      const firstArgument = node.arguments[0];

      if (firstArgument && firstArgument.type === 'ObjectExpression') {
        const componentProperty = firstArgument.properties.find(prop => prop.key.name === 'component');
        const variantProperty = firstArgument.properties.find(prop => prop.key.name === 'variant');
        const targetsProperty = firstArgument.properties.find(prop => prop.key.name === 'targets');

        const component = componentProperty ? componentProperty.value.value : undefined;
        const variant = variantProperty ? variantProperty.value.value : undefined;

        let targets;
        if (targetsProperty) {
          if (targetsProperty.value.type === 'ArrayExpression') {
            targets = targetsProperty.value.elements.map((element) => {
              if (element.type === 'Literal') {
                return element.value;
              } else if (element.type === 'ObjectExpression') {
                const nameProperty = element.properties.find(prop => prop.key.name === 'name');
                return nameProperty ? nameProperty.value.value : undefined;
              }
            });
          }
        }

        screenshots.push({ filePath, loc: node.loc, component, variant, targets });
      }
    }

    for (const key in node) {
      if (node[key] && typeof node[key] === 'object') {
        traverse(node[key]);
      }
    }
  }

  traverse(ast);

  return screenshots;
}

// Main script
const jsFiles = findJavaScriptFiles(rootDirectory, '_spec.js');
const allScreenshots = [];

jsFiles.forEach((file) => {
  const screenshots = extractHappoScreenshots(file);
  allScreenshots.push(...screenshots);
});

// Output the results
allScreenshots.forEach((screenshot) => {
  console.log(`File: ${screenshot.filePath}`);
  console.log(`Location: Line ${screenshot.loc.start.line}:${screenshot.loc.start.column}`);
  console.log(`Component: ${screenshot.component}`);
  console.log(`Variant: ${screenshot.variant}`);
  console.log(`Targets: ${screenshot.targets ? JSON.stringify(screenshot.targets) : 'undefined'}`);
  console.log('\n');
});

Save as extractScreenshotCalls.js and invoke this to test:

node extractScreenshotCalls.js

Here's the full conversation I had with chatgpt when creating the script. https://chat.openai.com/share/653691ae-5292-4e75-8352-43865c3c823f

If it works or is close to working, I can prepare code on the Happo side of things to support the "skip" feature. I've made an initial draft that looks to be working okay already, but I need to do some polish work before we ship to production.

[pudek357]

Your provided script works as expected. 🚀 I had to add a small adjustment for monorepo and Typescript. I will be able to provide such param:

--skippedExamples='[{ "component": "story1", "variant": "default", "targets":"[\"chrome-small\"]" }, { "component": "story2", "variant": "default", "targets":"[\"chrome-large\"]" }]'

Just to double-check, we don't need to provide browser and viewport keys for dynamic targets?

[trotzig]

We just need the name of the target, nothing else. 🙏

[trotzig]

I'll continue working on the happo server changes -- I'll let you know when I have something for you to test!

[trotzig]

For now, the UI will look something like this for skipped examples:

To avoid a big refactor, I'm showing the skipped examples in the "Unchanged" section. We can come back and discuss possible UI improvements when we've tried the new feature some more.

[trotzig]

Re-opening just to make sure you have time to test the new release @pudek357. Let me know if you run into any issues after installing happo-e2e@2.3.0 and using the --skippedExamples option.

[trotzig]

Docs will be eventually be updated on docs.happo.io, but for now you can read instructions in the release notes: https://github.com/happo/happo-e2e/releases/tag/v2.3.0

[trotzig]

I'm seeing some errors on the server side of things that I think could be related to you attempting this -- I'm working on adding some server side validation to the items sent as --skippedExamples. If you want to continue the conversation in private you can send an email to support@happo.io and I'll follow up there.

[pudek357]

The errors appeared because I didn't pass skippedExamples correctly. Everything is fine with new command.

[pudek357]

Hey @trotzig everything seems to working fine.

I have a tiny suggestion, please take a look at the screenshot:

In Added list of examples you can find This example was skipped.. This is because we have this test skipped:

  it.skip('shows TestComponent', () => {
    arrangeTest()

    cy.get('body').happoScreenshot({
      component: 'TestComponent',
      variant: 'primary',
      targets: ['chrome'],
    })

When we build --skippedExamples array, we still pass this example {component:'TestComponent',variant 'primary',target:'chrome'} to it. If I'd want to omit this specific case, I'd need to traverse back the AST tree to check if it. or describe. is skipped.

What do you think about filtering out skipped examples from Added list on your https://happo.io/a/../compare/ page?

[pudek357]

I realize max length for the --skippedExamples argument could come into play, but let's deal with that if it happens. The underlying implementation of happo-e2e finalize is making a regular http call to the happo API so worst case you can call the API directly and get around potential length issues.

@trotzig do you have currently error handling for max length of characters? I haven't encountered any errors yet, but I just want to clarify it.

Additionally, could you specify the actual maximum length allowed for this argument?

[trotzig]

Additionally, could you specify the actual maximum length allowed for this argument?

I dont' know if there's a straight answer to this question, as it depends on the OS. Here's a SO thread suggesting the full length of the command can't be longer than 131071 characters on Linux. We can adjust things if you see failures.

What do you think about filtering out skipped examples from Added list on your https://happo.io/a/../compare/ page?

This seems like a good idea. Let me work on that right away.

[trotzig]

I just deployed a change that will ignore skipped examples in added/deleted. Closing this issue for now, feel free to reopen if you notice anything out of the ordinary.