CI: Percy Integration #467
Conversation
|
@zeshanziya, what exactly is the intent of this PR? Builds are going to Percy at the moment. What's new in this change? I am wondering if we should leave it as-is for now and focus on caching in CI for today. |
@hussainweb This PR is for Percy CI integration. There was some issue running percy using ddev. It's almost completed now. Can you please review it. last few commits were about identifying branch and commit hash for Percy. I will move focus to caching now. |
zeshanziya
force-pushed
the
percy-integration
branch
from
19f59e1
to
83db21f
Compare
.ddev/commands/cypress/percy-run
Outdated
| @@ -0,0 +1,8 @@ | |||
| #!/bin/bash | |||
| #ddev-generated | |||
There was a problem hiding this comment.
@zeshanziya, is this actually ddev-generated? If it's copy/pasted, please remove this line.
.ddev/commands/cypress/percy-run
Outdated
| @@ -0,0 +1,8 @@ | |||
| #!/bin/bash | |||
| #ddev-generated | |||
| ## Description: Use Cypress in "runner" mode. | |||
There was a problem hiding this comment.
Is this also copy/paste? I am guessing this is doing more than just running Cypress.
There was a problem hiding this comment.
Sorry, my bad. Yes, it's a copy-paste. I have updated the doc comment now.
.github/workflows/ci.yml
Outdated
| # - name: Cypress Test | ||
| # run: ddev cypress-run |
There was a problem hiding this comment.
I kept it for review. so if we just want to run cypress without percy, we can use this. I will remove it from here and add it to the documentation.
There was a problem hiding this comment.
I'd rather remove it. If you really want to keep it, I suggest writing an actual comment before ddev percy-run explaining the alternative they can swap it with.
| @@ -4,4 +4,5 @@ export default defineConfig({ | |||
| e2e: { | |||
| baseUrl: 'https://contribtracker.ddev.site/', | |||
| }, | |||
| video: true, | |||
There was a problem hiding this comment.
Do we have to explicitly say this? What about screenshots? I ask because I recall not seeing screenshots in some of the recent artifacts.
There was a problem hiding this comment.
It only captures screenshots if a test fails. From version 13, capturing video is turned off by default, so I had to enable it.
I am trying to find out what changed in version 13 that stopped screenshots from being taken for each test. I am just curious how Cypress decides when to take a screenshot for a successful test, as a test may contain multiple actions expanding to 4 to 5 seconds or more.
There was a problem hiding this comment.
I am not sure either. I remember seeing the screenshots when I ran Cypress locally. But I don't think I saw it in all the artifacts I downloaded from GitHub runs.
Anyway, this is not a blocker. If the only action item is to add a property, let's do it, otherwise, ignore this.
There was a problem hiding this comment.
No, I don't see any attributes for capturing screenshots. We can capture screenshots manually in the code using cy.screenshot if required.
Anyway, it captures videos and screenshots in case of failure, so it serves our purpose for now.
.github/workflows/ci.yml
Outdated
| # - name: Cypress Test | ||
| # run: ddev cypress-run |
There was a problem hiding this comment.
I'd rather remove it. If you really want to keep it, I suggest writing an actual comment before ddev percy-run explaining the alternative they can swap it with.
| @@ -4,4 +4,5 @@ export default defineConfig({ | |||
| e2e: { | |||
| baseUrl: 'https://contribtracker.ddev.site/', | |||
| }, | |||
| video: true, | |||
There was a problem hiding this comment.
I am not sure either. I remember seeing the screenshots when I ran Cypress locally. But I don't think I saw it in all the artifacts I downloaded from GitHub runs.
Anyway, this is not a blocker. If the only action item is to add a property, let's do it, otherwise, ignore this.
No description provided.