cenfun / monocart-reporter

A playwright test reporter (Node.js)
https://cenfun.github.io/monocart-reporter/
MIT License
206 stars 12 forks source link

[Question] Code coverage when merging sharded reports #139

Closed edumserrano closed 2 months ago

edumserrano commented 2 months ago

I've used the documentation about Merge Shard Reports to successfully run my playwright tests in a sharded mode and then merge the monocart-reporter reports.

The problem I have though is that I'm using the Code Coverage Report feature and each shard contains a v8 code coverage report file. These files aren't merged using the technique described in the Merge Shard Reports section and though in this section you mention that:

You may need to transfer the attachments by yourself and update the path of the attachments with attachmentPath API.

That doesn't solve the issue of merging the code coverage reports right? If I manually transfer the code coverage attachments to the expected directory I will have the problem that the different shards have multiple code coverage files with the same directory structure. I would have to overwrite files ending up with the code coverage from only one of the shard runs right?

Any thoughts on this? Am I missing something or this isn't supported yet?

cenfun commented 2 months ago

@edumserrano Yes, it can only merge test reports, but not coverage reports. See Merge Coverage Reports for merging code coverage reports Example: merge-code-coverage

I don't think we need to transfer the code coverage attachments, because it will generate new code coverage attachments after merged. But we need to use the raw report for merging code coverage. (in the example repo, the raw report will be transferred by github actions upload-artifact and download-artifact)

Long time no see 😊 Currently, MCR has become an independent tool specifically for code coverage, and monocart-reporter is just a custom reporter for playwright that simply integrates MCR without covering all its features. Therefore, if we need to use advanced code coverage features, perhaps we need to write some scripts manually, or even use MCR separately (playwright-coverage).

If we want to continue integrating MCR with monocart-reporter, my suggestion would be: after the code coverage reports are merged, just copy and override the merged coverage report to the global coverage directory in the output directory of test report. For example: the monocart-reporter option outputFile is ./test-results/report.html, then the global coverage dir would be ./test-results/coverage/

edumserrano commented 2 months ago

Following the instructions from both Merge Shard Reports and Merge Coverage Reports I've managed to create a single merge.js file that merges both the monocart-reporter reports and the monocart-coverage-reports reports.

The problem I'm facing now is that the link to the global code coverage options in the merged monocart-reporter report is incorrect.

What I'm doing is running my Playwright tests with two shards:

Then I run the merge.js and output the merged reports to <root-dir>/shards/merged. The contents of the merge folder look like this:

root-dir/
└── shards/
    └── merged/
        ├── monocart-report.html
        ├── monocart-report.json
        └── code-coverage/
            └── v8/
                └── index.html

The link on the monocart-reporter report at <root-dir>/shards/merged/monocart-report.html to the global code coverage points to <root-dir>/code-coverage/v8/index.html.

On the merge function from monocart-reporter I have tried to set the CoverageReportOptions.reportPath to <root-dir>/shards/merged/code-coverage/v8/index.html but that didn't take effect. Is this perhaps a bug or am I doing something wrong? Shouldn't I be able to specify the path to the global code coverage for the merged monocart-reporter report? In fact, I can't specify the CoverageReportOptions.name either. It seems it tries to use some values from the sharded reports? The CoverageReportOptions.name is not very important here, it's fine if it uses the same name as the one from the sharded reports but I should be able to specify the reportPath right?

Note that on the playwright.config.ts which generates the sharded reports I do set the CoverageReportOptions.reportPath to path.resolve('./test-outputs/playwright/code-coverage', 'v8/index.html') which works fine when running without shards.

[!NOTE]

root-dir is used to mean the root directory of the project where for instance the playwright.config.ts, merge.js and package.json files are.

cenfun commented 2 months ago

It seems a bit complex to merge coverage manually. I will consider whether it can also merge the coverage report when merging the test report.

edumserrano commented 2 months ago

I guess it would be nice if merging the test reports would also take care of merging the coverage report. Meanwhile, following the instructions you provided it wasn't hard for me to get something like this:

// merge.js

const monocartReporter = require("monocart-reporter");
const coverageReport = require("monocart-coverage-reports");

async function mergeCodeCoverageReportsAsync() {
  const inputDir = [
    "./shards/1/code-coverage/raw",
    "./shards/2/code-coverage/raw",
  ];

  const coverageOptions = {
    // this name doesn't seem to have any effect either which makes sense since I'm only merging the coverageOptions  variable is only used to merge the code coverage reports, not to "attach" them to the monocart test report.
    name: "Merged Coverage Report",
    inputDir,
    outputDir: "./shards/merged/code-coverage",
    sourceFilter: (sourcePath) => {
      return (
        sourcePath.startsWith("projects/playground/src") ||
        sourcePath.startsWith("projects/ngx-module-federation-tools/")
      );
    },
    entryFilter: (entry) => {
      return !entry.url.includes("fonts.googleapis.com");
    },
    reports: [
      [
        "v8",
        {
          outputFile: "v8/index.html",
          inline: true,
          metrics: ["lines"],
        },
      ],
      [
        "console-summary",
        {
          metrics: ["lines"],
        },
      ],
    ],
  };

  await new coverageReport.CoverageReport(coverageOptions).generate();
}

