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

[engsys] Fix rush-runner #32392

Merged
merged 3 commits into from
Dec 31, 2024
Merged

[engsys] Fix rush-runner #32392

merged 3 commits into from
Dec 31, 2024

Conversation

xirzec
Copy link
Member

@xirzec xirzec commented Dec 31, 2024

Packages impacted by this PR

All packages using CI

Describe the problem that is addressed by this PR

@qiaozha noticed that tests were no longer failing the build. After some investigation, I realized that PR #32148 introduced a regression with how the process exitCode was being propagated. Previously, we were setting it globally via process.exitCode where we spawn the node process, but this was refactored to use return codes.

However, the return code was never used when the task was anything except lint and check-format meaning that build and test operations did not report failure correctly.

In the future, we should figure out a better way to abstract and test this script's return codes, but I'd like to get a fix in to prevent bad PRs from being merged in the meantime.

@xirzec xirzec added the EngSys This issue is impacting the engineering system. label Dec 31, 2024
@xirzec xirzec requested review from jeremymeng and timovv December 31, 2024 15:03
@xirzec xirzec requested a review from sarangan12 as a code owner December 31, 2024 15:22
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Member

@jeremymeng jeremymeng left a comment

Choose a reason for hiding this comment

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

Oops. Thanks for the fix!

@jeremymeng
Copy link
Member

communication-sms unit-tests failing as expected

@jeremymeng
Copy link
Member

/check-enforcer override

@jeremymeng jeremymeng merged commit b169bc9 into Azure:main Dec 31, 2024
14 checks passed
@xirzec xirzec deleted the fixRushRunner branch December 31, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EngSys This issue is impacting the engineering system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants