-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Maps] update sample data functional test to use a longer time range to ensure stable results #132744
Conversation
…to ensure stable results
@nreese when this one is ready, I can run against cloud CI to get those results, let me know. |
Pinging @elastic/kibana-gis (Team:Geo) |
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! 🤞
code review
@elasticmachine merge upstream |
Results from cloud: https://internal-ci.elastic.co/view/Stack%20Tests/job/elastic+estf-cloud-upgrade-snapshot-tests+main/11/testReport/ Looks like an improvement for web logs, but still needs a small tolerance increase and some others are failing for
However a larger difference is now seen for flights in some cases:
|
It looks like the screen sizes are different then the baselines. Could this be caused by cloud tests not using buildkite? Below are the diffs for those 2 tests. Maybe instead of a screen shot comparison, the tests could be updated to make sure data is successfully pulled and there are not unexpected errors? |
@nreese yes the upgrade tests are still in Jenkins. My goal right now is to unblock the 8.3 snapshot testing (which was recently added), so for me increasing the tolerance is okay for the existing tests. Then later in another PR, I can pair up with you or someone from GIS team to see the best way to handle maps testing in upgrade feature. We can certainly change the testing, but I would need to understand what you mean by verify data is successfully pulled versus using screenshots, I expect the data to be dynamic. Since, I am not too familiar with maps tests and would need to get some additional information before rewriting the tests, it is going to hold up stabilizing the test runs for 8.3 while I do this. Would you be okay with reverting the upgrade maps smoke test changes in this PR and I will add the existing screenshots to upgrade directory with the tolerance increase in the PR I have open now? Afterwards I will schedule a pair session to rewrite maps tests for upgrade. |
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.
Let's revert the changes in x-pack/test/upgrade/apps/maps/maps_smoke_tests.ts
in this PR and just increase the tolerance on the existing screenshots in #132374. We will do another PR later to rewrite the maps smoke tests for upgrade testing.
I have reverted changes to x-pack/test/upgrade/apps/maps/maps_smoke_tests.ts |
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
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
#91205 re-enabled Maps sample_data functional tests. The PR also switched the logic for setting the time range from setting the absolute range from 6 months in past to 6 months in the future to set a quick select time range 30 days in the past and 40 days in the future
The problem with this change is that the data sets extend into the future more than 40 days. Since sample data is shifted to be relative to current time, this can cause different results between runs. The screen shot below shows how web logs sample data set extends past 40 days in the future. Having a time range long enough to include all sample data ensures the results are the same between test runs.
The PR also