async function mergeTestReportsAsync() {
  const reportDataList = [
    "shards/1/monocart-report.json",
    "shards/2/monocart-report.json",
    "shards/3/monocart-report.json",
    "shards/4/monocart-report.json",
  ];

  await monocartReporter.merge(reportDataList, {
    name: "My Merged Report",
    outputFile: "shards/merged/monocart-report.html",
    // the below has no effect
    // coverage: {
    //   name: "new name",
    //   reportPath: "./shards/merged/code-coverage/v8/index.html",
    // }
  });
}

async function main() {
  await mergeCodeCoverageReportsAsync();
  await mergeTestReportsAsync();
};

main();

And when executing with node ./merge.js, the output is:

[MCR] Merged Coverage Report
┌───────┬────────────┬─────────┬───────────┬───────┐
│ Name  │ Coverage % │ Covered │ Uncovered │ Total │
├───────┼────────────┼─────────┼───────────┼───────┤
│ Lines │    89.36 % │   7,401 │       881 │ 8,282 │
└───────┴────────────┴─────────┴───────────┴───────┘
[MR] merging report data ...
[MR] report data loaded: shards/1/monocart-report.json
[MR] report data loaded: shards/2/monocart-report.json
[MR] report data loaded: shards/3/monocart-report.json
[MR] report data loaded: shards/4/monocart-report.json
[MR] generating test report ...
[MR] coverage: code-coverage/v8/index.html Coverage Report - Playground app for @ln/ngx-module-federation-tools (global)
[MR] My Merged Report
┌─────────────┬──────────────────────┐
│ Tests       │ 354                  │
│ ├ Passed    │ 203 (57.3%)          │
│ ├ Flaky     │ 0   (0.0%)           │
│ ├ Skipped   │ 148 (41.8%)          │
│ └ Failed    │ 3   (0.8%)           │
│ Steps       │ 18360                │
│ Suites      │ 69                   │
│ ├ Projects  │ 3                    │
│ ├ Files     │ 22                   │
│ ├ Describes │ 0                    │
│ └ Shards    │ 4                    │
│ Retries     │ 0                    │
│ Errors      │ 151                  │
│ Logs        │ 0                    │
│ Attachments │ 9                    │
│ Artifacts   │ 1                    │
│ Playwright  │ v1.46.1              │
│ Date        │ 20/08/2024, 10:54:11 │
│ Duration    │ 4m 12s               │
└─────────────┴──────────────────────┘
[MR] html report: shards/merged/monocart-report.html (json: shards/merged/monocart-report.json)
[MR] view report: npx monocart show-report shards/merged/monocart-report.html

The directory where the merged reports are outputted looks like this:

root-dir
  shards
    merged
      monocart-report.html
      monocart-report.json
      code-coverage
        v8
          index.html

With this I'm almost there, there are two things that are missing:

1) I need to copy the attachments from each of the shard test reports into the merged directory <root-dir>/shards/merged. Or else I won't see the screenshots for diff, actual or expected. I could probably adjust the merge.js script to do this as well but for now I'm doing it manually. I just copied the relevant folders from each of the shards directory into this folder so that I can see the screenshots from failed tests. 2) In the merged test report, the global code coverage link for the merged code coverage report is incorrect. It points to <root-dir>/code-coverage/v8/index.html. If we notice the output from running node .\merge.js, we can see the line [MR] coverage: code-coverage/v8/index.html Coverage Report - Playground app for @ln/ngx-module-federation-tools (global). This path is correct if it was relative to the directory where the merge report was created, which for my example would be <root-dir/shards/merged/code-coverage/v8/index.html but what we get is <root-dir/code-coverage/v8/index.html. I don't know why this is the case and it seems that setting the CoverageReportOptions.name and CoverageReportOptions.reportPath when calling monocartReporter.merge has no effect.

All this to say:

1) Yes it would be nice if merging the reports would be easier. If it would also merge the code coverage reports and even copy the screenshots to the merged directory. Being able to run a simple command similar to npx playwright merge-reports but for monocart-reporter would be truly awesome. However I can understand this is likely a large piece of work. What do you think? 2) Whilst you think whether or not the first option is something achievable, would it be perhaps a small/easy fix to allow the report path for the global code coverage to be set when merging the reports? That would at least mean there's a path forward for this right? Otherwise I don't know how I could resolve my issue. Or is this also something complicated to solve @cenfun?

