-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
ci: Support for limited tests run from PR description #34736
Conversation
WalkthroughThe recent changes across the GitHub workflows and scripts focus on refining the handling of tag parsing, adding output specifications, and enhancing the cypress test configuration. These updates streamline the tag parsing process, spec inclusion, and test specification handling for more efficient and insightful CI/CD operations. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .github/workflows/pr-automation.yml (4 hunks)
- .github/workflows/pr-cypress.yml (3 hunks)
- .github/workflows/scripts/test-tag-parser.js (2 hunks)
Additional comments not posted (12)
.github/workflows/pr-automation.yml (3)
22-22
: LGTM! Addingspec
to outputs.The addition of
spec
to the outputs section is correct.
46-54
: LGTM! Updating the checkAll step to handlespec
.The modifications correctly handle the presence of
spec
and adjust the matrix accordingly.
68-68
: LGTM! AddingSpec
to the output details.The addition of
Spec
to the output details is correct..github/workflows/scripts/test-tag-parser.js (6)
2-3
: LGTM! Handling predictable newlines.Replacing carriage return newlines with predictable newlines improves consistency.
4-7
: LGTM! Handling empty payload body.The code correctly handles the case where the body is empty by setting the action to failed.
23-24
: LGTM! Setting outputs for tags and specifications.The code correctly sets the outputs for tags and specifications based on the parsing results.
9-20
: LGTM! Updating error handling.The updated error handling improves the clarity of error messages and provides useful links for documentation.
31-43
: LGTM! Refactoring the parseTags function.The refactored
parseTags
function improves the parsing logic and adds support for code fences.
95-100
: LGTM! Adding matchCodeFence function.The
matchCodeFence
function correctly matches code fences in the body content to extract specifications..github/workflows/pr-cypress.yml (3)
Line range hint
6-10
:
LGTM! Addingspec
to inputs.The addition of
spec
to the inputs section is correct.
Line range hint
81-81
:
LGTM! Addingspec
to the perform-test job.The addition of
spec
to theperform-test
job is correct.
197-197
: LGTM! Addingspec
to the output details.The addition of
spec
to the output details is correct.
@sharat87 I am not sure if I got the message in the PR description correctly. The question is -- we're getting the message that "no tests ran", is that expected because we're only running individual specs right now? I don't see a problem if this is actually the case, as only power users would use the spec list way of testing and would very likely know that the "no tests ran" message is a consequence of not running tags. |
@sharat87 one small thing, can we use less machines based on number of specs to run? we have reduced to only one machine for |
Thanks for checking this out folks! ❤️ @riodeuno, I should've mentioned this, please ignore the Cypress report part of the PR description. It's not behaving correctly because there's changes in @ApekshaBhosale, yes, indeed. This is how Let me know what you guys think! 🙏 |
Specify the specs we want to run, in a plain code fence, where the first line has to exactly be
/test
. The remaining lines have to be one spec file per line.Like this:
spec
content, almost directly lands into the--spec
argument of Cypress. See docs at https://docs.cypress.io/guides/guides/command-line#cypress-run-spec-lt-spec-gt.ci-test-limited.yml
also doesn't do multiple runners either.Warning
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9803353429
Commit: e99ae38
Cypress dashboard.
Tags:
Spec: cypress/e2e/Regression/ClientSide/Fork/ForkApplication_spec.ts
cypress/e2e/Regression/ClientSide/JSLibrary/Library_spec.ts
It seems like no tests ran 😔. We are not able to recognize it, please check workflow here.
Fri, 05 Jul 2024 05:20:23 UTC
Summary by CodeRabbit