Migration/3.0

[jankapunkt]

trafficstars

This will make the package fully 3.0 aware

TODOS

• [x] npm deps updated
• [x] test app 3.0 updated
• [x] local lint passing
• [x] local tests (someapp) passing
• [x] fix html reporter #85
• [x] ci (github actions lint and test) running
• [x] coverage runs on multiple apps (needs users to test)
• [x] legacy coverage runs on multiple apps (needs users to test)
• [x] no package conflicts
• [x] documentation and README updated
• [ ] fix broken sourcemap paths

[jankapunkt]

@serut circleci fials due to permission issues, which only you can fix. Do you want to fix it or do you want circleci being removed from checks?

[jankapunkt]

~~@serut is there a reason why coverage and legacy coverage are split into two? Does it make sense to merge them, since Meteor 3.0 is breaking anyway?~~ Nevermind

[jankapunkt]

@serut I finally got it running and it also works in my apps and packages. However circleci is still not working but this is nothing I can fix. If we want people to test the new version it might make sense to publish a relase candidate for both of the packages.

[serut]

@serut I finally got it running and it also works in my apps and packages. However circleci is still not working but this is nothing I can fix. If we want people to test the new version it might make sense to publish a relase candidate for both of the packages.

I've fixed the SSH key to allow checkout. Do you know how to fix the build with Circle CI ?

Otherwise yes we can remove CircleCI build.

[jankapunkt]

@serut I never used it so I don't know a thing how it works internally, sorry. Would you mind publishing a 4.3.0-rc.0 for coverage and 0.4.0-rc.0 for legacy coverage? With these I could gather a few people from the community like @StorytellerCZ to test the RCs with real apps beyond my own.

[serut]

I do not have a setup to build the package and publish a release candidate. I've added you on Atmosphere package to let you publish the release candidate version of the package.

legacy coverage is for package and coverage package is for Meteor app as far I remember

[codecov-commenter]

:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.79%. Comparing base (2017158) to head (8363168). Report is 9 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##           master      #95       +/-   ##
===========================================
+ Coverage   76.13%   86.79%   +10.66%     
===========================================
  Files          19        1       -18     
  Lines         486       53      -433     
===========================================
- Hits          370       46      -324     
+ Misses        116        7      -109     

Files	Coverage Δ
client/methods.js	86.79% <100.00%> (ø)

... and 18 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 050d739...8363168. Read the comment docs.

[jankapunkt]

@StorytellerCZ do you have a project or package that depends on this here and are willing to test?

[jankapunkt]

@serut HTML reporter works for app tests but failed for packages with [500] {"type":"failed","message":"No coverage information have been collected"}. I'm investigating where this comes from.

[serut]

There is still no coverage on server side sourcecode. It may be a regression for users too

[StorytellerCZ]

@jankapunkt no, not at this time.

[jankapunkt]

There is still no coverage on server side sourcecode. It may be a regression for users too

@serut where? In this package or in a user's app? Because I am using it with one of our apps and it works for server and client, BUT if I use with a package test then there are indeed no server coverage.

I debugged yesterday and today but could not find the root cause!

What I found though is, that __coverage__ is never filled in package tests, while in app tests it does get filled. Thought this is something in hookLoader but commenting this file has no effect whatsoever.

Any Idea what's going on there?

[jankapunkt]

@serut this seems to be a path issue in legacy coverage source-map.js - the path is never correct and testingFromPackageDir is always false.

[jankapunkt]

Update: Hook.hookRunInThisContext is not working anymore, because vm.runInThisContext never gets called. I checked this with a Meteor 2 app and it seems the build process has changed somehow, at least I found no other explanation for this right now.

You can check by replacing Hook.runInThisContext with

const vm = Npm.require('vm');

// ... later in hookLoader

  vm.runInThisContext = function (code, options) {
    debugger
    return 'console.log("VM:' + options.filename + '");' + code;
  }

Neither debugger breaks in node inspector nor gets anything printed to the console.

[jankapunkt]

I think I finally found the issue: https://github.com/istanbuljs/istanbuljs/tree/main/packages/nyc-config-hook-run-in-this-context

Prior to node.js 11.11.0 require() was implemented using vm.runInThisContext(). This meant that running with hook-run-in-this-context enabled required disabling hook-require. Starting with node 11.11.0 require() is no longer implemented with vm.runInThisContext(), so hook-require still needs to be enabled. This base configuration enables hook-run-in-this-context and provides the correct setting for hook-require to ensure that modules loaded by require() are instrumented once.

