Skip to content

Conversation

cacieprins
Copy link
Contributor

  • Closes

Additional details

While the line count of the main workflows.yml has increased quite a bit, this is because circleci config pack unrolls/expands all anchors in the source yml files.

circleci config pack uses the FYAML convention to be able to break yaml files into smaller, easier to digest chunks.

This PR:

  • migrates workflows.yml to .circleci/src/workflows
  • defines a new pull-request workflow, which at the moment is identical to linux-x64 except it does not include references to any of the jobs excluded by the main build filters
  • migrates config.yml to .circleci/src/config
  • adds a husky pre-commit hook to pack directories in .circleci/src to a yaml file defined by the directory name in .circleci/
  • additionally, runs circleci config validate on these packed yml files, to ensure that they are valid circleci configs

Steps to test

How has the user experience changed?

PR Tasks

@cacieprins cacieprins marked this pull request as draft September 10, 2025 19:43
@cacieprins cacieprins marked this pull request as ready for review September 12, 2025 15:29
Copy link

cypress bot commented Sep 12, 2025

cypress    Run #65601

Run Properties:  status check passed Passed #65601  •  git commit eab6571804: Merge commit 'stash' into use-pack-for-circle
Project cypress
Branch Review use-pack-for-circle
Run status status check passed Passed #65601
Run duration 21m 43s
Commit git commit eab6571804: Merge commit 'stash' into use-pack-for-circle
Committer cypress-bot[bot]
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 28
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 791
View all changes introduced in this branch ↗︎
UI Coverage  62.34%
  Untested elements 27  
  Tested elements 48  
Accessibility  99%
  Failed rules  0 critical   3 serious   1 moderate   0 minor
  Failed elements 18  

@@ -278,7 +278,7 @@ describe('open', () => {
errCb(err)

expect(process.exit).toHaveBeenCalledWith(1)
// eslint-disable-next-line no-console

Copy link
Contributor Author

Choose a reason for hiding this comment

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

linting was failing without these changes

value: << pipeline.git.branch >>
- not:
matches:
pattern: /^pull\/[0-9]+/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

contributor prs still use the contributor linux-x64 workflow, for now. for an idea about how we can simplify that: #32494

Copy link
Member

Choose a reason for hiding this comment

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

that would be nice because not everything runs the same. It'd be nice to have that logic more clearly separated out.

@@ -0,0 +1,2844 @@
version: 2.1

chrome-stable-version: &chrome-stable-version "140.0.7339.127"
Copy link
Contributor

@AtofStryker AtofStryker Sep 16, 2025

Choose a reason for hiding this comment

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

The github action likely needs to be updated that was originally updating the .circleci/workflow file for the latest browser version. We are also using the yaml package to format the config. I did some work to port it over in https://github.com/cypress-io/cypress/pull/30989/files so hopefully that paints enough context if/what needs to change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the scripts & left a comment that updating them is necessary in 5fb5125

Copy link
Member

Choose a reason for hiding this comment

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

Might just want to verify this works after merge. I did run it, but there's no browser version updates so it didn't trigger the whole thing. https://github.com/cypress-io/cypress/actions/runs/17800753588

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

@cacieprins Can you add this branch to run the extended windows, binary, etc tests to ensure that workflow still works the same when you want it to run on your PR?

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

I went through and compared the jobs run + required job chain in CircleCI on previous branches to this one. Everything is running the same (although I wasn't able to verify that the 'ready-to-release' job will run since the dependent jobs have not all passed at any point.)

@@ -0,0 +1,49 @@
resource_class: small
Copy link
Member

Choose a reason for hiding this comment

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

This strangely maxes out its resources checking out code. I wonder if this could be sped up with more. (I realize you didn't change this from before). https://app.circleci.com/pipelines/github/cypress-io/cypress/74784/workflows/2d0ba1d8-892a-4aee-9cac-b0d07ffd0700/jobs/3142881/resources

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, probably! We could switch this to medium. We could also do some custom git credential stuff and do a super shallow checkout - all we need is the target workflow file. The baked in checkout command does a pretty good job of doing the minimal, but it can't carve it down that far.

I'd suggest we just bump it to medium, if we'd like to speed it up. There are much lower hanging fruit in the main pipeline for cost/performance.

Comment on lines +7 to +9
echo "❌ Warning: CircleCI CLI not found! Install the CircleCi CLI to edit the CircleCI configuration."
exit 1
fi
Copy link
Member

Choose a reason for hiding this comment

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

Verified this works
Screenshot 2025-09-17 at 10 41 58 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

Also fails if errors packing after install
Screenshot 2025-09-17 at 5 08 56 PM

Comment on lines +26 to +31
echo " 🔍 Validating ${output_file}"
if ! circleci config validate "$output_file"; then
echo " ❌ Validating ${output_file} failed"
exit 1
fi
echo " ✅ ${dirname}.yml configuration validated successfully"
Copy link
Member

Choose a reason for hiding this comment

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

Cool

Screenshot 2025-09-17 at 10 47 26 AM

@jennifer-shehane
Copy link
Member

@cacieprins Are these meant to run on the full run? They run on develop on the linux job, but not on this run with the 'full-workflow. (Not saying it necessarily should, just wanting to confirm what we want as the behavior here)

Also, it seems this branch is running both a linux-x64 workflow and a pull-request workflow, which is duplicating every running test.

Screenshot 2025-09-17 at 10 54 20 AM Screenshot 2025-09-17 at 10 56 52 AM

@jennifer-shehane jennifer-shehane self-requested a review September 17, 2025 15:32
Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

Perhaps revisit something to check in CI if someone skips the precommit hooks

Comment on lines +7 to +9
echo "❌ Warning: CircleCI CLI not found! Install the CircleCi CLI to edit the CircleCI configuration."
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Also fails if errors packing after install
Screenshot 2025-09-17 at 5 08 56 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants