Add automated E2E code coverage for client applications
Description Of Changes
Experimenting with using istanbul, @cypress/code-coverage, and codecov together to pull automated coverage reports for the client codebases 👍
Code Changes
- [X] Add
istanbulplugin toadmin-uiandprivacy-centerprojects, but only enable it in "test" builds used for Cypress (ie.cy:start) - [X] Use Cypress'
code-coverageplugin to publish coverage reports while running specs - [X] Update GitHub CI (
frontend_checks.yml) to collect coverage reports and upload to CodeCov - [X] Modify CodeCov actions to tag
pytestcoverage with a "backend" flag - [X] Modify CodeCov configuration with separate coverage targets for "backend" vs "admin-ui" vs "privacy-center"
Steps to Confirm
- [X] View the CodeCov checks attached to this PR:
Pre-Merge Checklist
- [X] All CI Pipelines Succeeded
- Documentation:
- [X] documentation complete, PR opened in fidesdocs
- [X] documentation issue created in fidesdocs
- [X] Issue Requirements are Met
- [X] Relevant Follow-Up Issues Created
- [X] Update
CHANGELOG.md - [X] For API changes, the Postman collection has been updated
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| fides-plus-nightly | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Jan 29, 2024 10:40pm |
Passing run #6098 ↗︎
| ⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results. | ||||
Details:
| Merge 30a866a7effcaaac8c97d6b96789cdc783e4dca6 into acc2541a03e872c1dce86ec8cf5a... | |||
| Project: fides | Commit: 8fe49616b0 ℹ️ |
||
| Status: Passed | Duration: 00:34 💡 | ||
| Started: Jan 29, 2024 10:49 PM | Ended: Jan 29, 2024 10:50 PM | ||
Review all test suite changes for PR #4563 ↗︎
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 78.52%. Comparing base (
13a040e) to head (30a866a). Report is 657 commits behind head on main.
:exclamation: There is a different number of reports uploaded between BASE (13a040e) and HEAD (30a866a). Click for more details.
HEAD has 12 uploads less than BASE
Flag BASE (13a040e) HEAD (30a866a) 12 0
Additional details and impacted files
@@ Coverage Diff @@
## main #4563 +/- ##
==========================================
- Coverage 86.79% 78.52% -8.28%
==========================================
Files 329 824 +495
Lines 19816 32088 +12272
Branches 2546 6215 +3669
==========================================
+ Hits 17199 25196 +7997
- Misses 2150 6361 +4211
- Partials 467 531 +64
| Flag | Coverage Δ | |
|---|---|---|
| admin-ui | 64.65% <ø> (?) |
|
| backend | 86.76% <ø> (?) |
|
| privacy-center | 69.62% <ø> (?) |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Amazing! A few comments:
- Do you know if there's a way to also instrument our unit tests and combine coverage with cypress? We have some unit tests—not a lot, but some that cover nitty gritty logic of helper funcs—which would be nice to have covered as well. Though again, there aren't that many (more so in fides.js) so I don't think this is a high priority
Yes, that's possible. For the pytest coverage we upload multiple reports to codecov and it aggregates it all together - although I find that kinda hard to "grok". A simpler way would be to run the unit tests & cypress tests from the same job so that the coverage report would aggregate locally on the GitHub runner and then upload together.
- I wonder if this can work to cover fides.js at all, given that its tests are in a different folder....
I tried really briefly to do this. It was easy enough to add rollup-plugin-istanbul to fides.js, but had a couple issues:
- it immediately explodes the bundle size and our build pipeline was very upset with me
- coverage reports didn't work when I ran Cypress tests from
/clients/privacy-center; I have to assume it was confused by directories saying/clients/fides-jsor something
Overall I punted on it pretty quickly.
- I never added tests to the Regulations page (I know, I know! I'm a monster, but time was short and I'll do it as part of https://ethyca.atlassian.net/browse/PROD-1563 😄 ). it uses many of the same components as the Locations page, but I noticed that
regulations.tsxdoesn't show up in the code coverage report at all, probably because no cypress test visits it. So just flagging that our % may be inflated if there are parts of the app that aren't covered, but are also never visited by cypress, it looks like they just won't be included, as opposed to lowering our %
Interesting... I'd have hoped that coverage reports should include all files, especially those that aren't exercised by tests. I wonder if this might be caused less by the lack of a Cypress test and more if cy:start doesn't force those files to actually get built.
- overall, great increment! :shipit:
😄
Just pushed an update that'll now pull all the source files into the coverage report, and fully untested files show up as semi-transparent entries with 0% coverage:
However, it's still misleading - Istanbul doesn't include these untested files in the total coverage statistics, which you can basically see in that screenshot above... it's showing "3/14 lines" covered but that's only because it's not including any of the untested files and all their LoC! This isn't just us, it seems like an open issue with Istanbul in general: https://github.com/istanbuljs/nyc/issues/1415
...overall I hesitate to merge something in that is actively misleading because it doesn't include untested files... 🤔.