microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
66.82k stars 3.67k forks source link

[Feature] "Accept new snapshot" button in html-reporter / UI Mode #24310

Open nikolay-yavorovskiy opened 1 year ago

nikolay-yavorovskiy commented 1 year ago

Hi all. Recently, on our project, we introduced screenshot tests using Playwright. These tests have shown themselves very well - every release they catch UI bugs. We began to increase their number but rather quickly ran into a problem.

Our workflow is as follows: we created a separate project in the Playwright config - purely UI tests, which run separately from functional tests. After the run - we look at the results in the standard Playwright html-reporter. In general, it is quite convenient. It highlights the diffs, you can see both the original and the new screenshot to view - everything is cool.

But relatively often there is such a situation that the test fails for valid reasons. Not because of some bug, but simply because some UI was changed, this is, so to speak, the expected failure. But it is important when looking at failed tests not to miss those that really fail due to bugs.

The problem is that now, after running the tests it is necessary, let's say, to update 20 that fell "validly" and accept new originals for them, but for 5 - it is not necessary, because they fell due to a bug - there is no tool to do so. You have to run the tests again with --update-snapshots, and the diffs will not be generated in this case. Then you need to sit in your IDE and watch and check which pictures to commit and which not. Plus, the stability is not always 100%, sometimes there are flake tests, and they also need to be looked at, so they are not accidentally committed, and so on. In short, it's quite an overhead dealing with this.

The presence of the "Accept new" button for each failed test separately - would completely solve this problem. Therefore, I really want to have a button like "Accept new golden snapshot", so that for each failed test, a new screenshot can be accepted as the new original, after which, accordingly, it will be replaced on disk in the directory with screenshots, it will appear in the diff in the git, and you will only need to commit it.

Also, it is desirable that pressing this button turns the test into "green", that is, it would turn the test from failed to pass.

I don't know how many people use screenshot tests using Playwright, but I think everyone who does has the same problem. Playwright essentially provides everything you need for screenshot testing, the only thing missing is a convenient way to manage screenshots.

shadowusr commented 1 year ago

Hey. I'm currently working on implementing this feature. Planning on sending a PR today. I'm not a member of playwright team though, so I'll have to get approval and discuss details with them.

shadowusr commented 1 year ago

Okay, the accept screenshot buttons themselves are mostly ready:

However, I've stumbled into some issues while implementing the aforementioned test status update and need some more time to work things out.

d-ryabtsev commented 1 year ago

Okay, the accept screenshot buttons themselves are mostly ready: 68747470733a2f2f692e696d6775722e636f6d2f76546f66654e752e676966

However, I've stumbled into some issues while implementing the aforementioned test status update and need some more time to work things out.

What is it that you are using to run the tests here? Is that the debugger? We don't run the tests with tracing turned on because that slows down the tests considerably.

shadowusr commented 1 year ago

What is it that you are using to run the tests here? Is that the debugger? We don't run the tests with tracing turned on because that slows down the tests considerably.

It's Playwright UI mode: https://playwright.dev/docs/test-ui-mode Usually used to run and debug tests during development locally. You don't have to use tracing in CI to use this.

The feature I'm working on is not connected to static HTML reports or other reports you might generate in your CI runs. I'm adding those buttons specifically to the UI mode.

nikolay-yavorovskiy commented 1 year ago

We don't use UI mode, it's not very convenient for us, as I mentioned in the feature title, we want this specifically in the html-reporter.

shadowusr commented 1 year ago

We don't use UI mode, it's not very convenient for us, as I mentioned in the feature title, we want this specifically in the html-reporter.

Ah, well, then there was some misunderstanding. I'm working on this feature exclusively for the UI mode.

As for HTML report, that's actually pretty non-trivial to implement as of now (if not outright impossible without severe changes to the playwright architecture):

What's preventing you from using UI mode? Maybe we can think in that direction and resolve UI mode issues, to me it sounds more realistic

nikolay-yavorovskiy commented 1 year ago

We don't use UI mode, because tests run significantly faster in regular headless mode, and generally I don't see any additional value in this mode, but maybe I didn't use it enough to see the benefits. Regular --debug is usually enough for debugging purposes...

nnson0310 commented 1 year ago

We don't use UI mode, it's not very convenient for us, as I mentioned in the feature title, we want this specifically in the html-reporter.

Ah, well, then there was some misunderstanding. I'm working on this feature exclusively for the UI mode.

As for HTML report, that's actually pretty non-trivial to implement as of now (if not outright impossible without severe changes to the playwright architecture):

