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

[code-infra] Run react-17 and react-next workflows on the next branch #42690

Merged
merged 16 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -888,48 +888,106 @@ workflows:
jobs:
- test_profile:
<<: *default-context

# This workflow can be triggered manually on the PR
react-17:
when:
equal: [react-17, << pipeline.parameters.workflow >>]
jobs:
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to duplicate the jobs here, I couldn't find a way to reuse them in CircleCI (the experience of configuring Circle CI is not great TBH 😅).

The whole point is that:
react-17 workflow can be triggered manually on any branch (e.g. in a PR) on demand through Circle CI.
react-17-cron workflow is triggered by a cron job (same as before this PR).
Same for react-next workflows.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's not great. Perhaps an orb could fix duplication here, but I don't really mind it so much.

- test_unit:
<<: *default-context
react-version: ^17.0.0
name: test_unit-react@17
- test_browser:
<<: *default-context
react-version: ^17.0.0
name: test_browser-react@17
- test_regressions:
<<: *default-context
react-version: ^17.0.0
name: test_regressions-react@17
- test_e2e:
<<: *default-context
react-version: ^17.0.0
name: test_e2e-react@17

# This workflow is identical to react-17, but scheduled
react-17-cron:
triggers:
- schedule:
cron: '0 0 * * *'
filters:
branches:
only:
- master
- next
jobs:
- test_unit:
<<: *default-context
react-version: ^17.0.0
name: test_unit-react@17
- test_browser:
<<: *default-context
react-version: ^17.0.0
name: test_browser-react@17
- test_regressions:
<<: *default-context
react-version: ^17.0.0
name: test_regressions-react@17
- test_e2e:
<<: *default-context
react-version: ^17.0.0
name: test_e2e-react@17

# This workflow can be triggered manually on the PR
react-next:
when:
equal: [react-next, << pipeline.parameters.workflow >>]
jobs:
- test_unit:
<<: *default-context
react-version: next
name: test_unit-react@next
Copy link
Member Author

Choose a reason for hiding this comment

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

Added names here to avoid confusing job names, like this:

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure why these ghost jobs appear though:

Perhaps it's a lag on the CircleCI side.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it got in a bad state after renaming those jobs? Maybe try opening a different PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried in #42991 and the result is the same...

Copy link
Member

@Janpot Janpot Jul 22, 2024

Choose a reason for hiding this comment

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

I believe circleci reuses some existing state when opening a PR for the same commit. At least I think I've observed that before. Trying out that hypothesis in mui/mui-x#13942

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, that seems to work!
I'll trigger the react-next workflow to make sure everything works as expected.

Copy link
Member Author

Choose a reason for hiding this comment

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

All looks good now!
Any objections to merging current PR?

Copy link
Member

Choose a reason for hiding this comment

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

No, go for it 👍

Copy link
Member

@ZeeshanTamboli ZeeshanTamboli Jul 23, 2024

Choose a reason for hiding this comment

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

Seeing these empty CI jobs after merging this in #42990 and other latest PRs. What can I do?

- test_browser:
<<: *default-context
react-version: next
name: test_browser-react@next
- test_regressions:
<<: *default-context
react-version: next
name: test_regressions-react@next
- test_e2e:
<<: *default-context
react-version: next
name: test_e2e-react@next
# This workflow is identical to react-next, but scheduled
react-next-cron:
triggers:
- schedule:
cron: '0 0 * * *'
filters:
branches:
only:
- master
- next
jobs:
- test_unit:
<<: *default-context
react-version: next
name: test_unit-react@next
- test_browser:
<<: *default-context
react-version: next
name: test_browser-react@next
- test_regressions:
<<: *default-context
react-version: next
name: test_regressions-react@next
- test_e2e:
<<: *default-context
react-version: next
name: test_e2e-react@next

typescript-next:
triggers:
- schedule:
Expand All @@ -938,6 +996,7 @@ workflows:
branches:
only:
- master
- next
Copy link
Member Author

Choose a reason for hiding this comment

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

Added next branch here for the typescript-next workflow as well.

jobs:
- test_types_next:
<<: *default-context
Expand Down
11 changes: 11 additions & 0 deletions scripts/useReactVersion.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,17 @@ async function main(version) {

// add newline for clean diff
fs.writeFileSync(packageJsonPath, `${JSON.stringify(packageJson, null, 2)}${os.EOL}`);

console.log('Installing dependencies...');
const pnpmInstall = childProcess.spawn('pnpm', ['install', '--no-frozen-lockfile'], {
shell: true,
stdio: ['inherit', 'inherit', 'inherit'],
});
pnpmInstall.on('exit', (exitCode) => {
if (exitCode !== 0) {
throw new Error('Failed to install dependencies');
}
});
}

const [version = process.env.REACT_VERSION] = process.argv.slice(2);
Expand Down
27 changes: 26 additions & 1 deletion test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,11 @@ For example, in https://app.circleci.com/pipelines/github/mui/material-ui/32796/

### Testing multiple versions of React

You can check integration of different versions of React (for example different [release channels](https://react.dev/community/versioning-policy) or PRs to React) by running `pnpm use-react-version <version>`.
You can check integration of different versions of React (for example different [release channels](https://react.dev/community/versioning-policy) or PRs to React) by running:

```bash
pnpm use-react-version <version>
```

Possible values for `version`:

Expand All @@ -254,6 +258,27 @@ Possible values for `version`:

#### CI

##### Circle CI web interface

There are two workflows that can be triggered for any given PR manually in the CircleCI web interface:

- `react-next`
- `react-17`

Follow these steps:

1. Go to https://app.circleci.com/pipelines/github/mui/material-ui?branch=pull/PR_NUMBER/head and replace `PR_NUMBER` with the PR number you want to test.
2. Click `Trigger Pipeline` button.
3. Expand `Add parameters (optional)` and add the following parameter:

| Parameter type | Name | Value |
| :------------- | :--------- | :------------------------- |
| `string` | `workflow` | `react-next` or `react-17` |

4. Click `Trigger Pipeline` button.

##### API request

You can pass the same `version` to our CircleCI pipeline as well:

With the following API request we're triggering a run of the default workflow in
Expand Down