nodejs
feature: add way of testing deps of a module
New feature to test all the dependencies of a module. This will test the versions of the dependencies that get installed at the time of npm install. Current work in progress tests and documentation to come. Please let me know if this should be intergarted into citgm or citgm-all - help would be appreciated on how to do this
Checklist
- [x]
npm testpasses - [ ] tests are included
- [ ] documentation is changed or added
- [x] contribution guidelines followed here
Codecov Report
Merging #765 into master will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## master #765 +/- ##
======================================
Coverage 95.5% 95.5%
======================================
Files 27 27
Lines 889 889
======================================
Hits 849 849
Misses 40 40
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 5c3967d...bd38d3f. Read the comment docs.
@richardlau I think an alternative way of producing this functionality would be to add an optional version field to the lookup for each module that citgm-all can handle. Then all that citgm-deps would do is the npm install and grab the versions of each dependency.
@nodejs/citgm opinions on adding an optional version field to the lookup?
I'm not 100% that this is the best way to approach the problem outlined in #631
Generally packages don't "just work" with CITGM. Usually we have to manually add them to the lookup table including meta data such as version prefix, package manager default, and what platforms that module is flaky on.
If we automate the process of testing all of the dependencies installed I fear we will end up with a ton of false negatives and in turn have to manually add all of those dependencies into the lookup table before we can get accurate results. At that point a CITGM-ALL run will cover all those dependencies.
We could likely use tags to similarly filter subsets of modules if we wanted to only test supported modules. A script that automated updating the lookup.json with tags that grouped related modules would likely be very helpful for that.
I'm not going to block this, but would like to hear a bit more about how this will be used and how the data we get from the script will be useful before supporting this feature
I agree with @MylesBorins that this might end up with a lot of flaky tests in the test suite. It might be a good idea to try to automate a few more things that we require less meta data (e.g., the version prefix and the default package manager could be detected). What platforms a module is flaky on is something difficult to automate though.
I think this is a good idea on a longer term. I am just not sure if we are already at the point that this works as expected.
@BridgeAR, @MylesBorins when this work was mentioned to me it was in the context of helping projects run test themselves (for example the Express project) which I think would be useful and a good extension of CITGM.
I'm thinking we should add the functionality to do that in a way that does not affect our existing CITGM runs (ie we won't need to worry about any flakiness) but does let package maintainers experiment with using CITGM to do additional testing. What do you think?
The plan was to implement this as a new command and not add it to the CITGM runs. This is so it could be used by module maintainers to test their module dependencies using CITGM without affecting existing nodejs CI test runs.
If there are features that module maintainers could use in CITGM, then visibility of CITGM could be increased, potentially leading to more contributors?
/cc @ljharb who was just talking to me about smoketesting needs as a module author