-
Notifications
You must be signed in to change notification settings - Fork 85
Handle IS_TEST param for fides-js builds to include test ids #6382
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6382 +/- ##
=======================================
Coverage 86.68% 86.68%
=======================================
Files 453 453
Lines 28938 28938
Branches 3232 3232
=======================================
Hits 25086 25086
Misses 3136 3136
Partials 716 716 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
🚀 🚀 🚀
fides
|
Project |
fides
|
Branch Review |
main
|
Run status |
|
Run duration | 01m 01s |
Commit |
|
Committer | Nate Smith |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
5
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
Closes ENG-1035
Description Of Changes
Adds back missing data-testid attributes in FidesJS components for development Docker builds. The issue occurred because FidesJS builds were stripping test IDs even in dev environments, making some e2e Privacy Center tests break.
The solution adds an IS_TEST build argument that preserves test IDs only for dev builds while maintaining the current behavior (stripped test IDs) for all production builds.
Code Changes
Steps to Confirm
docker build --build-arg IS_TEST=true --target=prod_pc -t test-privacy-center-with-testids .
That will show you the build command being run. For extra thoroughness, you could take the command, remove the ''--push" flag, and then build it locally as well.
I'll also pull down the dev image when it gets published after this is merged so I can double check.
Pre-Merge Checklist
CHANGELOG.md
updatedmain
downgrade()
migration is correct and works