-
Notifications
You must be signed in to change notification settings - Fork 11
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
Added major Tagged Test Filter configurations #639
Conversation
7011fcd
to
b519f40
Compare
Npm packages
|
.github/workflows/ci.yml
Outdated
@@ -6,7 +6,9 @@ on: | |||
tags: | |||
- "*" | |||
pull_request: | |||
|
|||
schedule: # Scheduled nightly and weekly jobs |
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.
Will this schedule also deploy the job? Either we should skip the deployment or create a new workflow with just the Cypress tests running.
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.
@hussainweb Creating a new separate workflow with just Cypress tests is a good idea since the above triggers all the ci.yml workflow.
export default defineConfig({ | ||
projectId: 'contrib-tracker', | ||
chromeWebSecurity: false, // To Bypass Same-Origin Policy - Useful for social-media-link-check test | ||
video: process.env.CYPRESS_VIDEO !== 'false', // Defaults to true unless explicitly set to 'false' | ||
e2e: { | ||
baseUrl: process.env.CYPRESS_BASE_URL || 'https://contrib.axelerant.com', // Default to prod url if no base URL is set | ||
env: { | ||
grepTags: 'expensive' |
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.
What does this line do? From my limited knowledge, it seems it will assign all tests this tag which seems incorrect. Please clarify.
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.
@hussainweb This line sets a default value for grepTags as expensive in the Cypress environment. This means, unless overridden, Cypress will only run tests that have been specifically tagged - expensive.
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.
@Yemaneberhan-Lemma, I am coming back to this after a while. Can you look at all the open comments? And why did you split the tests into two different workflows? It seems that you are doing a lot more here than just tagging tests.
@hussainweb, I moved Cypress-related processes into a separate workflow to improve maintainability and scalability by isolating test execution, enabling parallel runs, simplifying debugging, and offering better control. Also based on previous comment . |
.github/workflows/ci.yml
Outdated
web/themes/custom/contribtracker/cypress/screenshots | ||
web/themes/custom/contribtracker/cypress/videos | ||
retention-days: 3 | ||
#- name: Change admin password to make it easier for Cypress tests |
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.
Why comment out everything?
Also, let's discuss the test execution times. We still want to run some tests in the regular CI flow.
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.
Why comment out everything?
Also, let's discuss the test execution times. We still want to run some tests in the regular CI flow.
Created a separate workflow for it.
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.
There are a few spacing issues and one of the files has mixed import/require usage. I'll push changes to fix that.
- name: Cypress Test | ||
run: ddev cypress-run | ||
- name: Cypress Test (excluding expensive tests) | ||
run: ddev cypress-run --env grepTags=-expensive |
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.
Nice!
@Yemaneberhan-Lemma, I'm happy to finally merge this one. One thing that now remains is to meaningfully cut the list of tests. Right now, only one test gets skipped and it was not really expensive anyway. The last run on the main branch and a run here both took 1m 45s. |
For this to function, we need to have tagged test scripts.