cenfun commented 2 months ago

Thanks, I have figured out how to merge coverage reports, and also copy attachments to the merged directory when merging test reports. WIP.

cenfun commented 2 months ago

Please try monocart-reporter@2.7.0

Note: The coverage reports will be merged automatically if we specify the raw report in coverage options:

// global coverage options
coverage: {
    reports: [
        // for merging coverage reports
        'raw'
    ]
}
edumserrano commented 2 months ago

Testing results from monocart-reporter@2.7.0

Perhaps I'm doing something wrong but I got two problems:

1) On the merged report I got multiple links to the links to the code coverage under the artifacts section. There should only be one link correct? image

2) The merged code coverage file is incorrect. It contains instead the coverage report from the last shard only.

How I executed the test

I run 4 test executions (4 shards) and then manually copied the output from monocart-reporter into shards/<shard-number> folder. This resulted in the following folder structure:

root-dir/
└── shards/
    ├── 1/
    │   ├── monocart-report.json
    │   ├── monocart-report.html
    │   ├── code-coverage/
    │   │   └── raw/
    │   │       └── ...
    │   └── assets/
    ├── 2/
    │   ├── monocart-report.json
    │   ├── monocart-report.html
    │   ├── code-coverage/
    │   │   └── raw/
    │   │       └── ...
    │   └── assets/
    ├── 3/
    │   ├── monocart-report.json
    │   ├── monocart-report.html
    │   └── assets/
    └── 4/
        ├── monocart-report.json
        ├── monocart-report.html
        └── assets/

Then I ran npx tsx ./playwright.monocart-reporter-merge.ts to merge the sharded reports. The playwright.monocart-reporter-merge.ts file looks like this:

import { MonocartReporterOptions, merge } from "monocart-reporter";

async function mergeTestReportsAsync(): Promise<void> {
  const reportDataList: unknown[] = [
    "shards/1/monocart-report.json",
    "shards/2/monocart-report.json",
    "shards/3/monocart-report.json",
    "shards/4/monocart-report.json",
  ];
  const options: MonocartReporterOptions = {
    name: "My Merged Report",
    outputFile: "shards/merged/monocart-report.html",
  };
  await merge(reportDataList, options);
}

async function main(): Promise<void> {
  await mergeTestReportsAsync();
}

main();

This resulted in a merged folder being created with the following structure:

root-dir/
└── shards/
    └── merged/
        ├── monocart-report.html
        ├── monocart-report.json
        ├── code-coverage/
        │   └── v8/
        │       └── index.html
        └── assets/

The merged code coverage is incorrect

edumserrano commented 2 months ago

coverage options aren't respected during merge - intended?

In addition to the feedback above the MonocartReporterOptions.coverage doesn't seem to be respected at all when doing the merge with the merge function.

For instance, after I run the sharded tests, following the same scenario as described in my previous comment, if I run npx tsx ./playwright.monocart-reporter-merge.ts to merge the sharded reports and set the MonocartReporterOptions.coverage as follows:

import {
  MonocartReporterOptions,
  ReportDescription,
  merge,
} from "monocart-reporter";

async function mergeTestReportsAsync(): Promise<void> {
  const codeCoverageReports: ReportDescription[] = [
    [
      "v8",
      {
        outputFile: "v8/index.html",
        inline: true,
        metrics: ["lines"],
      },
    ],
    [
      "console-summary",
      {
        metrics: ["lines"],
      },
    ],
    [
      "cobertura",
      {
        file: "cobertura/code-coverage.cobertura.xml",
      },
    ],
    [
      "lcovonly",
      {
        file: "lcov/code-coverage.lcov.info",
      },
    ],
  ];
  const reportDataList: unknown[] = [
    "shards/1/monocart-report.json",
    "shards/2/monocart-report.json",
    "shards/3/monocart-report.json",
    "shards/4/monocart-report.json",
  ];
  const options: MonocartReporterOptions = {
    name: "My Merged Report",
    outputFile: "shards/merged/monocart-report.html",
    coverage: {
      name: "new name",
      reportPath: "./shards/merged/code-coverage/v8/index.html",
      reports: codeCoverageReports,
    }
  };
  await merge(reportDataList, options);
}

async function main(): Promise<void> {
  await mergeTestReportsAsync();
}

main();

I would expect to get different types of code coverage, one for V8, one for console summary, one for cobertura and one for lcovonly. However, I only get a V8 report. The name and reportPath are also ignored.

Would this be something that could be supported?

If it helps, the scenario for this would be running the Playwright tests in sharded mode in CI, then generating a merged report where:

Another problem with coverage options not being respected

