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

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

Closed
stevez opened this issue Dec 17, 2023 · 23 comments
Closed

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

stevez opened this issue Dec 17, 2023 · 23 comments

Comments

@stevez
Copy link

stevez commented Dec 17, 2023

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
Copy link
Owner

cenfun commented Dec 17, 2023

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
Copy link
Author

stevez commented Dec 17, 2023

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
Copy link

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
Copy link
Owner

cenfun commented Dec 27, 2023

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

  • Using Node.js env NODE_V8_COVERAGE=dir

    • 1, Before running your Node.js application, set env NODE_V8_COVERAGE=dir. After the application runs and exits, the coverage data will be saved to the dir directory in JSON file format.
    • 2, Read the json file(s) from the dir and generate coverage report. see example:

    cross-env NODE_V8_COVERAGE=.temp/v8-coverage node ./test/test-node-env.js && node ./test/generate-node-report.js

  • Using Inspector API

    • 1, Connecting to the V8 inspector and enable V8 coverage.
     const inspector = require('inspector');
     const startV8Coverage = async () => {
         const session = new inspector.Session();
         session.connect();
         await session.post('Profiler.enable');
         await session.post('Profiler.startPreciseCoverage', {
             callCount: true,
             detailed: true
         });
         return session;
     };
    • 2, Taking coverage data and adding to the report after your application runs.
     const takeV8Coverage = (session) => {
         return new Promise((resolve) => {
             session.post('Profiler.takePreciseCoverage', (error, coverage) => {
                 if (error) {
                     console.log(error);
                     resolve();
                     return;
                 }
                 resolve(coverage.result);
             });
         });
     };
     const coverageList = await takeV8Coverage(session);
     // filter node internal files
     coverageList = coverageList.filter((entry) => entry.url && !entry.url.startsWith('node:'));
     await new CoverageReport(coverageOptions).add(coverageList);
     await new CoverageReport(coverageOptions).generate();

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
Copy link

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

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

(Screenshots)

server1
server2

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

@cenfun
Copy link
Owner

cenfun commented Dec 28, 2023

@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
Copy link

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
Copy link
Owner

cenfun commented Jan 6, 2024

@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
Copy link
Owner

cenfun commented Jan 24, 2024

@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
Copy link

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
Copy link
Owner

cenfun commented Jan 25, 2024

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

1, First, please update to new version [email protected], 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
Copy link

Incredible, that all worked 😭🙌

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
Copy link
Owner

cenfun commented Jan 26, 2024

@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
Copy link

Nice! That's working for me :)

@stevez
Copy link
Author

stevez commented Feb 22, 2024

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
Copy link
Owner

cenfun commented Feb 22, 2024

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

  • Q1, Generating coverage reports.
    Monocart support all reports include both V8's and Istanbul's, it also supports two input formatts of data include both V8 data and Istanbul data. so don't worry about this part.
  • Q2, Generating or Collecting coverage data
    It requires you to provide the coverage data to Monocart, whether it is V8 or Istanbul.
    Please refer following, they both include server side and client side:

@stevez
Copy link
Author

stevez commented Feb 22, 2024

@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
Copy link
Owner

cenfun commented Feb 22, 2024

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

@stevez
Copy link
Author

stevez commented Feb 23, 2024

@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
Copy link
Author

stevez commented Feb 23, 2024

By the way my node version is 18.17.1

@cenfun
Copy link
Owner

cenfun commented Feb 23, 2024

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 cenfun closed this as completed Feb 23, 2024
@cenfun
Copy link
Owner

cenfun commented Feb 23, 2024

@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
Copy link
Author

stevez commented Mar 9, 2024

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

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

(Screenshots)

server1

server2

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

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

3 participants