-
Notifications
You must be signed in to change notification settings - Fork 362
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
test: e2e refactors [TESTENG-2] #9549
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
8e4ad33
to
334b129
Compare
334b129
to
43cd240
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9549 +/- ##
==========================================
- Coverage 49.83% 45.19% -4.64%
==========================================
Files 1247 923 -324
Lines 162233 121899 -40334
Branches 2888 2888
==========================================
- Hits 80845 55093 -25752
+ Misses 81216 66634 -14582
Partials 172 172
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
This looks good. Could we even go a step further though? I'm thinking ahead a bit so some details aren't clear, but what do you think about having a common
folder housing base
, ant
, and hew
? It would be nice if when another repo comes and asks about using UI tests, we could just lift the common folder into it's own repo.
@@ -38,7 +38,7 @@ export class UserManagement extends AdminPage { | |||
selector: '[data-testid="statusSelect"]', | |||
}); | |||
readonly actions = new ActionDropdownMenu({ | |||
childNode: new BaseComponent({ | |||
clickThisComponentToOpen: new BaseComponent({ |
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.
I like this name a lot better!
@@ -1,4 +1,4 @@ | |||
import { BaseReactFragment } from 'e2e/models/BaseComponent'; | |||
import { BaseReactFragment } from 'e2e/models/base/BaseReactFragment'; | |||
|
|||
/** | |||
* Represents the PageComponent component in src/components/Page.tsx |
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.
Could this PageComponent
file be in a common
or base
folder too?
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.
nope. it's a model that represents the component from https://github.com/determined-ai/determined/blob/main/webui/react/src/components/Page.tsx
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.
since they have a Page.tsx, we have a Page.ts model.
Good idea. I think base ant and hew will all eventually live in node modules-- but for now they can live in |
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.
this isnt actually deleted, just moved
Ticket
TESTENG-2
Description
refactors some common functionality into base models
Test Plan
Checklist
docs/release-notes/
See Release Note for details.