cenfun / monocart-coverage-reports

A code coverage tool to generate native V8 reports or Istanbul reports.
MIT License
69 stars 6 forks source link

[Question]: does it support server side code coverage? #1

Closed stevez closed 8 months ago

stevez commented 10 months ago

I wonder if this library supports server side code coverage. Currently we have nextJS applications (including both client and server code), use playwright + nyc to capture the code coverage. My question is if we use your library, does it support server side coverage as well? For both V8 and istanbul?

cenfun commented 10 months ago

For current version, the coverage data is collected by Chromium Coverage API, so it works on chromium based browser. I note that Node.js also support native v8 coverage: https://nodejs.org/docs/latest/api/v8.html#v8takecoverage I haven't test it if the Node.js v8 data format is same with chromium v8's. Anyway, it good idea to support both browser and Node.js, I will looking into it.

stevez commented 10 months ago

I followed the cypress approach for istanbul: 1) create an middleware endpoint in the backend; 2) when client side send request like 'GET localhost:3000/coverage, then return the json data, so far it works, https://docs.cypress.io/guides/tooling/code-coverage

maybe you can implement the similar solution for v8 as well.

michaelhays commented 10 months ago

I've only gotten coverage on my client components for now (i.e. not RSC or Next.js route handlers). I was going to recommend the same thing -- the Cypress full stack code coverage approach is probably a good place to start.

cenfun commented 10 months ago

Thanks for the cypress example. After trying, there are 2 ways that I can successfully generate the coverage report for server side (Node.js).

One thing that is different from the browser Chromium Coverage API is that you need to read the source code manually. @stevez @michaelhays please have a try and let me know if you have any questions.

michaelhays commented 10 months ago

Hey @cenfun, thanks for this! I updated my Next.js example project with the NODE_V8_COVERAGE approach.

The client-side coverage is still working well:

