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

Fix failing test: printablePdfV2 allows width and height to have decimal, and others #143873

Merged
merged 4 commits into from
Nov 4, 2022
Merged

Fix failing test: printablePdfV2 allows width and height to have decimal, and others #143873

merged 4 commits into from
Nov 4, 2022

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Oct 24, 2022 •
edited
Loading

Summary

Closes #143717
Closes #143738
Closes #143904

After looking into the issue, it seemed tests were failing, possibly due to improper cleanup from the API tests on Reporting's ILM policy: reports would be deleted in the background before they could be downloaded by the test:

[00:09:24]           │ info [o.e.x.i.IndexLifecycleTransition] [ftr] moving index [.reporting-2022-10-23] from [{"phase":"delete","action":"delete","name":"wait-for-shard-history-leases"}] to [{"phase":"delete","action":"delete","name":"cleanup-snapshot"}] in policy [kibana-reporting]
[00:09:24]           │ info [o.e.x.i.IndexLifecycleTransition] [ftr] moving index [.reporting-2022-10-23] from [{"phase":"delete","action":"delete","name":"cleanup-snapshot"}] to [{"phase":"delete","action":"delete","name":"delete"}] in policy [kibana-reporting]
[00:09:24]           │ info [o.e.c.m.MetadataDeleteIndexService] [ftr] [.reporting-2022-10-23/yK_tEOhHQhqxf3L6gZqu1A] deleting index
[00:09:25]           │ proc [kibana] [2022-10-24T18:14:30.360+00:00][WARN ][plugins.reporting] No hits resulted in search
[00:09:25]           │ debg Report at path /api/reporting/jobs/download/l9n3lr6c026u85f10d2yr4by returned code 404
[00:09:25]           │ debg --- retry.tryForTime error: expected 404 to equal 200

Tests are no longer flaky: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1510

