test: docstring revisions [TESTENG-5] #9478
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9478 +/- ##
==========================================
- Coverage 51.18% 43.93% -7.25%
==========================================
Files 746 422 -324
Lines 110899 70932 -39967
Branches 2777 2777
==========================================
- Hits 56759 31163 -25596
+ Misses 53966 39595 -14371
Partials 174 174
Flags with carried forward coverage won't be shown. Click here to find out more. |
| */ | ||
| export class Row extends NamedComponent { | ||
| readonly defaultSelector = 'tr.ant-table-row'; | ||
| readonly keyAttribute = 'data-row-key'; |
There was a problem hiding this comment.
just one NIT, these hardcoded identifiers are in two places? what happens if someone changes the static data, will test just break? is there a better approach to have hardcoded values in one place? Just wondering, i might be totally wrong.
There was a problem hiding this comment.
hm, guess not sorry. I wasn't sure if these were being used to test the rendered ui elements so if keys for some odd reason changed would test fail.
There was a problem hiding this comment.
yeah this attribute comes from ant. perhaps an update to ant could hurt this test model, but at the point we might have to maintain even more. luckily, there would only be one test edit necessary for each ant locator change.
There was a problem hiding this comment.
overall great job! love to see more test work going on! love the time spent fixing code smells, readmes, and comments helps when reviewing.
only NITs are around hardcoded identifiers and timeouts in test that could lead to overhead test fix maintenance or flakes, but that just me.
Ticket
TESTENG-5
Description
docstring changes. i discovered a few discrepencies in page models where the original doesn't exist. i tracked them in comments in a different jira ticket im working on this sprint.
Test Plan
Checklist
docs/release-notes/.See Release Note for details.