-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
fix(ci): run Tests on Crowdin PRs
#7668
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
Conversation
Signed-off-by: Aviv Keller <me@aviv.sh>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the CI workflow to ensure that tests on Crowdin pull requests can be manually deployed via a label while avoiding recursive triggers from the repository’s GITHUB_TOKEN.
- Added support for the pull_request_target event in the workflow triggers.
- Updated conditional execution for quality checks and tests jobs based on the PR type and branch.
- Adjusted the Git Checkout step to correctly reference the commit in pull_request_target events.
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #7668 +/- ##
==========================================
- Coverage 74.61% 74.60% -0.02%
==========================================
Files 96 96
Lines 7689 7689
Branches 192 192
==========================================
- Hits 5737 5736 -1
- Misses 1950 1951 +1
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Lighthouse Results
|
bjohansebas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Bump @nodejs/web-infra to approve. No rush, obviously, this is just blocking Crowdin PRs from landing. |
|
FYI it was intentional back then to not run the tests on Crowdin PRs. |
|
That can still be arranged, would you prefer that? |
|
Personally, I prefer the alternative solution, as it's less "hacky" and more future-proof: Open Crowdin PRs from @nodejs-github-bot rather than |
|
Actually, @nodejs-github-bot is a previous contributor, do we already have a token? |
|
I'm actually going to continue with that, I like it much better. |
That one is reserved by the Build Team -- but we have one specific bot for Crowdin :) whose we have access (web infra team) |
I’d really appreciate it if someone could take a second look to ensure everything is set up correctly.
To prevent recursive triggers, events triggered by the repository’s
GITHUB_TOKEN(such as the creation of Crowdin PRs) do not trigger workflows. This wasn’t a problem before, as these workflows were manually triggered for security reasons.After switching to Codecov, these tests were automated because they are safe to run. However, automatic deployment does not occur on Crowdin PRs. Therefore, this PR introduces the option for manual deployment on Crowdin PRs via the label.