[jankapunkt]

Well I finally got it covered. The crucial parts are to add a .babelrc file with plugin config to the package-level root (tested with some other package). Then there is another fix (that seems to apply only to this package) and this is to add **/lmieulet:meteor-coverage/** to include in coverage.json in order to include this package.

[jankapunkt]

@serut @StorytellerCZ please note that this package is not backwards compatible to Meteor 2 so I prepared the versions in the documentation etc. for a major bump to

lmieulet:[email protected]
lmieulet:[email protected]

these versions will be updated, once I have meteortesting:[email protected] released, which will add 5.0.0 to the weak dependency. Otherwise the dependencies would not resolve.

See: https://github.com/Meteor-Community-Packages/meteor-mocha/pull/157

[jankapunkt]

Instrumentation works fine but sourceMap paths seem to be different in Meteor 3. Basically package paths not contains author:pacakgename when instrumented by babel+istanbul which leads to missing source files when opening the specific files in the coverage reports.

Problem is, that filePath can't be modified, because it's a read only so i will try to copy them as a while into a new Object and then alter the paths.

[jankapunkt]

I opened an issue as it seems this is a problem with Meteor itself, since the babel-plugin-istanbul gets already a wrong path at compile-time: https://github.com/meteor/meteor/issues/13287

[serut]

Nice work @jankapunkt !

If the source map file path cannot be fixed on the Meteor side, maybe we can try to fix it on the package side as the file must be generated and located somewhere?

While we wait for the fix on the Meteor side, we can publish a version on atmosphere to let people test the release ?

[jankapunkt]

@serut yes please publish an RC or Beta of them both packages so people can at least test if things work on the application-level tests.

[jankapunkt]

@serut when attempting to alter the paths afterwards, I get

W20240820-14:19:47.948(2)? (STDERR) [coverage] TypeError: Cannot set property path of #<FileCoverage> which has only a getter
W20240820-14:19:47.948(2)? (STDERR)     at Object.writeFile (packages/lmieulet:meteor-coverage/server/report/report-generic.js:50:43)
W20240820-14:19:47.948(2)? (STDERR)     at Object.generate (packages/lmieulet:meteor-coverage/server/report/report-generic.js:38:10)
W20240820-14:19:47.948(2)? (STDERR)     at Object.generateJSONReport (packages/lmieulet:meteor-coverage/server/report/report-remap.js:29:16)
W20240820-14:19:47.949(2)? (STDERR)     at Object.generate (packages/lmieulet:meteor-coverage/server/report/report-remap.js:39:10)
W20240820-14:19:47.949(2)? (STDERR)     at Object.generateReport (packages/lmieulet:meteor-coverage/server/report/report-service.js:29:21)
W20240820-14:19:47.949(2)? (STDERR)     at Object.exportFile (packages/lmieulet:meteor-coverage/server/handlers.js:86:19)
W20240820-14:19:47.949(2)? (STDERR)     at packages/lmieulet:meteor-coverage/server/router.js:53:16
W20240820-14:19:47.949(2)? (STDERR)     at packages/lmieulet:meteor-coverage/server/router.js:26:19
W20240820-14:19:47.949(2)? (STDERR)     at processTicksAndRejections (node:internal/process/task_queues:95:5)

where FileCoverage is created by the istanbul instrumenter and actually has literally only getters :-D

[serut]

Strangely, I cannot publish a rc version :

=> Build failed:                              
   
   While selecting package versions:
   error: Conflict: Constraint lmieulet:[email protected] || 2.0.1 || 3.0.0 || 4.0.0 || 5.0.0 is not satisfied by lmieulet:meteor-coverage 5.0.0-rc.0.
   Constraints on package "lmieulet:meteor-coverage":
   * lmieulet:meteor-coverage@=5.0.0-rc.0 <- top level
   * lmieulet:[email protected] <- local-test:lmieulet:meteor-coverage 5.0.0-rc.0
   * lmieulet:[email protected] || 2.0.1 || 3.0.0 || 4.0.0 || 5.0.0 <- meteortesting:mocha 3.2.0

So I'll publish directly the version 5.0.0

[jankapunkt]

@serut I can add the RC to meteortesting:mocha so you don't need to publish a new major immediately

[serut]

@serut I can add the RC to meteortesting:mocha so you don't need to publish a new major immediately

It's too late !! 5.0.0 is available with current branch content

[jankapunkt]

No problem, will keep working on this until we get it running for apps and packages

[jankapunkt]

@serut coming back at this, since updating to Meteor 3.0.4 (and later) its not working anymore as they did some changes to the babel compiler apparently:

error log

/home/jankapunkt/.meteor/packages/meteor-tool/.3.0.4.8vvvjd.vzb8i++os.linux.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.linux.x86_64/tools/runners/run-app.js:380
        throw e;
        ^

TypeError: Cannot read properties of null (reading 'id')
    at InputFile.resolve (/tools/isobuild/compiler-plugin.js:406:61)
    at packages/babel-compiler.js:592:23
    at Array.some (<anonymous>)
    at requireWithPrefixes (packages/babel-compiler.js:587:26)
    at requireWithPath (packages/babel-compiler.js:505:14)
    at resolveHelper (packages/babel-compiler.js:477:24)
    at packages/babel-compiler.js:450:19
    at Array.forEach (<anonymous>)
    at walkHelper (packages/babel-compiler.js:449:10)
    at walkBabelRC (packages/babel-compiler.js:435:24)
    at BabelCompiler.BCp._inferHelper (packages/babel-compiler.js:543:19)
    at BabelCompiler.BCp._inferFromBabelRc (packages/babel-compiler.js:377:14)
    at BabelCompiler.BCp.inferExtraBabelOptions (packages/babel-compiler.js:351:10)
    at BabelCompiler.BCp.processOneFileForTarget (packages/babel-compiler.js:227:10)
    at packages/babel-compiler.js:136:25
    at JsOutputResource.finalize (/tools/isobuild/compiler-plugin.js:938:7)
    at JsOutputResource.hasPendingErrors (/tools/isobuild/compiler-plugin.js:943:5)
    at JsOutputResource.reportPendingErrors (/tools/isobuild/compiler-plugin.js:948:9)
    at ImportScanner.scanFile (/home/jankapunkt/.meteor/packages/meteor-tool/.3.0.4.8vvvjd.vzb8i++os.linux.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.linux.x86_64/tools/isobuild/tools/isobuild/import-scanner.ts:1085:9)
    at ImportScanner.scanImports (/home/jankapunkt/.meteor/packages/meteor-tool/.3.0.4.8vvvjd.vzb8i++os.linux.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.linux.x86_64/tools/isobuild/tools/isobuild/import-scanner.ts:741:9)
    at Function.computeJsOutputFilesMap (/tools/isobuild/compiler-plugin.js:1404:9)
    at ClientTarget._emitResources (/tools/isobuild/bundler.js:1166:30)
    at /tools/isobuild/bundler.js:861:7
    at Object.enterJob (/tools/utils/buildmessage.js:387:12)
    at ClientTarget.make (/tools/isobuild/bundler.js:849:5)
    at /tools/isobuild/bundler.js:3293:7
    at /tools/isobuild/bundler.js:3452:25
    at Object.capture (/tools/utils/buildmessage.js:282:5)
    at bundle (/tools/isobuild/bundler.js:3274:18)
    at bundleApp (/tools/runners/run-app.js:584:26)
    at AppRunner._runOnce (/tools/runners/run-app.js:629:35)
    at AppRunner._runApp (/tools/runners/run-app.js:951:23)

Node.js v20.18.0

I will update to provide a fix and push to this branch here. Maybe this will also help to resolve the correct paths for the packages.

[jankapunkt]

I found the http package is still pinned to 1.1, potentially causing package dependency conflicts (newer major version is 2.x) however http is considered deprecated because node fetch is now native (incl. AbortController) so this could be dropped and replaced with native fetch calls. I could add this to the PR if you like @serut what do you think?

[serut]

I'm not using in way Meteor

So yes, if you think it works, go !

[Zegnat]

@jankapunkt is this still something you are continuing work on? I tried to introduce coverage to a Meteor app based on lmieulet:[email protected] and lmieulet:[email protected] (which seemingly came from this branch?) on Meteor 3.3, and haven‘t been able to get any coverage to output yet (the COVERAGE_APP_FOLDER is created but remains empty).

With that said, it could be that I am doing something wrong or that the README (even within this migration/3.0 branch) is incomplete for a full Meteor stack that:

1. Uses [email protected],
2. Has meteor.modern set to true in package.json.

I would love to help testing if you still wanted people to add this to their Meteor applications 👍