Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace some workflow tests with Node test suite #542

Merged
merged 1 commit into from
May 23, 2024
Merged

Replace some workflow tests with Node test suite #542

merged 1 commit into from
May 23, 2024

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented May 22, 2024

A number of our workflows use write tokens for integration tests. This isn't great and prevents third-party contributions, so let's replace that with tests that we can mock API calls and other benefits such as code coverage monitoring.

Replaced tests

Action Status
check-commit-format ✅ Could do with better coverage but is no worse than what we had before
dismiss-approvals
label-pull-requests ✅ Could do with better coverage but is no worse than what we had before
post-comment

Still integration tests, but in good state

Action Status
brew-script Composite action that executes Ruby. Unchanged as the existing workflow doesn't need sensitive tokens
git-try-push Executes commands so messier to mock. Need for write tokens when testing removed in #541
git-user-config Executes commands. Unchanged as the existing workflow doesn't need sensitive tokens
setup-commit-signing Shell script so can't be integrated into the Node test suite. Need for secrets when testing removed in #540
setup-homebrew Shell script. Unchanged as the existing workflow doesn't need sensitive tokens and is fairly comprehensive as is

Note that these will not have code coverage monitored as a result.

No tests; need attention

Action Status Recommendation
bump-formulae Deprecated but stable.
Any bump-package test coverage would cover this one
Add tests to bump-packages
bump-packages Composite action. No tests Figure out how to test this
create-gcloud-instance Shell scripts. No tests Will deprecate this soon
failures-summary-and-bottle-result Composite action. No tests Figure out how to test this
find-related-workflow-run-id Uses gh. No tests Migrate this to use octokit.js
limit-pull-requests Uses gh. No tests Migrate this to use octokit.js
post-build
pre-build
Composite action.
No tests and has a history of "blind changes"
Move this to homebrew-core
remove-disabled-formulae Uses Ruby, executes commands. No tests Figure out how to test this
wait-for-idle-runner Can easily be added to the Node test suite
but isn't expected to stay around for long
Will deprecate this soon

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Nice work, makes sense to me! Haven't done detailed review of the NodeJS code but approach is sensible.

@Bo98 Bo98 merged commit 1b4aed5 into master May 23, 2024
17 checks passed
@Bo98 Bo98 deleted the node-test branch May 23, 2024 14:11
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants