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.

Sign up for GitHub

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

Jump to bottom

[Reporting/Fn Tests] Unskip Screenshot Tests #78190

Merged
merged 5 commits into from
Sep 24, 2020
Merged

[Reporting/Fn Tests] Unskip Screenshot Tests #78190

merged 5 commits into from
Sep 24, 2020

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Sep 22, 2020 •
edited
Loading

Summary

Four of the Reporting functional tests had to be skipped in the code because they became flaky. This turned out to be due to the test code not waiting long enough for screenshot reports (PNG, PDF) to finish. They were waiting 15 seconds, which is way too low, considering that the default timeout for generating a report is 2 minutes.

Closes #76581
Closes #76551

@tsullivan tsullivan added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 22, 2020
tsullivan
tsullivan commented Sep 22, 2020
View reviewed changes
x-pack/test/reporting_api_integration/reporting_and_security/network_policy.ts
@@ -40,7 +40,7 @@ export default function ({ getService }: FtrProviderContext) {
filter(({ statusCode }) => statusCode === 500),
map(({ message }) => message),
first(),
timeout(15000)
timeout(120000)
Copy link
Member Author

Choose a reason for hiding this comment

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

This change is not related to a test that is currently failing, but the value was still way too low and was on the cusp of being flaky.

One of the reasons being very low was acceptable in this test is that the use case it covers has bug which makes it fail early instead of going through the usual 3 attempts to complete a report.

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@tsullivan
Copy link
Member Author

@elasticmachine merge master

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics 📚

✅ unchanged

History

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@tsullivan tsullivan added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Sep 24, 2020
@tsullivan tsullivan merged commit a23a5c3 into elastic:master Sep 24, 2020
@tsullivan tsullivan deleted the reporting/unskip-screenshot-tests branch September 24, 2020 18:07
@tsullivan tsullivan mentioned this pull request Sep 24, 2020
Merged
tsullivan added a commit to tsullivan/kibana that referenced this pull request Sep 24, 2020
@tsullivan @elasticmachine
[Reporting/Fn Tests] Wait longer for reports to finish (elastic#78190)
a8dbf94 
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 28, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

1 similar comment
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 29, 2020
tsullivan added a commit that referenced this pull request Sep 29, 2020
@tsullivan @elasticmachine
[Reporting/Fn Tests] Wait longer for reports to finish (#78190) (#78479)
bc1440e 
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@streamich streamich streamich approved these changes

Assignees
No one assigned
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
4 participants
@tsullivan @kibanamachine @elasticmachine @streamich