It means I can't define the monocart-reporter coverage report options on the playwright.config.ts like this:

if (isRunningInShardedMode) {
    _codeCoverageReports = [
      [
        "raw", // used for for merging coverage reports from multiple shards into a single coverage report.
        {
          outputDir: "raw",
        },
      ],
      [
        "console-summary",
        {
          metrics: ["lines"],
        },
      ],
    ];
  } else {
    _codeCoverageReports = [
      [
        "v8",
        {
          outputFile: v8RelativeFilePath, 
          inline: true,
          metrics: ["lines"], 
        },
      ],
      [
        "console-summary",
        {
          metrics: ["lines"],
        },
      ],
      [
        "cobertura",
        {
          file: "cobertura/code-coverage.cobertura.xml",
        },
      ],
      [
        "lcovonly",
        {
          file: "lcov/code-coverage.lcov.info",
        },
      ],
    ];
  }

The above sets the monocart-reporter coverage options to only be raw and console-summary when running in sharded mode but to also use v8, cobertura and lcovonly when NOT running in sharded mode. I have some logic that defines the isRunningInShardedMode depending on whether or not the playwright test command was used with the --shard option.

If I try the above, then when I do the merge I do NOT get any code coverage. This is perhaps because you only generate v8 html code coverage if the playwright.config.ts requested it when the sharded run was executed.

I think this should not be the case. After running with the raw reporter we should be able to generate any kind of code coverage report regardless of what was set on the playwright.config.ts for the sharded run right?

edumserrano commented 2 months ago

Suggestion

It seems that some of these problems come from how you are deciding to merge monocart reports and its code coverage. If we take a look at the merge-cli from Playwright what they suggest is:

1) use the blob reporter to get raw data. 2) use the merge-cli to merge the report 3) optionally pass in a playwright configuration file for the merge operation, otherwise the default playwright.config.ts will be used. Note that configuration file for the merge only needs to export the configuration for the testDir and reporter values of the playwright configuration. Everything else is likely ignored.

If we draw a parallel for the monocart-reporter:

1) You also use the raw reporter to get code coverage. You have a json file with the monocart report data which probably serves as raw data for merging the test results. 2) Instead of a merge-cli we create a small merge script which uses a merge function from the monocart-reporter package to merge the reports 3) In the merge script we can specify the name of the merged report as well as the source of the sharded reports but nothing else. In contrast with the playwright merge cli which let's you specify other merge related settings such as the reporters to output from the blob (raw) reporter.

I think a lot of the issues I reported on the previous comments would be solved if the merge function respected more of the MonocartOptions which let you specify things like, where to output the reports, which code coverage options to use, etc.

This said, it might be really hard to do this so let me know your thoughts...

cenfun commented 2 months ago

I found that the output directory of raw report is code-coverage/raw in the shards. The default path should be coverage/raw, I think that's why the merge failed. Please do not use a custom output directory for shard coverage, this is not supported for now (maybe fix it next patch).

In fact, the merge is generating new coverage reports with new coverage options from the raw data files.

I know the blob reporter. When Playwright did not yet support the blob reporter, Monocart already had the merge functionality. Moreover, Monocart needs to collect a lot of test data at runtime, so it cannot obtain the report for Monocart from the blob reporter. Therefore, the merge approach is different.

cenfun commented 2 months ago

Please try monocart-reporter@2.7.1

edumserrano commented 2 months ago

Change the directory from code-coverage to just coverage

As per:

I found that the output directory of raw report is code-coverage/raw in the shards. The default path should be coverage/raw, I think that's why the merge failed. Please do not use a custom output directory for shard coverage, this is not supported for now (maybe fix it next patch).

I tried changing the coverage directory to be coverage instead of code-coverage, however that didn't change the outcome. On monocart-reporter@2.7.0 it still resulted in an incorrect code coverage after the merge. Still showing two links on the monocart report and they both show the code coverage from the last shard,

This is just to keep you informed, to let you know that it didn't work but it doesn't matter much because the behavior is different with version 2.7.1

Testing results from monocart-reporter@2.7.1

[!NOTE]

Apologies if I didn't express myself correctly in the previous posts. I didn't mean to say that you should do things like the blob reporter and the merge-cli from Playwright. I was merely trying to use that as an example to point out that when trying to do a merge with monocart-reporter, the merge didn't seem to respect the configuration provided to it (like the fact that it wouldn't obey the code coverage options).

Version 2.7.1 changes this though, the code coverage options are now respected and things like the scenario from Another problem with coverage options not being respected are now working as expected.

Version 2.7.1 has improved on all of the issues I reported here 🎉. I believe I found a small bug but I'll open a separate issue for it as to not side track this one.

Thank you for the quick resolution @cenfun 🥇