Fix DependencyGraph dag extra render #2749
Conversation
Signed-off-by: Michael Dwyer <[email protected]>
Signed-off-by: Michael Dwyer <[email protected]>
| wrapper.instance().handleMatchCountChange(matchCount); | ||
| expect(wrapper.state('matchCount')).toBe(matchCount); | ||
| }); | ||
| it('passes computed match count to DAGOptions based on uiFind and dependencies', () => { |
There was a problem hiding this comment.
Deleted a few tests that no longer made sense after these changes and replaced with this new test
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2749 +/- ##
==========================================
- Coverage 96.86% 96.84% -0.03%
==========================================
Files 256 256
Lines 7951 7951
Branches 2001 2079 +78
==========================================
- Hits 7702 7700 -2
- Misses 249 250 +1
- Partials 0 1 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@hari45678 please review |
There was a problem hiding this comment.
Great work migrating this file to typescript
|
The new changes doesn't seem to break any previous functionality |
yurishkuro
added
the
changelog:bugfix-or-minor-feature
|
I merged main hoping it will help with coverage, but it didn't. Are the new tsx tests actually running? I didn't find them by name in the CI run log. |
|
Its weird actually because it shows locally. New tsx tests are indeed running Locally it shows: Number of tests passed in CI And locally |
|
In CI also |
|
my bad, I see it in CI too but then it needs to have more test to increase code coverage, it went down. |
|
Correct. @mdwyer6 PTAL |
Signed-off-by: Michael Dwyer <[email protected]>
Signed-off-by: Michael Dwyer <[email protected]>
|
@hari45678 I've added code coverage |
## Which problem is this PR solving? jaegertracing#2715 ## Description of the changes - Removes unneeded useEffect hook and simplifies logic - Converts DependencyGraph/index.jsx to TS because above change had moved typed functions to an untyped file ## How was this change tested? - Generated dependencies locally with HotRod and validated visually + logged render count to verify DAG component no longer double renders - Passes existing tests (some unit tests were updated) ## Checklist - [X] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [X] I have signed all commits - [X] I have added unit tests for the new functionality - [X] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Michael Dwyer <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
3 participants
Which problem is this PR solving?
#2715
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test