Skip to content
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

Fix broken functional tests on TSVB #31675

Closed
wants to merge 5 commits into from

Conversation

sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Feb 21, 2019

Fix #19471 .
A window size was changed to 1250x600 due to significant inconsistency of a plot rendering in CI browser with local one.
Screenshots were updated due to new size.

  • Here is a comparison from CI build when size was 1000x500:

session_baseline_comparison

  • Then comparePngs func try to compare 2 images with different sizes, and resize them to the same value in px:
    image

  • It leads to an expected failure (an inconsistency is clearly visible):
    failure_tsvb_dashboard

@sulemanof sulemanof changed the title Fix/#19471 Fix broken functional tests on TSVB Feb 21, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alexwizp
Copy link
Contributor

I see that we still have difference between our baseline image and ci image
@vitalics could you please have a look?

@stacey-gammon
Copy link
Contributor

I wonder why ci is passing? Maybe the threshold is too high. It'd be interesting to spit that number out. Honestly though, with the visual testing tools @liza-mae is looking into, I am thinking screenshot testing our own stuff at this point might be a waste of effort.

Liza, do you think the visual testing stuff will cover this kind of screenshot comparisons with our visualizations? I think so... What do you think about just ripping this stuff out?

Just don't rip out anything the reporting stuff needs, since that can't be tested with the visual testing tools.

@liza-mae
Copy link
Contributor

@stacey-gammon yes a visual testing tool can cover this. So we can skip doing it in our current way until we have a tool in place.

@flash1293
Copy link
Contributor

I'm not 100% sure but the current consensus is we don't need this at the moment, so the PR can be closed, right?

@sulemanof sulemanof closed this Mar 6, 2019
@LeeDr
Copy link

LeeDr commented Sep 20, 2019

Please keep this open until the skipped test is either fixed or replaced with a visual testing tool test.

@LeeDr LeeDr reopened this Sep 20, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@LeeDr
Copy link

LeeDr commented Sep 20, 2019

Actually, it should be an open issue, not this PR. I'll close this and reopen #19471

@LeeDr LeeDr closed this Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Time series visual builder vertical lines and times are not consistent when setting time zones
7 participants