Improve coverage by adding unit tests for log.ts #2272
Conversation
k8s-ci-robot
added
approved
package.json
Outdated
| ], | ||
| "exclude": [ | ||
| "src/gen/*/**.ts", | ||
| "src/gen/**.ts", |
There was a problem hiding this comment.
I think this makes the line above it unnecessary.
There was a problem hiding this comment.
If I remove that line it prints coverage for all of the generated API files. I agree that semantically I feel like you are correct, but in practice it doesn't work right.
There was a problem hiding this comment.
Oh wait. I think it's because the existing pattern should be src/gen/**/*.ts. That fixes it for me locally.
There was a problem hiding this comment.
As far as I understand globs, the **.ts would not be different than *.s and **/*.ts would pick up all ts files including subdirs. I've actually tested it right now with ls. So this should be enough:
| "src/gen/**.ts", | |
| "exclude": [ | |
| "src/gen/**/*.ts", |
|
Comments addressed, please re-check. |
|
Thanks for the help with the glob! This is ready for (re)review |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, cjihrig The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
Largely generated by github copilot (ftw!)
Found a genuine bug where the fetch
response.bodywasn't being read as a stream.Add an exclusion for some generated code that we probably don't want to test.