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

[Uptime] Unskip certs func tests #73086

Merged

Conversation

justinkambic
Copy link
Contributor

Summary

Fixes #70493.

We've had these tests skipped for some time. This patch attempts to update the tests to prevent flaky failures.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@justinkambic justinkambic added bug Fixes for quality problems that affect the customer experience blocker test_xpack_functional v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.10.0 v7.9.0 labels Jul 23, 2020
@justinkambic justinkambic self-assigned this Jul 23, 2020
@justinkambic
Copy link
Contributor Author

Flaky test run passed, but an unrelated test had one failure: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/654/.

@justinkambic justinkambic force-pushed the uptime_unskip-certs-func-tests branch from f063f67 to 7daa422 Compare July 23, 2020 18:58
@justinkambic
Copy link
Contributor Author

Second round of flaky test runner passed: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/658/

@justinkambic justinkambic marked this pull request as ready for review July 23, 2020 21:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@justinkambic justinkambic added the release_note:skip Skip the PR/issue when compiling release notes label Jul 23, 2020
@justinkambic
Copy link
Contributor Author

Third run of flaky test runner passed x50, there were failures but none of the them were related to these tests: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/661/.

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinkambic i don't see any change beside un-skipping the test?

@justinkambic
Copy link
Contributor Author

@shahzad31 this one was flaky because we sometimes ended up with two items when we were only expecting one. Could it be possible that we have updated the behavior of the list on the cert page at some point in the last month? I am having a really hard time reproducing this issue.

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just had a failure occur in 7.x https://kibana-ci.elastic.co/job/elastic+kibana+7.x/6833/

I don't think the fact that this is hard to reproduce means we should just re-enable the test as it's causing a real problem for other devs when it fails in PRs.

@justinkambic
Copy link
Contributor Author

I don't think the fact that this is hard to reproduce means we should just re-enable the test as it's causing a real problem for other devs when it fails in PRs.

Yeah I wasn't aware of the recent failure when I made that comment. I'm going to need to delve deeper into this one.

@justinkambic justinkambic force-pushed the uptime_unskip-certs-func-tests branch from 3d83dd7 to b5b9ee9 Compare July 27, 2020 21:17
@justinkambic
Copy link
Contributor Author

justinkambic commented Jul 27, 2020

Flaky test run with additional patch that should fix this issue: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/680/

@justinkambic
Copy link
Contributor Author

@spalger I ran this flaky test build and it seems to have only run the SSL tests in x-pack ci-group 6, and I can't figure out why I'm not seeing the certs test run. It's in that group.

Parameters for the job are here: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/680/parameters/

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

@spalger
Copy link
Contributor

spalger commented Jul 28, 2020

@spalger I ran this flaky test build and it seems to have only run the SSL tests in x-pack ci-group 6, and I can't figure out why I'm not seeing the certs test run. It's in that group.

Parameters for the job are here: kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/680/parameters

That doesn't seem to be the case to me, but it does look like the job timed out which is going to cause erratic failure reporting. You should probably lower the iteration count so that it can complete within the time limit.

@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, seems ok now, flkay runner are passing as well https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/709/

@justinkambic
Copy link
Contributor Author

@spalger I think we're at a point now where it's worth a try unskipping. @shahzad31 added an improvement and I've updated the code that generates the data used by the test. The failures before were happening because we were expecting one item but received two; the field the objects are grouped by was different between two objects. I think now that field should be the same for any generated data.

@justinkambic justinkambic requested a review from spalger July 30, 2020 20:37
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks for working on this and I hope it works!

@spalger
Copy link
Contributor

spalger commented Jul 30, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@justinkambic justinkambic merged commit 61194b6 into elastic:master Jul 31, 2020
justinkambic added a commit to justinkambic/kibana that referenced this pull request Jul 31, 2020
* Temporarily unload all other functional tests.

* Unskip certificates test.

* Uncomment skipped test files.

* Add explicit fields to check generator call to prevent grouping issues that can lead to test flakiness.

* added wait for loading

* update missing func

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Shahzad <[email protected]>
justinkambic added a commit to justinkambic/kibana that referenced this pull request Jul 31, 2020
* Temporarily unload all other functional tests.

* Unskip certificates test.

* Uncomment skipped test files.

* Add explicit fields to check generator call to prevent grouping issues that can lead to test flakiness.

* added wait for loading

* update missing func

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Shahzad <[email protected]>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 4, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

2 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

justinkambic added a commit that referenced this pull request Aug 6, 2020
* Temporarily unload all other functional tests.

* Unskip certificates test.

* Uncomment skipped test files.

* Add explicit fields to check generator call to prevent grouping issues that can lead to test flakiness.

* added wait for loading

* update missing func

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Shahzad <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Shahzad <[email protected]>
justinkambic added a commit that referenced this pull request Aug 6, 2020
* Temporarily unload all other functional tests.

* Unskip certificates test.

* Uncomment skipped test files.

* Add explicit fields to check generator call to prevent grouping issues that can lead to test flakiness.

* added wait for loading

* update missing func

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Shahzad <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Shahzad <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 6, 2020
@justinkambic
Copy link
Contributor Author

@justinkambic justinkambic deleted the uptime_unskip-certs-func-tests branch August 6, 2020 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability test_xpack_functional v7.9.0 v7.10.0 v8.0.0
Projects
None yet
5 participants