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

[BUG] Attachments not found #34

Closed
edumserrano opened this issue Jun 5, 2023 · 16 comments
Closed

[BUG] Attachments not found #34

edumserrano opened this issue Jun 5, 2023 · 16 comments

Comments

@edumserrano
Copy link

edumserrano commented Jun 5, 2023

Hi,

It seems attachments are not being fetched properly depending on how you setup your playwright outputDir and monocart-reporter outputFile.

If I setup my playwright.config.ts as such:

const _testsDir = "./tests";
const _testsOutputBaseDir = path.join(_testsDir, "test-results");
const _testReportersOutputBaseDir = path.join(_testsOutputBaseDir, "reporters");
const _htmlReporterDir = path.join(_testReportersOutputBaseDir, "html");
const _monoCartReporterDir = path.join(_testReportersOutputBaseDir, "monocart");
const _monoCartReporterOutputFile = path.join(_monoCartReporterDir, "index.html");

export default defineConfig({
    ...
    outputDir: `${_testsOutputBaseDir}`,
    reporter: [
        [
            "monocart-reporter",
            {
                name: "Playwright Monocart Report",
                outputFile: _monoCartReporterOutputFile,
                coverage: {
                    sourceFilter: (sourceName: any) => sourceName.search(/src\//) !== -1,
                },
            },
        ],
    ],
    ...
});

And have an automatic fixture as such:

const { addCoverageReport, attachCoverageReport } = require("monocart-reporter");

export { expect } from "@playwright/test";

interface CodeCoverageFixture {
    autoTestFixture: void;
}

export const test = baseTest.extend<CodeCoverageFixture>({
    autoTestFixture: [
        async ({ page }, use): Promise<void> => {
            // coverage API is chromium only
            if (test.info().project.name === "chromium") {
                await Promise.all([
                    page.coverage.startJSCoverage(),
                    page.coverage.startCSSCoverage(),
                ]);
            }

            await use();

            if (test.info().project.name === "chromium") {
                const [jsCoverage, cssCoverage] = await Promise.all([
                    page.coverage.stopJSCoverage(),
                    page.coverage.stopCSSCoverage(),
                ]);
                const coverageList = [...jsCoverage, ...cssCoverage];
                await addCoverageReport(coverageList, test.info());
                const report = await attachCoverageReport(coverageList, test.info());
            }
        },
        {
            auto: true,
        },
    ],
});

Then I run a test that produces a snapshot and I have this output:

[MCR] coverage: tests/test-results/coverage-8aefe567157b61ce7446/index.html Coverage Report - basic test
[MCR] coverage: tests/test-results/reporters/monocart/coverage/index.html Coverage Report - Playwright Monocart Report (global)
[MCR] json report: tests/test-results/reporters/monocart/index.json
[MCR] html report: tests/test-results/reporters/monocart/index.html
[MCR] view report: npx monocart show-report tests/test-results/reporters/monocart/index.html

Notice that both the HTML report and the global coverage report share the same base dir tests/test-results/reporters/monocart/ and as such I can navigate from the index.html to the global code coverage report:

  • I can go from http://localhost:8090/index.html to http://localhost:8090/coverage/index.html

However the screenshot attachments and the code coverage attachments display as not found. When I try to access the test code coverage it tries to go to http://localhost:8090/coverage-8aefe567157b61ce7446/index.html and because the path coverage-8aefe567157b61ce7446/index.html does not exist at tests/test-results/reporters/monocart it can't find the file. That path is at tests/test-results/.

The same happens for the screenshots. If I try to go to a screenshot it tries to go to http://localhost:8090/find-institution-example-basic-test-chromium/basic-test-1-expected.png and it cant find the path /find-institution-example-basic-test-chromium/basic-test-1-expected.png at tests/test-results/reporters/monocart so it fails. The path is at tests/test-results/.

It seems to me that the attachments path uses playwrights outputDirto store data instead of the directory relative to where the monocart html report is at which is indicated byoutputFile` and then there's a mismatch in where files should be ? Do you think this is a bug or am I misusing the configuration?

Let me know if you need more info.

@edumserrano edumserrano changed the title [BUG] Test coverage not found [BUG] Attachments not found Jun 5, 2023
@edumserrano
Copy link
Author

edumserrano commented Jun 5, 2023

So might not be a bug. A couple of things:

  1. I browsed around your repo and found the attachmentPath configuration value which can be added to the playwright.config.ts as such:
    reporter: [
        [
            "monocart-reporter",
            {
                name: "Playwright Monocart Report",
                outputFile: _monoCartReporterOutputFile,
                attachmentPath: (currentPath: any, extras: any) => {
                    console.log(currentPath, extras, path);
                    return "somethig?"; // allows you to remap where attachments should be located?
                    // return `https://cenfun.github.io/monocart-reporter/${relativePath}`;
                },
            },
        ],
    ],

Not sure what this could be used for but I used it with some console.log to help debug.

  1. It's now obvious to me that the html report cannot serve files that are OUTSIDE the directory where the html file is.

So perhaps this bug can be closed and at best there's a chance for an improvment:

  1. At some point, ideally as soon as possible, validate if the attachments are saved in a valid path and if not show an error message or even fail the creation of the report.

What do you think @cenfun ?

@edumserrano
Copy link
Author

edumserrano commented Jun 5, 2023

Actually, I'm not sure how to make this work. I keep going back and forth thinking if this is a bug or not.

The problem I have is that now I need to be careful in how I configure the directories so that the html report is at the same level as playwright's outputDir. Either that or I'm missing something.

The other thing I noticed is that the html reporter doesn't rely on the files produced by playwright into the outputDir. It seems to create copies of the files and move then inside the output directory configured for the html reporter. Perhaps that's what monocart-reporter should be doing as well?

@cenfun
Copy link
Owner

cenfun commented Jun 6, 2023

monocart-reporter will not copy any attachments (screenshots images/videos, trace etc.) created by playwright but link with relative path in report. (attachments maybe hundreds or even thousands, no reason copy them)
for your case:
1, outputDir is tests/test-results/, so an attachment could be tests/test-results/my-case/screenshot.png
2, monocart-reporter dir is tests/test-results/reporters/monocart/, so linked attachment path could be ../../my-case/screenshot.png
3, when you serve dir tests/test-results/reporters/monocart/, the attachment could be not found cause it not in serve dir.

It is recommended that outputDir and monocart-reporter outputFile use the same dir:

const outputDir = `./tests/test-results/`;
module.exports = {
    outputDir: outputDir,
    reporter: [
        ['monocart-reporter', {  
            name: `My Test Report ${date}`,
            outputFile: `${outputDir}/index.html`
        }]
    ]
};

In this way, index.html will be the root dir and serve all attachements., everything will work.

do you want html and monocart-reporter to be used together?
it works but will sharing the attachments. and use config option attachmentPath will help to customize the attchment path.

@edumserrano
Copy link
Author

edumserrano commented Jun 6, 2023

  1. I think I can use html and monocart-reporter together without a problem. html reporter copies the screenshots it needs (not sure about all attachments) into its outputFolder as defined in:
reporter: [
  ["html", { outputFolder: _htmlReporterDir }]
]

The reason I like the html reporter is minor and I might stop using it in the future. For now I like the screenshot comparison tool. The fact that it shows 3 tabs with Diff, Actual and Expected is really neat to me. Plus the control to move the slider over the images to check where the differences occur is also very nice.

If monocart-reporter could have the tabs like this and the slider then I'd drop the html reporter. However, not sure it's worth your investment in getting this "nice to have".

  1. Your solution is what I ended up doing. Making sure that outputDir and monocart-reporter outputFile use the same dir. However, note that when you try to do something similar with the html reporter it gives you a warning. It comes out as an error but the html reporter doesn't fail:

Configuration Error: HTML reporter output folder clashes with the tests output folder:

html reporter folder: <...>\tests\test-results\reporters\html
test results folder: <...>\tests\test-results

So for monocart-reporter your recommendation is to keep the output dir synced with playwright's output dir but the html reporter's recommendation is the opposite. And even though you are right, probably no need to copy every attachment from playwright's output dir into the reporter's dir, that is what the html reporter seems to be doing and it does then let the user configure the html reporter however he sees fit.

I understand it might not be worth giving the flexibility to the user to configure monocart-reporter to use any dir but if that's the case then I think an improvement would be to put a warning and consider failing the monocart-reporter run if you detect a missconfiguration where the attachments will not be able to be served by the generated html report.

@cenfun
Copy link
Owner

cenfun commented Jun 6, 2023

1, I haven't heard about the screenshot comparison tool before. Maybe it worth to be implemented in monocart-reporter let me do some research, thanks for the suggestion.

2, yes, the html output folder clashes with the tests output folder, my idea is that put html dir under output dir as a sub dir:

import path from 'path';

const name = 'multiple-html';
const outputDir = path.resolve(`./docs/${name}`);

export default {
    testDir: `../../tests/${name}`,
    outputDir: outputDir,

    reporter: [
        ['list'],
        ['html', {
            open: 'never',
            outputFolder: path.resolve(outputDir, 'html'),
            outputFile: 'index.html'
        }],
        ['monocart-reporter', {
            name,
            outputFile: path.resolve(outputDir, 'index.html')
        }]
    ]
};

image

  • 1 attachments
  • 2 html dir
  • 3 monocart-reporter html files

any thought?

@edumserrano
Copy link
Author

edumserrano commented Jun 6, 2023

  1. To be clear regarding the first point what I meant was this part of the html reporter:

I think the tab is better and allows for in place comparison. Meaning when I swap from one tab to another the image is always in the same place and allows for quick understanding of what changed.

image

The slider also helps with the visual difference:

image

  1. I understand your recommendation and it works just fine. I would still point out that this means we don't let the user choose whatever paths he wants. For instance I can't do this:
  • /tests/test-results: this folder is the output from playwright tests and contains screenshots etc
  • /tests/test-results/reporters: this is a folder where all reporters output would be. One can choose to have many reporters for different reasons I guess.
  • tests/test-results/reporters/monocart: this is the folder for the monocart-reporter output, where the index.html for the report would be.

This structure won't work because, as you've said, the report won't have "access" to the output files from playwright which are at /tests/test-results.

So at this point I see at least 2 options:

A) We accept that the above structure is not valid, to allow for playwright output files to be accessed the index.html must be in the same directory as playwright's output folder. If we define this as the requirement then I think the monocart-reporter should print out some warning/error console message when running so that the user knows something is wrong. You could even consider failing the creation of the monocart-reporter report so that the user doesn't miss the error message and properly updates the config.

B) We want to allow more flexibility for the user to select any folder structure he so wishes. This potentially involves more code changes and I think it means copying the attachments from playwright's output dir into monocart-reporter output dir. The trade off here is that in one hand you provide more flexibility but in the other this means taking up more space duplicating the attachments.

I'm fine with either option. I usually tend to prefer flexibility but I understand it comes at a cost and as such a simple message to identify the bad config might be the way to go here.

@cenfun
Copy link
Owner

cenfun commented Jun 8, 2023

image

Just to let you know, a similar image diff view implemented, try monocart-reporter@1.6.31

@edumserrano
Copy link
Author

edumserrano commented Jun 8, 2023

Tried it, love it 🤗

I do think it makes it much easier to spot differences because you can swap from tab to tab and get a feel for the difference. A couple of things that would be nice to haves:

  1. Could the order of the tabs be: "Diff" then "Actual" then "Expected" ? That's what the html reporter does and I think it makes sense. In case of a screenshot comparison failure showing the "Diff" tab first makes sense to me as a user.

  2. Whilst doing this did you take a stab at getting something similar to the image slider from the html reporter working on the "Actual" and "Expected" tabs? This bit:

image

Thanks a lot for these updates @cenfun

@edumserrano
Copy link
Author

I feel like I'm hijacking the initial question for this issue and asking for other things but since we've started talking about some nice to haves on monocart-html to completely replace the html-reporter, how hard do you think it would be to have this detail ported to monocart-html:

image

The html reporter will let you expand the steps and show the associated test code line if applicable. Currently, monocart-hmtl does know which line matches the step as shown by

image

However I do think it's much nicer to be able to just expand the step and see it in context.

@cenfun
Copy link
Owner

cenfun commented Jun 9, 2023

1, we can sort as "Diff" "Actual" "Expected"

2, if we do that, it will be exactly the same as the official html report. before doing that, could you please try holding and releasing the mouse on the image? it will quickly switch image previews between them. or tell us the benefits of the image slider.

3, I thought about this feature before. but there are 2 small problems, it takes time to collect the code snippets for all steps, and the code snippets will increase the size of the report file. So before doing that I would like to know, in what cases we need to check the code snippets for a successful step?

Thanks,

@edumserrano
Copy link
Author

edumserrano commented Jun 9, 2023

  1. Thanks!

  2. This is awesome. I much prefer your implementation actually.

I just didn't know about the hold/release on images. I'd suggest perhaps adding a label above the tabs (or somewhere) explaining that behaviour to the user? Some short phrase to let users know they can do just what you told me.

  1. No real need other than "it looked nice". You are right. Better to hold off on this one until someone can come with a proper need for something similar.

Feel free to close this issue.

@cenfun
Copy link
Owner

cenfun commented Jun 9, 2023

for 2, Yes, I thought there should be some notes for the user, but I don't know where to put it, maybe in tooltip? And I am a bit confused about what the content of the notes is because I am not a Engilish native speaker, can you help me? Thanks

@cenfun
Copy link
Owner

cenfun commented Jun 9, 2023

image
what do you think?

@edumserrano
Copy link
Author

edumserrano commented Jun 9, 2023

The tooltip idea is great. For the phrasing I would do something like:

  • For quick comparison of the image, click and hold on the image. Depending on which tab you are on, the image will swap to a different one. For example: to compare the actual image with the expected one, select the Actual tab, click and hold the mouse on the image and it will show the image from the Expected tab. Releasing the mouse will swap back to the image from the Actual tab.

I did something a bit more verbose because I think this behaviour is not that common so I felt better to add an example.

@cenfun
Copy link
Owner

cenfun commented Jun 9, 2023

thanks, monocart-reporter@1.6.32 released.

@edumserrano
Copy link
Author

I can confirm it's working as expected. Awesome 👍

image

This issue morphed from attachments not found, to improving the images. I should have created another issue for that to make things clearer. Apologies for that.

Regarding the original purpose of this issue:
I think you can close this since I understand how the paths must be configured for the reporter to pick up files. The only improvement you might consider is trying to detect when the user has misconfigured the paths and put a warning message on the output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants