ci: don't output code coverage table in logs #5813
Conversation
Problem: c8 was not given a reporter, therefore it defaulted to `text` which is the terminal. Solution: Set it to `lcov`, which is what we have been using previously. Note: More work may be needed on the .c8rc.json files. We might not need the one for core/ anymore. They may not be configured in the optimal way. Is it behaving how we want it to?
jpinkney-aws
left a comment
jpinkney-aws
left a comment
There was a problem hiding this comment.
Do we run any tests at all from core anymore? If not, we would probably also need to update other codecov paths as well
packages/toolkit/package.json
Outdated
| "test": "npm run testCompile && c8 --allowExternal ts-node ../core/scripts/test/launchTest.ts unit dist/test/unit/index.js ../core/dist/src/testFixtures/workspaceFolder", | ||
| "testE2E": "npm run testCompile && c8 --allowExternal ts-node ../core/scripts/test/launchTest.ts e2e dist/test/e2e/index.js ../core/dist/src/testFixtures/workspaceFolder", | ||
| "testInteg": "npm run testCompile && c8 --allowExternal ts-node ../core/scripts/test/launchTest.ts integration dist/test/integ/index.js ../core/dist/src/testFixtures/workspaceFolder", | ||
| "test": "npm run testCompile && c8 --reporter lcov --allowExternal ts-node ../core/scripts/test/launchTest.ts unit dist/test/unit/index.js ../core/dist/src/testFixtures/workspaceFolder", |
There was a problem hiding this comment.
do we need to manually add the flag if its already in the .c8rc.json? The only reason we had to do that for allowExternal was because it wasn't exposed in the c8rc
There was a problem hiding this comment.
I thought I had seen that amazonq also output the table, but it doesn't. It seems to work just fine with the c8rc. Thanks
Which paths need updated? The only thing I can think of is deleting core's c8rc. Not sure if there would be side effects from this or if anything else needs updating. I figured I would leave this investigation for future work. |
jpinkney-aws
left a comment
jpinkney-aws
left a comment
There was a problem hiding this comment.
I think the only other thing we would need to do is:
- delete core's .c8rc.json
- remove ./coverage/core/lcov.info instances in the ci (both github and codebuilds):
It looks like we future proofed ourselves and already upload toolkits lcov.info if it exists: https://github.com/aws/aws-toolkit-vscode/blob/master/.github/workflows/node.js.yml#L103
|
I will create an AI to address these |
Problem: c8 was not given a reporter, therefore it defaulted to `text` which is the terminal. Solution: Set it to `lcov`, which is what we have been using previously. Note: More work may be needed on the .c8rc.json files. We might not need the one for core/ anymore. They may not be configured in the optimal way. Is it behaving how we want it to? --- <!--- REMINDER: Ensure that your PR meets the guidelines in CONTRIBUTING.md --> License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
3 participants
Problem: c8 was not given a reporter, therefore it defaulted to
textwhich is the terminal.Solution: Set it to
lcov, which is what we have been using previously.Note: More work may be needed on the .c8rc.json files. We might not need the one for core/ anymore. They may not be configured in the optimal way. Is it behaving how we want it to?
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.