docs(adr): add Percy automation ADR#515
Conversation
|
There was a problem hiding this comment.
Pull request overview
Adds an ADR documenting the decision to automate Percy visual regression testing with partial builds in GitHub Actions.
Changes:
- Introduces ADR 0002 describing Percy automation approach (partial PR builds + full main builds).
- Documents dependency tracking via
component-metadata.jsonand expected tradeoffs.
| - Using the existing `component-metadata.json` mappings (component and submodules fields) to determine snapshot dependencies | ||
| - Triggering full builds only when global files change (tokens, variables, mixins) | ||
|
|
||
| 2. **Full builds for main commits**: Execute and auto-approve the complete snapshot suite on the main branch | ||
|
|
||
| 3. **Simplified dependency tracking**: Re-use existing component metadata instead of automated code analysis (e.g., mixin dependency detection), as the additional complexity provides minimal benefit |
There was a problem hiding this comment.
For clarity/consistency in an ADR, consider formatting field names as code (e.g., component and submodules) and using “Reuse” instead of “Re-use” (common spelling in most style guides).
| - Using the existing `component-metadata.json` mappings (component and submodules fields) to determine snapshot dependencies | |
| - Triggering full builds only when global files change (tokens, variables, mixins) | |
| 2. **Full builds for main commits**: Execute and auto-approve the complete snapshot suite on the main branch | |
| 3. **Simplified dependency tracking**: Re-use existing component metadata instead of automated code analysis (e.g., mixin dependency detection), as the additional complexity provides minimal benefit | |
| - Using the existing `component-metadata.json` mappings (`component` and `submodules` fields) to determine snapshot dependencies | |
| - Triggering full builds only when global files change (tokens, variables, mixins) | |
| 2. **Full builds for main commits**: Execute and auto-approve the complete snapshot suite on the main branch | |
| 3. **Simplified dependency tracking**: Reuse existing component metadata instead of automated code analysis (e.g., mixin dependency detection), as the additional complexity provides minimal benefit |
agliga
left a comment
There was a problem hiding this comment.
I would call the file 002-visual-regression-testing
If it ever gets superceeded its hard to see what superceeded it.
I also would add the problem not being about the snapshots but rather we need to test visual regression on our css code.
I think being generic is key with these adrs
I like the steps part and such.
I would add also other frameworks we looked at if we did any why we didn't pick those.
|
So we already have a 0002: #509. Even if that one gets rejected, it will stay as 0002 |
My understanding was that this ADR was about automating Percy visual regression, not about visual regression itself as we decided on that already.
Agree that it should be generic, but as mentioned above I think it should be about the automation so |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
That was my understanding too. |
Hmm I think there's a disconnect here. What prompted in writing this ADR is that we wanted to show that we chose Percy visual regression testing over Vitest snapshot or whatever other testing framework. If we keep it as is, Im wondering then what is the point of this ADR.
Our goal is always to automate as many things as we can. The assumption is that we try to automate everything. It would be like adding changesets instead of just publishing manually. We don't need an ADR for that, we just do it because our goal is to automate everything we can... |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@agliga make sense, and I agree. I have updated the ADR with the information regarding the visual-html that we discussed in the past. Can you check if I missed any item to be added? |
agliga
left a comment
There was a problem hiding this comment.
To me this looks great! I think its almost there.
I think there's a couple of minor edits, but Im fine approving after this.
I would also remove any mention of eBay as a whole.
We should talk about this from an open source perspective.
Co-authored-by: Andrew Gliga <agliga4@gmail.com>
Co-authored-by: Andrew Gliga <agliga4@gmail.com>
ArtBlue
left a comment
There was a problem hiding this comment.
Looks great. Just a couple of minor things.
| Key factors in this decision: | ||
|
|
||
| 1. **Existing infrastructure**: We already have an existing Percy account, eliminating procurement and setup overhead | ||
| 2. **Cross-browser testing**: Percy provides testing across multiple browsers (Chrome, Firefox, Safari, Edge), which visual-html does not natively support |
There was a problem hiding this comment.
There are a couple of other factors IMO.
- Percy has responsive testing out of the box, which also has specific mobile browser testing
- Percy is actively being maintained and improved, so we should have more abilities to scale and automate in the future.
Reintroduce the original operational context and implementation details for Percy automation, including partial PR builds, full main builds, and metadata-based dependency tracking. Capture the key cost, speed, and maintenance tradeoffs without duplicating rationale across sections. Co-authored-by: Cursor <cursoragent@cursor.com>
6651a0b
Description
Created ADR following changes from #414