Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Jan 22, 2026

Summary

This PR uses the node:test package to assert expected cases in place of the amazing mocha and chai packages β˜• 🫑

Notes

Also removed are the kind packages adjacent: πŸ’Œ

Coverage is changed from c8 to --experimental-test-coverage which is checking both "src" and "test" directories but updates for #487 might bring options to change this also πŸ§ͺ

Requirements

@zimeg zimeg added this to the 2.2 milestone Jan 22, 2026
@zimeg zimeg self-assigned this Jan 22, 2026
@zimeg zimeg requested a review from a team as a code owner January 22, 2026 06:52
@zimeg zimeg added dependencies Pull requests that update a dependency file semver:patch tests labels Jan 22, 2026
@zimeg
Copy link
Member Author

zimeg commented Jan 22, 2026

πŸ”¬ Hmm... I'm not sure coverage reports are generated as noted in codecov/feedback#107 for Node 20?

info - 2026-01-22 06:58:51,703 -- Found 0 coverage files to report

πŸ”— https://github.com/slackapi/slack-github-action/actions/runs/21239153963/job/61113138905?pr=538#step:8:260

Perhaps this is saved as a draft PR for now, or coverage is checked otherwise in the meantime. Open to suggestions!

package.json Outdated
"lint:fix": "biome check --write",
"lint": "biome check",
"test": "c8 mocha test/*.spec.js",
"test": "node --test --experimental-test-coverage test/*.spec.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome πŸ’― it would allow us to reduce our dependencies

My only concern here is that --experimental-test-coverage is still experimental and subject to breaking changes and potential removal, I haven't been able to find a stable target release date for it.

I'm not opposed to testing this out on this project but we need to be ready to potentially roll this back if the feature changes or gets removed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WilliamBergamin I share this concern but think adjusting with those changes might be nice to prefer fewer packages installed 🎁

A commit 41a3be0 was added for generating coverage reports for the @codecov steps which might help avoid regression in PRs too! IIRC this'll be running after a merge to main but I'll report back πŸ‘Ύ

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ‘οΈβ€πŸ—¨οΈ Hmm a fast follow was needed to upload the coverage report instead of test reports! #539

@zimeg
Copy link
Member Author

zimeg commented Jan 24, 2026

@WilliamBergamin I share the excitement - these are nice defaults to use! Thanks so much for the review too 🚒 ✨

Let's merge this now forgone updates of #533 and #536 πŸ™

@zimeg zimeg merged commit a47c150 into main Jan 24, 2026
6 checks passed
@zimeg zimeg deleted the zimeg-test-node-runner branch January 24, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file semver:patch tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants