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

chore(workflows/pr-test): run rari alongside legacy #37766

Merged
merged 3 commits into from
Jan 23, 2025
Merged

Conversation

caugner
Copy link
Contributor

@caugner caugner commented Jan 23, 2025

Description

Merges pr-test-new-ci.yml into pr-test.yml, creating two jobs:

  • legacy (without rari), which is run if vars.TEST_LEGACY == 'true'
  • tests (with rari), which is run unless vars.TEST_SKIP_RARI == 'true'

Which job results are uploaded as artifacts depends on vars.TEST_LEGACY and vars.TEST_UPLOAD_LEGACY. If both are 'true', then the legacy test results are uploaded, otherwise the rari test results.

Note: These are repository variables, currently set as follows:

TEST_LEGACY=true
TEST_UPLOAD_LEAGCY=true
TEST_SKIP_RARI=false

Motivation

Allows us to roll out rari to all PRs, and to disable it temporarily if any issues come up.

Additional details

Part of MP-1851.

Related issues and pull requests

Related PRs:

@caugner caugner requested a review from a team as a code owner January 23, 2025 09:22
@github-actions github-actions bot added system [PR only] Infrastructure and configuration for the project size/m [PR only] 51-500 LoC changed labels Jan 23, 2025
.github/workflows/pr-test.yml Fixed Show resolved Hide resolved
.github/workflows/pr-test.yml Fixed Show resolved Hide resolved
Use variables to control which tests to run.
Introduces TEST_UPLOAD_LEGACY variable, currently set to true.
Copy link
Contributor

@argl argl left a comment

Choose a reason for hiding this comment

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

LGTM

@caugner
Copy link
Contributor Author

caugner commented Jan 23, 2025

Thanks, I'll merge in about 1h (approximately 12.15pm UTC).

@caugner caugner merged commit a250332 into main Jan 23, 2025
12 of 13 checks passed
@caugner caugner deleted the rari-pr-test branch January 23, 2025 12:13
@caugner
Copy link
Contributor Author

caugner commented Jan 23, 2025

Unfortunately, I missed the important details that repository variables aren't passed to workflows triggered by pull requests in forks, so it now runs with rari by default.

caugner added a commit that referenced this pull request Jan 23, 2025
Variables are not passed to worfklows that are triggered
by a pull request from a fork, so #37766 did
not work as intended, and rari is always used for tests.

To mitigate this, we extract a new legacy test workflow,
that we can disable, or re-enable in case fo rari issues.
caugner added a commit that referenced this pull request Jan 23, 2025
Variables are not passed to workflows that are triggered
by a pull request from a fork, so #37766 did
not work as intended, and rari is always used for tests.

To mitigate this, we extract a new legacy test workflow,
that we can disable, or re-enable in case fo rari issues.
caugner added a commit that referenced this pull request Jan 23, 2025
Variables are not passed to workflows that are triggered
by a pull request from a fork, so #37766 did
not work as intended, and rari is always used for tests.

To mitigate this, we extract a new legacy test workflow,
that we can disable, or re-enable in case of rari issues.
caugner added a commit that referenced this pull request Jan 23, 2025
Variables are not passed to workflows that are triggered
by a pull request from a fork, so #37766 did
not work as intended, and rari is always used for tests.

To mitigate this, we extract a new legacy test workflow,
that we can disable, or re-enable in case of rari issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/m [PR only] 51-500 LoC changed system [PR only] Infrastructure and configuration for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants