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

Functional tests - stabilize reporting tests for cloud execution #83787

Merged
merged 2 commits into from
Nov 20, 2020
Merged

Functional tests - stabilize reporting tests for cloud execution #83787

merged 2 commits into from
Nov 20, 2020

Conversation

pheyos
Copy link
Member

@pheyos pheyos commented Nov 19, 2020 •
edited
Loading

Summary

This PR fixes some reporting API test failure that occurred during cloud execution.

Details

Retry in deleteAllReports

  • This fails regularly on cloud as the request oftentimes needs more than 100ms to return, so the code runs into a timeout even though the request returns successfully.
  • Increasing the Rx.interval value is an option that I considered but response times can significantly vary depending on the cloud region and on the location of the test runner. That's why I think it makes sense to fire the request until it is successful or the timeout is reached no matter how long one single request takes (unless it's longer than the timeout, in which case the operation will fail as expected).
  • Another argument for changing the implementation is that the rxjs toPromise will be removed in a future version.

Closes #78065
Closes #78066
Closes #78067
Closes #78068
Closes #80266
Closes #80267
Closes #80269
Closes #80270
Closes #80271

Sorting in merge user state for docvalue_fields

  • I've noticed a cloud test failure on expect(resText).to.eql(fixtures.CSV_RESULT_DOCVALUE); with the following output:
+ expected - actual

 "Jun 26, 2019 @ 00:00:00.000","[""Men's Accessories""]",EUR,4,570161,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0606606066"",""ZO0596305963""]"
 "Jun 26, 2019 @ 00:00:00.000","[""Women's Shoes"",""Women's Clothing""]",EUR,17,570200,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0025100251"",""ZO0101901019""]"
 "Jun 26, 2019 @ 00:00:00.000","[""Women's Clothing"",""Women's Shoes""]",EUR,27,732050,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0101201012"",""ZO0230902309"",""ZO0325603256"",""ZO0056400564""]"
 "Jun 26, 2019 @ 00:00:00.000","[""Men's Clothing"",""Men's Shoes""]",EUR,52,719675,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0448604486"",""ZO0686206862"",""ZO0395403954"",""ZO0528505285""]"
-"Jun 26, 2019 @ 00:00:00.000","[""Men's Accessories"",""Men's Clothing""]",EUR,41,570079,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0598505985"",""ZO0449304493""]"                                                                                                    
-"Jun 26, 2019 @ 00:00:00.000","[""Men's Clothing"",""Men's Shoes""]",EUR,37,569637,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0300103001"",""ZO0688106881""]"                                                                                                          
-"Jun 26, 2019 @ 00:00:00.000","[""Women's Accessories"",""Women's Clothing""]",EUR,26,570588,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0092000920"",""ZO0152001520""]"                                                                                                
+"Jun 26, 2019 @ 00:00:00.000","[""Women's Clothing"",""Women's Accessories""]",EUR,26,570396,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0495604956"",""ZO0208802088""]"                                                                                                
+"Jun 26, 2019 @ 00:00:00.000","[""Women's Shoes"",""Women's Accessories""]",EUR,17,570037,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0321503215"",""ZO0200102001""]"                                                                                                   
+"Jun 26, 2019 @ 00:00:00.000","[""Women's Shoes"",""Women's Clothing""]",EUR,24,569311,3,"Jun 26, 2019 @ 00:00:00.000","[""Dec 15, 2016 @ 00:00:00.000"",""Dec 15, 2016 @ 00:00:00.000""]","[""ZO0024600246"",""ZO0660706607""]"
  • I could reproduce this consistently on the cloud cluster but not on my local machine (also, this seems to be passing fine on CI).
  • So I've compared the results of the underlying saved searches locally vs. in cloud and I've noticed that the entries are displayed in different order. I suspect this is because the value of the sort field order_by is exactly the same in all of the documents.
  • Adding the order_id as a secondary sort field gave me consistent results in cloud and on my local machine.

Closes #80268

@pheyos pheyos self-assigned this Nov 19, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

@pheyos pheyos marked this pull request as ready for review November 19, 2020 13:55
@pheyos pheyos requested a review from tsullivan November 19, 2020 13:55
@pheyos pheyos added release_note:skip Skip the PR/issue when compiling release notes and removed backport:skip This commit does not require backporting labels Nov 19, 2020
@pheyos pheyos requested a review from joelgriffith November 19, 2020 16:57
joelgriffith
joelgriffith approved these changes Nov 19, 2020
View reviewed changes
Copy link
Contributor

@joelgriffith joelgriffith left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the description, will be curious to see how that retry handler deals with 409s

x-pack/test/reporting_api_integration/services.ts

const reportsDeleted = await deleted$.toPromise();
expect(reportsDeleted).to.be(true);
await retry.tryForTime(5000, async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious to see how this handles the 409's we return sometimes. I'm not super familiar with how retry.tryForTime works internally

Copy link
Member Author

Choose a reason for hiding this comment

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

The .expect(200) will throw an error for any response code different than 200 and this will be caught by the retry, that will then execute the code block again (until the timeout is reached).

@pheyos pheyos merged commit 2b3fe1f into elastic:master Nov 20, 2020
@pheyos pheyos deleted the stabilize_cloud_reporting_tests branch November 20, 2020 04:47
@pheyos pheyos mentioned this pull request Nov 20, 2020
Merged
pheyos added a commit to pheyos/kibana that referenced this pull request Nov 20, 2020
@pheyos
Functional tests - stabilize reporting tests for cloud execution (ela…
c10778c 
…stic#83787)

This PR fixes some reporting API test failure that occurred during cloud execution.
@pheyos pheyos mentioned this pull request Nov 20, 2020
Merged
pheyos added a commit to pheyos/kibana that referenced this pull request Nov 20, 2020
@pheyos
Functional tests - stabilize reporting tests for cloud execution (ela…
1150a23 
…stic#83787)

This PR fixes some reporting API test failure that occurred during cloud execution.
pheyos added a commit that referenced this pull request Nov 20, 2020
@pheyos
Functional tests - stabilize reporting tests for cloud execution (#83787
749c22b 
) (#83883)

This PR fixes some reporting API test failure that occurred during cloud execution.
pheyos added a commit that referenced this pull request Nov 20, 2020
@pheyos
Functional tests - stabilize reporting tests for cloud execution (#83787
ac4344f 
) (#83884)

This PR fixes some reporting API test failure that occurred during cloud execution.
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 20, 2020
@gmmorris
Merge branch 'master' into alerts/broken-json
2a14916 
* master: (38 commits)
  [ML] Data frame analytics: Adds functionality to map view (elastic#83710)
  Add usage collection for savedObject tagging (elastic#83160)
  [SECURITY_SOLUTION] 145: Advanced Policy Tests (elastic#82898)
  [APM] Service overview transactions table (elastic#83429)
  [ML] Fix Single Metric Viewer not loading if job is metric with no partition (elastic#83880)
  do not export types from 3rd party modules as 'type' (elastic#83803)
  [Fleet] Allow to send SETTINGS action (elastic#83707)
  Fixes Failing test: Chrome X-Pack UI Functional Tests.x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details·ts - Actions and Triggers app Alert Details Alert Instances renders the active alert instances (elastic#83478)
  [Uptime]Reduce chart height on monitor detail page (elastic#83777)
  [APM] Prefer `APIReturnType` over `PromiseReturnType` (elastic#83843)
  [Observability] Fix telemetry for Observability Overview (elastic#83847)
  [Alerting] Adds generic UI for the definition of conditions for Action Groups (elastic#83278)
  ensure workload agg doesnt run until next interval when it fails (elastic#83632)
  [ILM] Policy form should not throw away data (elastic#83077)
  [Monitoring] Stop collecting Kibana Usage in bulkUploader (elastic#83546)
  [TSVB] fix wrong imports (elastic#83798)
  [APM] Correlations UI POC (elastic#82256)
  list all the refs in  tsconfig.json (elastic#83678)
  Bump jest (and related packages) to v26.6.3 (elastic#83724)
  Functional tests - stabilize reporting tests for cloud execution (elastic#83787)
  ...
@pheyos pheyos mentioned this pull request Jan 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelgriffith joelgriffith joelgriffith approved these changes

@tsullivan tsullivan Awaiting requested review from tsullivan

Assignees

@pheyos pheyos

Labels
release_note:skip Skip the PR/issue when compiling release notes test-api-integration v7.10.1 v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting_and_security/bwc_generation_urls·ts - Reporting APIs BWC report generation urls "before all" hook in "BWC report generation urls" [test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting/usage·ts - Reporting APIs reporting usage "before all" hook in "reporting usage" [test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting/csv_saved_search·ts - Reporting APIs Generation from Saved Search ID Saved Search Features "after all" hook for "Formatted date_nanos data, custom time zone" [test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting/csv_job_params·ts - Reporting APIs Generation from Job Params "after all" hook for "Accepts jobParams in query string" [test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting/bwc_generation_urls·ts - Reporting APIs BWC report generation urls "before all" hook in "BWC report generation urls"
[test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting_and_security/csv_job_params·ts - Reporting APIs Generation from Job Params "after all" hook for "Accepts jobParams in query string" [test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting_and_security/csv_saved_search·ts - Reporting APIs Generation from Saved Search ID Merge user state into the query for docvalue_fields [test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting_and_security/network_policy·ts - Reporting APIs Network Policy should fail job when page voilates the network policy [test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting_and_security/spaces·ts - Reporting APIs Exports and Spaces "after all" hook for "should complete a job of PDF export of a dashboard in non-default space" [test-failed]: X-Pack Reporting API Integration Tests1.x-pack/test/reporting_api_integration/reporting_and_security/usage·ts - Reporting APIs Usage "before all" hook in "Usage"
3 participants
@pheyos @kibanamachine @joelgriffith