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

Add documentation/example for coverage with c8 #89

Closed
mindplay-dk opened this issue May 10, 2021 · 8 comments
Closed

Add documentation/example for coverage with c8 #89

mindplay-dk opened this issue May 10, 2021 · 8 comments
Labels

Comments

@mindplay-dk
Copy link
Contributor

I've been trying to set up c8 to instrument code-coverage in my zora project.

It seems there are plenty of wrong ways to set this up - for example, I intuitively tried this:

node build.js -- test && c8 node --enable-source-maps dist/test.js | tap-difflet

This leads to really strange results, as the tap-output from zora gets the default text-report from c8 appended to the stream, causing the tap-difflet reporter to report the wrong number of passed/failed tests.

I got a little closer with something like this:

node build.js -- test && c8 --reporter none --all node --enable-source-maps dist/test.js | tap-difflet && c8 report

In other words, run c8 with --reporter none while instrumenting code-coverage, avoiding the garbled tap stream - then run c8 again with report to make it pick up the coverage data from the first run. It took a while to figure this out, as the command-line docs for c8 aren't great either.

My next problem is, it reports for the test-files themselves (*.test.ts) and also for index.mjs in node_modules/zora for some reason - I've tried to control this with the --include and --exclude switches, to no avail:

node build.js -- test && c8 --reporter none --all --include 'src' --exclude '**/*.test.*' node --enable-source-maps dist/test.js | tap-difflet && c8 report

It's not clear to me if these switches apply to c8 instrumentation run, or the reporter run... I tried using the switches with c8 report instead - adding the --include switch results in an empty report, while adding the --exclude switch has no effect at all.

Since c8 is the code-coverage tool you recommend, presumably you've managed this get this working? 🙂

Can we have a working example in the README maybe?

Do you have a working project you can point to for reference?

(Stuff like this is very time-consuming and frustrating. Have you thought about making a home page or wiki for zora or something? I'd like to help with documentation. Zora is very much "batteries not included", which I like - but it's really important we can point people to working examples or documentation that is easy to follow.)

@lorenzofox3
Copy link
Owner

The whole point of a unix pipeline and the unix philosophy is to split a set of tasks into different specialised processes which communicate by the standard streams (stdout and stdin).

In our case that would be a node process (running zora) producing a TAP stream which is piped into a reporting process (tap-difflet here). With the output of c8 into the stdout, you add to the TAP stream some text tap-dfflet can not parse and does not ignore apparently. That is not really related to zora itself but I understand your confusion if you are not familiar with the Unix philosophy.

I am not really fond of the code coverage metric but if I had to use it I would rather use a different script to avoid the cost of code instrumentation on every run.

You use TS if I am correct. So you can have a look at that gist.

Alternatively, you can ask c8 to use a reporter which does not output anything on the stdout: c8 -r html node foos.js

I would let the reporter to each developer so if one wants to use tap-difflet, he can install globally tap-difflet and then run npm t | tap-difflet for example.

Regarding you second problem. I don't know what your build.js does but I usually don't have to exclude the test files so the issue is probably related to the build script.

Can we have a working example in the README maybe?

That be great ! I had started a while back a list of recipes (which might be out of date) which could help you.

You might find that article of your interest as well

@mindplay-dk
Copy link
Contributor Author

I understand the basics of streams and pipes - what wasn't obvious is the fact that c8 would just start spitting out content by default or what the alternative was - that it needs to run twice to get it working as part of an existing pipeline, and so on. It's not really well documented.

The recipe list looks like a good starting point. The TS recipe from the gist probably should be added? And there should be a link to the recipes repo, front-and-center, in the README - probably in the "Installation" section, as this will be the first thing anybody needs in order to get rolling. 🙂

Hmm, looking at this makes me think about building a simple web-based tool which would scaffold the whole package.json for you - something with options for language, entry-points, etc... just something for the 90% use-cases so anybody can get started in under two minutes... I think this would definitely help drive adoption. I'd like to see zora take over the world. 😉

@mindplay-dk
Copy link
Contributor Author

In your examples, what does the RUN_ONLY flag do? Which tool is picking this up?

@lorenzofox3
Copy link
Owner

Zora uses is to allow the only mode: by default zora throws if it encounters a only while not in only mode

@mindplay-dk
Copy link
Contributor Author

I sank another half-day into this and still no closer to a working project. 😕

I gave your gist a go first, but that didn't work. I don't think this approach is going to work with esbuild, since it requires "type": "module" in the package.json file, and the only way it works under node is without "type": "module".

So I went back to the recipe, but it's not very different from what I had already, and it doesn't work for me.

mindplay@RSCHULTZ:~/workspace/zora-recipes/3_node_typescript$ npm run test

> node_simple@1.0.0 test
> npm run test:build && npm run test:run


> node_simple@1.0.0 test:build
> tsc


> node_simple@1.0.0 test:run
> node ./test/index.js

internal/modules/cjs/loader.js:1216
      throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath);
      ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/mindplay/workspace/zora-recipes/3_node_typescript/node_modules/zora/dist/bundle/index.js
require() of ES modules is not supported.
require() of /home/mindplay/workspace/zora-recipes/3_node_typescript/node_modules/zora/dist/bundle/index.js from /home/mindplay/workspace/zora-recipes/3_node_typescript/test/addition.spec.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /home/mindplay/workspace/zora-recipes/3_node_typescript/node_modules/zora/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1216:13)
    at Module.load (internal/modules/cjs/loader.js:1049:32)
    at Function.Module._load (internal/modules/cjs/loader.js:937:14)
    at Module.require (internal/modules/cjs/loader.js:1089:19)
    at require (internal/modules/cjs/helpers.js:73:18)
    at Object.<anonymous> (/home/mindplay/workspace/zora-recipes/3_node_typescript/test/addition.spec.js:3:16)
    at Module._compile (internal/modules/cjs/loader.js:1200:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1220:10)
    at Module.load (internal/modules/cjs/loader.js:1049:32)
    at Function.Module._load (internal/modules/cjs/loader.js:937:14) {
  code: 'ERR_REQUIRE_ESM'
}

I'm using node 14.4.0.

I'm having a little more luck in my own project - it's similar, but I use esbuild rather than tsc:

{
  "type": "module",
  "scripts": {
    "build": "node build.js",
    "test": "npm run test:build && npm run test:run",
    "test:build": "node build.js -- test",
    "test:run": "node --enable-source-maps dist/test.js | tap-difflet --pessimistic",
    "test:coverage": "c8 --reporter none --all npm run test && c8 report --reporter text --reporter lcovonly",
    "test:type-check": "tsc --noEmit && echo 'Type-check succeeded'",
    "test:ci": "npm run build && npm run test:type-check && npm run test",
    "server": "serve"
  },
  "devDependencies": {
    "@babel/core": "^7.13.10",
    "@babel/preset-env": "^7.13.10",
    "babel-preset-minify": "^0.5.1",
    "c8": "^7.7.2",
    "esbuild": "^0.9.0",
    "serve": "^11.3.2",
    "tap-difflet": "^0.7.2",
    "typescript": "^4.2.3",
    "zora": "^4.0.2"
  }
}

This runs, and I thought I had code-coverage set up and working with c8, but it only appears to work - it's simply reporting 100% code-coverage for all files. If I remove an entire test, the corresponding module still gets 100% code-coverage, haha - I thought my coverage states looked a little too good to be true. 😉

It seems to have something to do with source-maps. If I switch between inline and external source-maps, I get wildly different results: with inline source-maps, the reporter actually lists my source .ts files in the report, but lists them all with 100% coverage. If I switch to external source-maps, it doesn't seem to pick those up at all - it displays only the compiled entry-point test.js file in the report, although now it seems to accurately report the coverage percentage; not that that's at all helpful, since I can't tell which lines were covered.

I'm all out of ideas. Why is every JS tooling experience like this. Nothing should be this hard. I don't think I'm smart enough to solve this. I think I'll just give up and proceed without code-coverage for now. 😑

@lorenzofox3
Copy link
Owner

lorenzofox3 commented May 11, 2021

yes, it is always a pain to mix node, typescript, bundler, ES module, commonjs module, etc. I try to stay away of all these unnecessary build steps (doest TS actually help you to ship better code ? )

I am not familiar with many of the tools you are using so I am not sure I can help much. Have you tried nyc for code coverage ?

@mindplay-dk
Copy link
Contributor Author

does TS actually help you to ship better code ?

On this project, yes - especially on the test side, it's massively better.

I will try nyc at some point, maybe next week - the tool fatigue is way too much right now.

You understand the recipe didn't run at all, out of the box, right? It's probably outdated with regards to either node or some of the npm dependencies. Why does every JS project seem to fall apart in about a year or so. 😂

@lorenzofox3
Copy link
Owner

closed in favor of #50

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

No branches or pull requests

2 participants