test: Add permissions and roles e2e tests #4172
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
github-actions
bot
added
front-end
Uffizzi Ephemeral Environment
|
novakzaballa
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4172 +/- ##
========================================
Coverage 97.38% 97.39%
========================================
Files 1189 1190 +1
Lines 41420 41527 +107
========================================
+ Hits 40338 40445 +107
Misses 1082 1082 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
That was a lot of code to review, @kyle-ssg. Overall things look good. I've included some comments but I'll leave the final call on them to your discretion. Also, it looks like CI is currently failing, but I'll leave that up to you to fix.
frontend/e2e/init.cafe.js
Outdated
| // test('Segment-part-1', async () => { | ||
| // await testSegment1() | ||
| // await logout() | ||
| // }) |
There was a problem hiding this comment.
Here and below, many tests are commented out. Why?
There was a problem hiding this comment.
Sorry this is an old pr that I'm still reviving, should have been marked as draft
frontend/e2e/init.cafe.js
Outdated
| // | ||
| // test('Environment-permission', async () => { | ||
| // await environmentPermissionTest() | ||
| // await logout() | ||
| // }) |
There was a problem hiding this comment.
Another commented out test.
| click, createEnvironment, | ||
| log, |
There was a problem hiding this comment.
Shouldn't createEnvironment be on its own line?
| } from '../helpers.cafe'; | ||
| import { | ||
| PASSWORD, | ||
| E2E_NON_ADMIN_USER_WITH_PROJECT_PERMISSIONS, E2E_NON_ADMIN_USER_WITH_ENV_PERMISSIONS |
There was a problem hiding this comment.
nit: Shouldn't E2E_NON_ADMIN_USER_WITH_ENV_PERMISSIONS be on its own line?
| }) | ||
| }} | ||
| } | ||
| ds |
There was a problem hiding this comment.
I assume ds is a custom prop? If so, a more descriptive name would be good.
| <div> | ||
| <IdentifierString | ||
| value={ | ||
| (identity && identity.identity.identifier) || id |
There was a problem hiding this comment.
Calling identity.identity looks weird / wrong.
Thanks for submitting a PR! Please check the boxes below:
pre-committo check lintingdocs/if required so people know about the feature!Changes
Add e2e test for permissions and roles.
Closes #3854
How did you test this code?
We needed the branch test: seed data for permission and roles e2e tests to test it