@tsullivan tsullivan force-pushed the fix/test-flaky-chromium-1 branch 2 times, most recently from 2b2aaff to 445308e Compare October 26, 2022 17:39
@tsullivan tsullivan added release_note:skip Skip the PR/issue when compiling release notes (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels Oct 26, 2022
@elastic elastic deleted a comment from kibana-ci Oct 26, 2022
@tsullivan
Copy link
Member Author

I am seeing this error in the logs when there is a failure:

[00:09:20]         │ proc [kibana] [2022-10-27T03:47:43.864+00:00][ERROR][plugins.screenshotting] Error: Screenshotting encountered a timeout error: "render complete" took longer than 120 seconds. You may need to increase "xpack.screenshotting.capture.timeouts.renderComplete" in kibana.yml.
[00:09:20]         │ proc [kibana]     at /var/lib/buildkite-agent/builds/kb-n2-4-spot-2-4b48a3662b82c850/elastic/kibana-flaky-test-suite-runner/kibana-build-xpack/x-pack/plugins/screenshotting/server/screenshots/observable.js:89:73
[00:09:20]         │ proc [kibana]     at /var/lib/buildkite-agent/builds/kb-n2-4-spot-2-4b48a3662b82c850/elastic/kibana-flaky-test-suite-runner/kibana-build-xpack/node_modules/rxjs/dist/cjs/internal/util/pipe.js:21:56
[00:09:20]         │ proc [kibana]     at Array.reduce (<anonymous>)
  1. The default xpack.reporting.queue.timeout setting is still too low. Reports in the job queue are timing out and the error code stamped on the job is unknown_error
  2. Completing with warnings should work in this case, and allow the test to pass. Therefore I think we don't need to increase the default xpack.screenshotting.capture.timeouts.renderComplete for now.

@tsullivan tsullivan force-pushed the fix/test-flaky-chromium-1 branch 2 times, most recently from da8d5a8 to 48234e1 Compare November 3, 2022 02:42
@tsullivan tsullivan force-pushed the fix/test-flaky-chromium-1 branch from 48234e1 to 3bae9ca Compare November 3, 2022 22:59
@tsullivan tsullivan changed the title Fix failing test: printablePdfV2 allows width and height to have decimal Fix failing test: printablePdfV2 allows width and height to have decimal, and others Nov 3, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 58 64 +6
osquery 108 113 +5
securitySolution 440 446 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 66 72 +6
osquery 109 115 +6
securitySolution 517 523 +6
total +20

History

  • 💚 Build #84945 succeeded 48234e17f0957d52d6cdcdfa8fab1cdc72e2fb2b
  • 💚 Build #83408 succeeded da8d5a828ae608be1d0a1d89f014202be1e0ea21
  • 💔 Build #83399 failed 710604f2fccb42444001f0bc16a572f59a340397
  • 💚 Build #83325 succeeded bf343ba99098d9d4d6679bebd28ece8bd1c9ad81
  • 💔 Build #83295 failed 1374d3dd9ef1c155d2197740ef4b00153b0a146b
  • 💚 Build #83022 succeeded 9f1e8d230b4a06be62f1449c4129c94543724c88

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

@tsullivan tsullivan marked this pull request as ready for review November 4, 2022 16:55
@tsullivan tsullivan requested review from a team as code owners November 4, 2022 16:55
@tsullivan tsullivan merged commit 88cfa2a into elastic:main Nov 4, 2022
@kibanamachine kibanamachine added v8.6.0 backport:skip This commit does not require backporting labels Nov 4, 2022
@tsullivan tsullivan added v8.0.0 v8.1.0 v8.2.0 v8.3.0 v8.4.0 v8.5.0 and removed backport:skip This commit does not require backporting labels Nov 4, 2022
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 7, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 143873 locally

1 similar comment
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 143873 locally

@tsullivan tsullivan mentioned this pull request Nov 9, 2022
Merged
tsullivan added a commit to tsullivan/kibana that referenced this pull request Nov 9, 2022
@tsullivan
Fix failing test: printablePdfV2 allows width and height to have deci…
a711cdf 
…mal, and others (elastic#143873)

* comment test suites for flaky

* test2

* ilm test cleanup

* restore

(cherry picked from commit 88cfa2a)
tsullivan added a commit to tsullivan/kibana that referenced this pull request Nov 9, 2022
@tsullivan
Fix failing test: printablePdfV2 allows width and height to have deci…
12a75d3 
…mal, and others (elastic#143873)

* comment test suites for flaky

* test2

* ilm test cleanup

* restore

(cherry picked from commit 88cfa2a)
@tsullivan tsullivan mentioned this pull request Nov 9, 2022
Closed
tsullivan added a commit to tsullivan/kibana that referenced this pull request Nov 9, 2022
@tsullivan
Fix failing test: printablePdfV2 allows width and height to have deci…
4b2c528 
…mal, and others (elastic#143873)

* comment test suites for flaky

* test2

* ilm test cleanup

* restore

(cherry picked from commit 88cfa2a)

# Conflicts:
#	x-pack/test/reporting_api_integration/reporting_and_security/ilm_migration_apis.ts
#	x-pack/test/reporting_api_integration/reporting_and_security/validation.ts
@tsullivan tsullivan mentioned this pull request Nov 9, 2022
Merged
@tsullivan
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
✅ 8.5
✅ 8.4
✅ 7.17

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

tsullivan added a commit that referenced this pull request Nov 9, 2022
@tsullivan
[8.5] Fix failing test: printablePdfV2 allows width and height to hav…
bd34b0c 
…e decimal, and others (#143873) (#144920)

# Backport

This will backport the following commits from `main` to `8.5`:
- [Fix failing test: printablePdfV2 allows width and height to have
decimal, and others
(#143873)](#143873)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2022-11-04T20:46:57Z","message":"Fix
failing test: printablePdfV2 allows width and height to have decimal,
and others (#143873)\n\n* comment test suites for flaky\r\n\r\n*
test2\r\n\r\n* ilm test cleanup\r\n\r\n*
restore","sha":"88cfa2a4ddd4ddc7977cc7f286e9f678f0c1ce19","branchLabelMapping":{"^v8.6.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Reporting","release_note:skip","backport
missing","Team:Global
Experience","v8.4.0","v8.5.0","v7.17.7","v8.6.0"],"number":143873,"url":"https://github.com/elastic/kibana/pull/143873","mergeCommit":{"message":"Fix
failing test: printablePdfV2 allows width and height to have decimal,
and others (#143873)\n\n* comment test suites for flaky\r\n\r\n*
test2\r\n\r\n* ilm test cleanup\r\n\r\n*
restore","sha":"88cfa2a4ddd4ddc7977cc7f286e9f678f0c1ce19"}},"sourceBranch":"main","suggestedTargetBranches":["8.4","8.5","7.17"],"targetPullRequestStates":[{"branch":"8.4","label":"v8.4.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.5","label":"v8.5.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"7.17","label":"v7.17.7","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.6.0","labelRegex":"^v8.6.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/143873","number":143873,"mergeCommit":{"message":"Fix
failing test: printablePdfV2 allows width and height to have decimal,
and others (#143873)\n\n* comment test suites for flaky\r\n\r\n*
test2\r\n\r\n* ilm test cleanup\r\n\r\n*
restore","sha":"88cfa2a4ddd4ddc7977cc7f286e9f678f0c1ce19"}}]}]
BACKPORT-->
tsullivan added a commit that referenced this pull request Nov 9, 2022
@tsullivan
[7.17] Fix failing test: printablePdfV2 allows width and height to ha…
8d449d1 
…ve decimal, and others (#143873) (#144922)

# Backport

This will backport the following commits from `main` to `7.17`:
- [Fix failing test: printablePdfV2 allows width and height to have
decimal, and others
(#143873)](#143873)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2022-11-04T20:46:57Z","message":"Fix
failing test: printablePdfV2 allows width and height to have decimal,
and others (#143873)\n\n* comment test suites for flaky\r\n\r\n*
test2\r\n\r\n* ilm test cleanup\r\n\r\n*
restore","sha":"88cfa2a4ddd4ddc7977cc7f286e9f678f0c1ce19","branchLabelMapping":{"^v8.6.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Reporting","release_note:skip","backport
missing","Team:Global
Experience","v8.4.0","v8.5.0","v7.17.7","v8.6.0"],"number":143873,"url":"https://github.com/elastic/kibana/pull/143873","mergeCommit":{"message":"Fix
failing test: printablePdfV2 allows width and height to have decimal,
and others (#143873)\n\n* comment test suites for flaky\r\n\r\n*
test2\r\n\r\n* ilm test cleanup\r\n\r\n*
restore","sha":"88cfa2a4ddd4ddc7977cc7f286e9f678f0c1ce19"}},"sourceBranch":"main","suggestedTargetBranches":["8.4","8.5","7.17"],"targetPullRequestStates":[{"branch":"8.4","label":"v8.4.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.5","label":"v8.5.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"7.17","label":"v7.17.7","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.6.0","labelRegex":"^v8.6.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/143873","number":143873,"mergeCommit":{"message":"Fix
failing test: printablePdfV2 allows width and height to have decimal,
and others (#143873)\n\n* comment test suites for flaky\r\n\r\n*
test2\r\n\r\n* ilm test cleanup\r\n\r\n*
restore","sha":"88cfa2a4ddd4ddc7977cc7f286e9f678f0c1ce19"}}]}]
BACKPORT-->
@kibanamachine kibanamachine added v7.17.8 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Nov 9, 2022
@tsullivan tsullivan deleted the fix/test-flaky-chromium-1 branch June 20, 2023 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vadimkibana vadimkibana vadimkibana 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 Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v7.17.7 v7.17.8 v8.4.0 v8.5.0 v8.5.1 v8.6.0
Projects
None yet
Milestone
No milestone
4 participants
@tsullivan @kibana-ci @kibanamachine @vadimkibana