-
Notifications
You must be signed in to change notification settings - Fork 527
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
Monitor Tab: Cannot choose another timeframe #898
Conversation
Codecov Report
@@ Coverage Diff @@
## main #898 +/- ##
==========================================
+ Coverage 95.43% 95.47% +0.04%
==========================================
Files 240 240
Lines 7471 7475 +4
Branches 1867 1868 +1
==========================================
+ Hits 7130 7137 +7
+ Misses 335 332 -3
Partials 6 6
Continue to review full report at Codecov.
|
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.
LGTM, thanks for fixing! Curious, is there a test we could write to prevent this from happening again?
Signed-off-by: nofar9792 <nofar.cohen@logz.io>
…er-ui into fix-timeframe-bug
@albertteoh i added the test + small change in the implementation |
@yurishkuro @albertteoh I don't understand why the test coverage decreased. |
I don't understand the root cause either, but I have observed flakiness in codecov in the past with jaeger-ui PRs. The workaround was to close and re-open the PR to see if it resolves the issue, which I'll try now for you. I'd be keen to learn if anyone else has insights on why the codecov check is failing. |
FWIW, a link to what codecov thinks is where the coverage regression exists, which doesn't seem to be related to this PR: https://app.codecov.io/gh/jaegertracing/jaeger-ui/compare/898/changes. Any ideas @yurishkuro? |
Which problem is this PR solving?
Resolve (#897)
before:
Screen.Recording.2022-02-27.at.14.13.10.mov
after:
Screen.Recording.2022-02-27.at.14.14.19.mov
The issue was the calculation of the timeframe. I did in the constructor and not on every new state