* In order to update screenshots, we need access to FS, but currently, HTML report is a self-contained folder with files that can be served as a web page and has no server counterpart

* HTML report is meant to be detached from the project. For instance, it can be uploaded to S3 or somewhere else, then accepting screenshots from it doesn't make sense

What's preventing you from using UI mode? Maybe we can think in that direction and resolve UI mode issues, to me it sounds more realistic

I think the reason which prevent me and other Playwright users using UI mode is below:

GotBinGo commented 1 year ago

@shadowusr I appreciate the work you are doing, this will be really useful.

I would say @nikolay-yavorovskiy has a good point, I also think that having that functionality in the HTML reporter would be amazing.

The solution I've thought of is, that the HTML reported could support generating a patch file or script. That way a small JS script on the report page could just generate a the patch that the developer could apply locally.

On way of doing that would be creating a real git patch file that includes all the binary image data. I think that is hard in pure JS.

Another approach would be just creating a small bash script that could fetch the images from the origin the report is currently viewed at. This is important to us because we would like our CI system to host the report file. The dream for us is to be able to update the expected images without generating screenshots on the developers machine locally. The patch script proposed here could be run by the developer, it would fetch the new expected images into the correct folders from the playwright html report that is hosted by the CI system. The developer could then commit the new images.

I've been thinking about generating that patch code in a Tampermonkey script, but it would be nice to see this functionality in the official html reporter.

shadowusr commented 1 year ago

@GotBinGo just to be clear, for the time being I've stopped working on this functionality because the playwright team is not ready to collaborate on such large contributions and has different views on this feature. I think it's better to either wait for them to implement it or wait for someone to discuss all the details with maintainers before trying to contribute

shadowusr commented 1 year ago

@aslushnikov, hey! The issue's been stale for while, so I wonder what the current status is and when it may be possible for this feature to be implemented?

No pressure at all, just want to know if we can count on this to be released in a near future. Let me know if I can do something (either help in some way or contribute code) to speed things up.

aslushnikov commented 1 year ago

Hey @shadowusr, it's tentatively scheduled for 1.39!

d-ryabtsev commented 1 year ago

Hey @shadowusr, it's tentatively scheduled for 1.39!

@aslushnikov We are really looking forward to this feature. It is absolutely essential for visual testing

nikolay-yavorovskiy commented 1 year ago

@aslushnikov Hi Andrey. I see that the 1.39 tag was removed, so we should not expect this feature any time soon? 🙁

edumserrano commented 11 months ago

I do understand the feelings conveyed before regarding the why people would prefer a way to have the "Accept new snapshot" button in an html-reporter as well as I understand the challenges/design choices the playwright team would have to make to provide it.

However, I'd like to say that I've upvoted this issue and I would be happy if at least I got the "Accept new snapshot" button on the UI mode! My team's flow looks like this:

For us the button on UI mode would already be a great addition.

stuymedova commented 9 months ago

@aslushnikov Hi, are there any plans on working on this issue? Can we expect to see a solution soon? 🙏

nikolay-yavorovskiy commented 9 months ago

@aslushnikov @mxschmitt @pavelfeldman Hi, I have another idea, which might be a little easier to implement. Can we get a standard diff image tool when we run tests with --update-snapshots? Just showing the previous and the current versions side by side, diff etc. Then you can simply roll back screenshots that are incorrect in your IDE. I expected that it would be easier to implement it rather than creating an "Accept screenshot" button that you seem to struggle with to introduce it seems.

zinaelnahel89 commented 7 months ago

Has this feature been implemented yet ?

zinaelnahel commented 3 months ago

@mxschmitt was this implemented ? Or why is it closed ?

yokoyama-takuya commented 1 week ago

I want that one too.

I have tried the code that autosaves the image and gives the error. Setting is updateSnapshots: 'missing' .


import path from "path";
import fs from 'fs-extra'

test("test name", async ({ page, context }, testInfo) => {
  // .... write your page request

  const fileName = "any file name";
  const filePath = path.resolve(testInfo.snapshotPath(), fileName);

  const screenshot = await page.screenshot();

  if (fs.existsSync(filePath)) {
    try {
      expect(screenshot).toMatchSnapshot({ name: fileName });
    } catch (e) {
      // Automatically save when there is a difference and cause an error.
      fs.outputFileSync(filePath, screenshot);
      throw e;
    }
  } else {
    fs.outputFileSync(filePath, screenshot);
  }
});

But when error, incorrect reports. Actual and Expected are now the same.
Howerer correct expected image at execution time have saved in outputDir: './test-results'