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

Search Sessions: Improve Flaky Functional Test #89370

Merged
merged 7 commits into from
Feb 2, 2021
Merged

Search Sessions: Improve Flaky Functional Test #89370

merged 7 commits into from
Feb 2, 2021

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Jan 26, 2021 •
edited
Loading

Summary

Part of #89069

I ran the functional test runner against a dev proxy to the test server to emulate a slow connection between the browser and Kibana server:

# test server:
node scripts/functional_tests_server.js --config x-pack/test/send_search_to_background_integration/config.ts -- --server.port=5699

# proxy: 
node index.js --timeoutTime=400 --targetUrl=http://localhost:5699 --listenPort=5620 --noSsl=true

When the browser was slowed down, I could reproduce some forms of test failures. This PR addresses the flakiness, so that tests pass when the browser connections are slow.

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@tsullivan tsullivan added v7.12.0 v7.13.0 v7.14.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Jan 26, 2021
tsullivan
tsullivan commented Jan 26, 2021
View reviewed changes
...earch_to_background_integration/tests/apps/management/search_sessions/sessions_management.ts
describe('New search sessions', () => {
before(async () => {
await PageObjects.common.navigateToApp('dashboard');
log.debug('wait for dashboard landing page');
retry.tryForTime(10000, async () => {
Copy link
Member Author

@tsullivan tsullivan Jan 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If we are on a loading screen, dashboard.loadSavedDashboard has flaky behavior: it wants to take us to the landing page, so tries to click a breadcrumb. But then the loading screen goes away and we are on the landing page. The landing page doesn't have a breadcrumb to itself, so dashboard.loadSavedDashboard breaks.

Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't navigateToApp('dashboard'); always take us to the landing page? I would think if that navigate method always sets the URL and uses the timestamp hash that it would reliably be on the landing page? I think some dashboard tests did something to avoid the timestamp hash in the URL to preserve state. Maybe that's related.

Copy link
Member Author

Choose a reason for hiding this comment

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

It takes us to the landing page, which could actually be displaying a loading screen. This retry waits until any loading screen is cleared.

Dosant
Dosant approved these changes Jan 27, 2021
View reviewed changes
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Thanks for the slow proxy tip!
It is also worth running through flaky test runner for the piece of mind

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

lizozom
lizozom approved these changes Jan 28, 2021
View reviewed changes
Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

LGTM

@tsullivan
Copy link
Member Author

The test may still be flaky

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@LeeDr
Copy link
Contributor

LeeDr commented Feb 2, 2021

I started a flaky test runner job on this PR https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1203

@tsullivan tsullivan changed the title Search Sessions: Unskip Flaky Functional Test Search Sessions: Improve Flaky Functional Test Feb 2, 2021
@tsullivan tsullivan merged commit 10e1f1c into elastic:master Feb 2, 2021
@tsullivan tsullivan deleted the flaky/search-sessions branch February 2, 2021 20:33
tsullivan added a commit to tsullivan/kibana that referenced this pull request Feb 2, 2021
@tsullivan @kibanamachine
Search Sessions: Improve Flaky Functional Test (elastic#89370)
4676fa6 
* Search Sessions: Unskip Flaky Functional Test

* typo

* keep skip for now

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@tsullivan tsullivan mentioned this pull request Feb 2, 2021
Merged
tsullivan added a commit that referenced this pull request Feb 2, 2021
@tsullivan @kibanamachine
Search Sessions: Improve Flaky Functional Test (#89370) (#90083)
56ae0b7 
* Search Sessions: Unskip Flaky Functional Test

* typo

* keep skip for now

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

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

@LeeDr LeeDr LeeDr left review comments

@lizozom lizozom lizozom approved these changes

@Dosant Dosant Dosant approved these changes

@lukasolson lukasolson Awaiting requested review from lukasolson

Assignees
No one assigned
Labels
blocker release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v7.13.0 v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tsullivan @elasticmachine @kibanamachine @LeeDr @lizozom @Dosant