(Screenshots) ![client](https://github.com/cenfun/monocart-coverage-reports/assets/6445661/c5d20e4f-4a14-4964-a491-eb74119d1996)

But it looks like the server-side coverage is erroneously reporting 100% coverage:

(Screenshots) ![server1](https://github.com/cenfun/monocart-coverage-reports/assets/6445661/e83155d4-fcf2-4f89-8a06-b27bd1666c0e) ![server2](https://github.com/cenfun/monocart-coverage-reports/assets/6445661/51b367a4-e541-4d25-8041-ef92374da358)

I'm out of time today, but I'll try to keep exploring this over the next couple of weeks.

cenfun commented 10 months ago

@michaelhays Seems that NODE_V8_COVERAGE=dir requires the node program to exit normally to generate coverage data. But next start will start a web server which will never exit normally, so it cannot generate coverage data to the dir. I created a PR with a new approach similar to Inspector. It will start coverage on global setup and collect coverage on global teardown, but Im not sure the result is correct because very few coverage data can be obtained: image

michaelhays commented 10 months ago

Ooh okay got it. Thanks for the PR, I've tested it out and got the same results. I'll keep playing around with it to see if I can find anything else there!

cenfun commented 9 months ago

@michaelhays there is a playwright issue which causes Node.js can NOT generate coverage data with env NODE_V8_COVERAGE=dir, because the webserver is killed instead of terminating gracefully.

I found a new option to collect the v8 coverage data, check the repo: https://github.com/cenfun/nextjs-with-playwright There is a problem to Next.js, the same source file like counter.tsx could be packed to both client side and server side bundle, so some lines could be executed on the server, and some lines could be executed on the client, it does not make sense for coverage report. image

image

cenfun commented 9 months ago

@michaelhays There have been some recent updates about the coverage report. Essentially, the function of coverage merging is now supported. In other words, the coverage data of the front-end and back-end can be merged together. The specific updates have been updated to this repository: https://github.com/cenfun/nextjs-with-playwright image

michaelhays commented 9 months ago

Wow, great work @cenfun!

I'm still having some trouble getting the server coverage to appear at all in my example project, not sure why. I'll continue playing around with it in the coming days.

cenfun commented 9 months ago

After made some changes, it seems like the coverage is finally being correctly obtained. image

1, First, please update to new version monocart-coverage-reports@2.3.1, there is a bug has been fixed. 2, Please run server with dev instead of start

- NODE_V8_COVERAGE=.coverage NODE_OPTIONS=--inspect npm run start
+ NODE_V8_COVERAGE=.coverage NODE_OPTIONS=--inspect npm run dev

I don't know why build + start prevents the generation of coverage, but using dev seems to provide a complete coverage. By the way, the version of Node.js I'm using is v20.9.0. (it seems there is a issue to coverage if version >= 20.10.0, see here) 3, When you start server with dev, it actually start two server or something with debug port 9229 and 9230:

[WebServer] Debugger listening on ws://127.0.0.1:9229/72b4e870-29e8-4828-8d73-96198314ef40
For help, see: https://nodejs.org/en/docs/inspector
[WebServer] Debugger listening on ws://127.0.0.1:9230/e789e92f-7847-47aa-bea1-37847f218683
For help, see: https://nodejs.org/en/docs/inspector
[WebServer]    the --inspect option was detected, the Next.js router server should be inspected at port 9230.
[WebServer]    ▲ Next.js 14.1.0
   - Local:        http://localhost:3000

Note, please take coverage with 9230 in globalTeardown.js I don't know why maybe there is sub process starting for real server.

- const res = await fetch("http://127.0.0.1:9229/json");
+ const res = await fetch("http://127.0.0.1:9230/json");

BTW, there is mistake in ComponentServer.tsx image Hope you getting it works.

michaelhays commented 9 months ago

Incredible, that all worked :sob::raised_hands:

Yep, I'd previously changed the inspector port from 9230 to 9229 since next start only seems to provide the one at 9229, but as you well know that one isn't working as expected.

You are an absolute machine, I seriously cannot thank you enough

cenfun commented 9 months ago

@michaelhays There is an optimization that connecting CDP, you can simply use the tool chrome-remote-interface.

import CDP from 'chrome-remote-interface';

const client = await CDP({
    port: 9230
});

await client.Runtime.enable();
const data = await client.Runtime.evaluate({
    expression: 'new Promise(resolve=>{ require("v8").takeCoverage(); resolve(process.env.NODE_V8_COVERAGE); })',
    includeCommandLineAPI: true,
    returnByValue: true,
    awaitPromise: true
});
await client.Runtime.disable();
await client.close();

const dir = data.result.value;

example: https://github.com/cenfun/nextjs-with-playwright/blob/main/global-teardown.js

michaelhays commented 9 months ago

Nice! That's working for me :)

stevez commented 8 months ago

Nice job! @cenfun, I want to confirm that do you support istanbul for the server side coverage? Right now we still need the istanbul solution, thanks!

cenfun commented 8 months ago

@stevez It supports all Istanbul built-in reports. I guess what you're asking are two different questions.

stevez commented 8 months ago

@cenfun, thanks for your reply, my question is specifically for the Istanbul support on server side code coverage. Since I saw the above discussions and examples are all based on v8, I need some examples for the server side coverage using Istanbul. Thanks

cenfun commented 8 months ago

@stevez I create a example repo for istanbul: https://github.com/cenfun/nextjs-with-playwright-istanbul

stevez commented 8 months ago

@cenfun, thanks for your example repo, I can run it in my local, thanks. One more thing I got the following message:

[WebServer] Starting inspector on 127.0.0.1:9230 failed: address already in use

I wonder is this normal? Or I should use a different port, like 9229?

Thanks

stevez commented 8 months ago

By the way my node version is 18.17.1

cenfun commented 8 months ago

9230 failed: address already in use

By default the debugging port is 9229 if no port is specified

"test:start": "cross-env INSTRUMENT_CODE=true NODE_OPTIONS=--inspect next dev",

And for nextjs, We will get the prompt in the console:

[WebServer] Debugger listening on ws://127.0.0.1:9229/044f4bac-ed2e-4b32-97b7-bd91a4b6b150
For help, see: https://nodejs.org/en/docs/inspector
[WebServer] Debugger listening on ws://127.0.0.1:9230/4e5b49f1-d0d8-411e-b9db-6b83813c8e38
For help, see: https://nodejs.org/en/docs/inspector
[WebServer]    the --inspect option was detected, the Next.js router server should be inspected at port 9230.

This tell us we should use 9230 (9229+1) as the debugging port. It seems that nextjs has started two services.

In fact you can use any port like 8112

"test:start": "cross-env INSTRUMENT_CODE=true NODE_OPTIONS=--inspect=8112 next dev",

And also please update the port to 8113 in global-teardown.js

    const client = await CDP({
        port: 8113
    });

I believe Node 18 is working, but you could try upgrading to Node 20.

cenfun commented 8 months ago

@stevez I've made some further optimizations for the next.config: https://github.com/cenfun/nextjs-with-playwright-istanbul Basically, we can simply use the option forceSwcTransforms to enable or disable the default compiler.

forceSwcTransforms: !process.env.INSTRUMENT_CODE

That is, it enables the Babel compiler only during testing.

stevez commented 7 months ago

Hey @cenfun, thanks for this! I updated my Next.js example project with the NODE_V8_COVERAGE approach.

The client-side coverage is still working well:

(Screenshots) ![client](https://github.com/cenfun/monocart-coverage-reports/assets/6445661/c5d20e4f-4a14-4964-a491-eb74119d1996)

But it looks like the server-side coverage is erroneously reporting 100% coverage:

(Screenshots) ![server1](https://github.com/cenfun/monocart-coverage-reports/assets/6445661/e83155d4-fcf2-4f89-8a06-b27bd1666c0e) ![server2](https://github.com/cenfun/monocart-coverage-reports/assets/6445661/51b367a4-e541-4d25-8041-ef92374da358)

I'm out of time today, but I'll try to keep exploring this over the next couple of weeks.

@michaelhays I found my repo has the similar issues, the coverage is almost 100% just for a single playwright test, I wonder how did you fix it? we use custom sever side rendering using express, front end is react