feat(plugin-coverage): add setup wizard binding#1273
feat(plugin-coverage): add setup wizard binding#1273hanna-skryl wants to merge 4 commits intomainfrom
Conversation
|
View your CI Pipeline Execution ↗ for commit 9bc20e0
☁️ Nx Cloud last updated this comment at |
@code-pushup/ci
@code-pushup/cli
@code-pushup/core
@code-pushup/create-cli
@code-pushup/models
@code-pushup/nx-plugin
@code-pushup/axe-plugin
@code-pushup/coverage-plugin
@code-pushup/js-packages-plugin
@code-pushup/eslint-plugin
@code-pushup/jsdocs-plugin
@code-pushup/lighthouse-plugin
@code-pushup/typescript-plugin
@code-pushup/utils
commit: |
Code PushUp🤨 Code PushUp report has both improvements and regressions – compared current commit cc4e3f8 with previous commit 28ccfcb. 🕵️ See full comparison in Code PushUp portal 🔍 🏷️ Categories
👍 2 groups improved, 👎 2 groups regressed, 👍 5 audits improved, 👎 6 audits regressed, 16 audits changed without impacting score🗃️ Groups
30 other groups are unchanged. 🛡️ Audits
651 other audits are unchanged. |
Code PushUp🤨 Code PushUp report has both improvements and regressions – compared current commit cc4e3f8 with previous commit 28ccfcb. 💼 Project
|
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Code coverage | 🟢 92 | 🟢 93 |  |
| Documentation | 🟡 69 | 🟡 68 |  |
4 other categories are unchanged.
👍 1 group improved, 👎 1 group regressed, 👍 3 audits improved, 👎 1 audit regressed, 2 audits changed without impacting score
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| Code coverage | Code coverage metrics | 🟢 92 | 🟢 93 | |
| JSDocs coverage | Documentation coverage | 🟡 69 | 🟡 68 | |
13 other groups are unchanged.
🛡️ Audits
| 🔌 Plugin | 🛡️ Audit | 📏 Previous value | 📏 Current value | 🔄 Value change |
|---|---|---|---|---|
| JSDocs coverage | Functions coverage | 🟥 22 undocumented functions | 🟥 33 undocumented functions |  |
| Code coverage | Function coverage | 🟩 93.5 % | 🟩 95.7 % |  |
| Code coverage | Line coverage | 🟨 89.1 % | 🟩 91 % | |
| Code coverage | Branch coverage | 🟨 89.6 % | 🟨 89.7 % |  |
| JSDocs coverage | Variables coverage | 🟥 13 undocumented variables | 🟥 24 undocumented variables |  |
| JSDocs coverage | Types coverage | 🟥 8 undocumented types | 🟥 10 undocumented types |  |
438 other audits are unchanged.
💼 Project models
🤨 Code PushUp report has both improvements and regressions.
🕵️ See full comparison in Code PushUp portal 🔍
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Documentation | 🟡 67 | 🟡 67 |  |
| Code coverage | 🟢 94 | 🟢 94 |  |
2 other categories are unchanged.
👍 1 group improved, 👎 1 group regressed, 👍 1 audit improved, 👎 1 audit regressed
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| JSDocs coverage | Documentation coverage | 🟡 67 | 🟡 67 | |
| Code coverage | Code coverage metrics | 🟢 94 | 🟢 94 | |
10 other groups are unchanged.
🛡️ Audits
| 🔌 Plugin | 🛡️ Audit | 📏 Previous value | 📏 Current value | 🔄 Value change |
|---|---|---|---|---|
| JSDocs coverage | Types coverage | 🟥 64 undocumented types | 🟥 64 undocumented types |  |
| Code coverage | Line coverage | 🟩 95 % | 🟩 94.6 % |  |
435 other audits are unchanged.
💼 Project create-cli
🥳 Code PushUp report has improved.
🕵️ See full comparison in Code PushUp portal 🔍
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Code coverage | 🟢 90 | 🟢 90 | |
5 other categories are unchanged.
👍 1 group improved, 👍 1 audit improved
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| Code coverage | Code coverage metrics | 🟢 90 | 🟢 90 | |
14 other groups are unchanged.
🛡️ Audits
| 🔌 Plugin | 🛡️ Audit | 📏 Previous value | 📏 Current value | 🔄 Value change |
|---|---|---|---|---|
| Code coverage | Line coverage | 🟩 91.2 % | 🟩 91.2 % | |
442 other audits are unchanged.
11 other projects are unchanged.
matejchalk
left a comment
There was a problem hiding this comment.
The new adjustments mechanism is very elegant. 👍
| it('should return CJS config unchanged', () => { | ||
| const input = `module.exports = { coverageReporters: ['text'] };`; | ||
| expect(addLcovReporter(input, 'jest')).toBe(input); | ||
| }); |
There was a problem hiding this comment.
Aren't we able to add LCOV to CommonJS configs as well? I'm guessing that CommonJS configs are still very common for Jest.
There was a problem hiding this comment.
magicast doesn't support CJS, and your comment made me think about the other cases where we can't reliably modify the config either (factory functions, variable references, missing files). Instead of trying to cover every edge case, I decided to make the binding tell the user when manual setup is needed.
generateConfig now receives the wizard's virtual tree, tries to add the LCOV reporter, and if it can't for any reason, prepends a comment to the generated plugin code.
The tree parameter is a minimal { read, write } contract that bindings can ignore if they don't need to modify files. It's not ideal to expose the tree this way, but I couldn't think of better alternatives. The LCOV change is arguably a side effect, but at the same time, failing to complete it would render the coverage plugin setup incomplete 😕
Let me know if you have other ideas for handling this.
There was a problem hiding this comment.
generateConfignow receives the wizard's virtual tree, tries to add the LCOV reporter, and if it can't for any reason, prepends a comment to the generated plugin code.
This is a very good solution. I like it a lot. 🙂
It's not ideal to expose the tree this way
I think it's OK. All changes go through the tree; that's the main thing.
matejchalk
left a comment
There was a problem hiding this comment.
(I clicked Approve automatically without thinking. 🤦)
| generateConfig: ( | ||
| answers: Record<string, PluginAnswer>, | ||
| ) => PluginCodegenResult; | ||
| tree?: PluginSetupTree, |
There was a problem hiding this comment.
Why is tree optional? Is there a scenario when it shouldn't be provided? 🤔
| it('should omit coverageToolCommand when test command is empty', async () => { | ||
| const { pluginInit } = await generateConfig({ | ||
| 'coverage.testCommand': '', | ||
| }); |
There was a problem hiding this comment.
This is somewhat nuanced, but I prefer to have the execution of the function we're testing - coverageSetupBinding.generateConfig in this case - very explicit and transparent within the test.
That means instead of:
await generateConfig();
await generateConfig({
'coverage.testCommand': '',
});It would be:
await coverageSetupBinding.generateConfig(defaultOptions, tree);
await coverageSetupBinding.generateConfig({
...defaultOptions,
'coverage.testCommand': '',
}, tree);BTW, if coverageSetupBinding is too verbose, you could rename it to something shorter like binding, which is still unambiguous in the context of this test file.
Closes #1254
Coverage plugin binding for the setup wizard. Auto-detects Jest/Vitest, prompts for coverage options with smart defaults, and generates plugin initialization code.
For Jest/Vitest,
generateConfigreceives the wizard's virtual tree and usesmagicast(0.3.x, as the latest 0.5.x is incompatible with ourmoduleResolution: "node") to add an LCOV reporter to the test config. If the modification fails, a comment is prepended topluginInitto remind the user to configure LCOV manually.