percy / percy-ember

Ember addon for visual regression testing with Percy
https://docs.percy.io/docs/ember
MIT License
73 stars 44 forks source link

`percySnapshot` + `percyCSS` does not work as expected #333

Closed twokul closed 3 years ago

twokul commented 3 years ago

Hello!

I have been trying to use percyCSS option as outlined in the documentation, and I cannot get it to work.

Here is an example:

// This is a schematic example with QUnit and Ember
test('An amazing test', async function(assert) {
  await visit('/some-url');
  await percySnapshot(assert, {
    percyCSS: `
      .an-existing-css-class {
        display: none !important;
      }
    `
  });
});

I have tried specifying the same CSS in Percy configuration and that did not work:

// .percy.js
module.exports = {
  version: 2,
  snapshot: {
    widths: [1440],
    // match testing container
    minHeight: 900,
    percyCSS: `
      .an-existing-css-class {
        display: none !important;
      }
    `
  },
  discovery: {
    allowedHostnames: [],
    // https://git.io/JZLF4
    // Increased asset discovery timeout to help with font loading
    networkIdleTimeout: 250,
  },
  static: {
    baseUrl: '/',
    files: '**/*.{html,htm}',
    ignore: '',
  },
  upload: {
    files: '**/*.{png,jpg,jpeg}',
    ignore: '',
  },
};

I have tried using the media query, like so and that did not work:

@media only percy {
  .an-existing-css-class {
    display: none !important;
  }
}

The only way I got it to work is via a new class that I have to apply to every element of the DOM I want hidden which is subpar:

@media only percy {
  .hide-in-percy {
    display: none !important;
  }
}

I feel like I'm missing something obvious but cannot seem to see it, unfortunately.

I am using the latest published versions of both @percy/percy-ember & @percy/cli.

Robdel12 commented 3 years ago

Hey @twokul! The best way to debug this if you give a snapshot ID where this CSS is expected to work. Usually when we see this not work it's due to specificity (but having the snapshot will lend more info as to what's going on). There isn't anything particularly special about Percy CSS -- it's added to the bottom of the page. It's possible if the CSS you're trying to override is more specific than what the passed Percy only CSS is

twokul commented 3 years ago

@Robdel12 ๐Ÿ‘‹ Here is a link to the build & screenshot. The CSS class is .nav-date-chooser (trying to hide it.)

I was somewhat surprised that percyCSS worked in one case but did not in others ๐Ÿ˜„ I have to be missing something obvious ๐Ÿ˜„

Robdel12 commented 3 years ago

Thanks for that link ๐Ÿ˜ƒ I see in a snapshot below the linked one it does have Percy specific CSS applied:

image

This would be the snapshot percyCSS option passed (not the @media only percy approach). That does look like it works there.

But the linked snapshot doesn't have any Percy specific CSS file (hm, why? ๐Ÿคจ) It seems like these are being passed as per-snapshot options and .percy.js config might not be captured?. Can you run with --verbose with that CSS in the config file? it should log out the config that's being used for that build. And that CSS will be applied to each captured snapshot too.

twokul commented 3 years ago

Here is another test run with the following config:

module.exports = {
  version: 2,
  snapshot: {
    widths: [1440],
    // match testing container
    minHeight: 900,
    percyCSS: `
      div[data-test-nav-date-chooser], .report-page-thumbnail canvas {
        display: none !important;
      }
    `,
  },
};

div[data-test-nav-date-chooser] elements are hidden as expected; .report-page-thumbnail canvas are not.

It seems like these are being passed as per-snapshot options and .percy.js config might not be captured?

I do think the config is captured because some of the styles are applied from it. I could not get the per-snapshot option to work at all ๐Ÿค”

Output with --verbose:

โžœ  percy exec -v -- ember test --filter='[visual acceptance]'
[percy:config] Found config file: .percy.js (0ms)
[percy:config] Using config:
{
  version: 2,
  snapshot: {
    widths: [
      1440
    ],
    minHeight: 900,
    percyCSS: '\n' +
      '      div[data-test-nav-date-chooser], .report-page-thumbnail canvas {\n' +
      '        display: none !important;\n' +
      '      }\n' +
      '    '
  },
  discovery: {
    allowedHostnames: [],
    networkIdleTimeout: 500
  },
  static: {
    baseUrl: '/',
    files: '**/*.{html,htm}',
    ignore: ''
  },
  upload: {
    files: '**/*.{png,jpg,jpeg}',
    ignore: ''
  }
} (16ms)
[percy:client] Creating a new build... (13ms)
[percy:core:install] Successfully downloaded Chromium 812851 (64823ms)
[percy:core] Percy has started! (2234ms)
[percy:cli:exec] Running "ember test --filter=[visual acceptance]" (0ms)

In any case, let me know if I can provide any other info ๐Ÿ’ช

Robdel12 commented 3 years ago

That looks to be working:

image

All the snapshots I've downloaded from that build have the Percy CSS present, so that's good news ๐Ÿ˜ƒ

From here it looks like it's a selector issue. In the Panel Controls | Transaction Center renders as expected snapshot, the CSS is present but there's no selector on the page with div[data-test-nav-date-chooser].

image


A little tricky tip for .report-page-thumbnail canvas -- that wont work because we're not rendering canvas elements in Percy's browsers. Canvas doesn't carry any state in the DOM, so we serialize it into an image. You'll want report-page-thumbnail img for that one to hide the canvas el (which is really an image). We copy over attrs/classes from the canvas element, so if you target by selector rather than tag it would work too (looks like there's a .report-thumbnail-canvas class on the canvas element).

twokul commented 3 years ago

Alright! ๐Ÿš€

So this config seems to work as expected, link to the build:

module.exports = {
  version: 2,
  snapshot: {
    widths: [1440],
    // match testing container
    minHeight: 900,
    percyCSS: '.report-thumbnail-canvas, div[data-test-nav-date-chooser] { display: none !important; }',
  },
  discovery: {
    allowedHostnames: [],
    // https://git.io/JZLF4
    // Increased asset discovery timeout to help with font loading
    networkIdleTimeout: 500,
  },
  static: {
    baseUrl: '/',
    files: '**/*.{html,htm}',
    ignore: '',
  },
  upload: {
    files: '**/*.{png,jpg,jpeg}',
    ignore: '',
  },
};

@Robdel12 Do you have any idea why the following does not work?

// This is a schematic example with QUnit and Ember
test('An amazing test', async function(assert) {
  await visit('/some-url');
  await percySnapshot(assert, {
    percyCSS: `
      .an-existing-css-class {
        display: none !important;
      }
    `
  });
});
github-actions[bot] commented 3 years ago

This issue is stale because it has been open for more than 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] commented 3 years ago

This issue was closed because it has been stalled for 28 days with no activity.