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

[Infra] Attempt to fix flaky test #192091

Merged

Conversation

crespocarlos
Copy link
Contributor

@crespocarlos crespocarlos commented Sep 4, 2024

fixes #191806

Summary

It was a weird problem that could only be reproduced by running the host_view test after the node_details test. The cause was that the synthtrace data was not fully cleaned in between tests

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6854

[❌] x-pack/test/functional/apps/infra/config.ts: 9/50 tests passed.

see run history

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6863

[❌] x-pack/test/functional/apps/infra/config.ts: 0/50 tests passed.

see run history

@crespocarlos
Copy link
Contributor Author

/ci

Comment on lines -30 to -31
'metricbeat-*',
'logs-*',
Copy link
Contributor Author

@crespocarlos crespocarlos Sep 5, 2024

Choose a reason for hiding this comment

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

Note

These were not assigned to any metric type

loadTestFile(require.resolve('./metrics_anomalies'));
loadTestFile(require.resolve('./metrics_explorer'));
loadTestFile(require.resolve('./node_details'));
loadTestFile(require.resolve('./hosts_view'));
loadTestFile(require.resolve('./metrics_source_configuration'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note

This test is potentially dangerous because it changes the index pattern, which could affect al subsequent tests if it fails to return the data to the original state.

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, could you add a comment to the code alerting about that and stating that it is best to keep it last?

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6869

[❌] x-pack/test/functional/apps/infra/config.ts: 0/25 tests passed.

see run history

after(async () => kibanaServer.savedObjects.cleanStandardList());
before(async () =>
Promise.all([
esArchiver.load('x-pack/test/functional/es_archives/infra/alerts'),
Copy link
Contributor Author

@crespocarlos crespocarlos Sep 5, 2024

Choose a reason for hiding this comment

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

Note

This test was 100% dependent on the previous test running the alert archive.

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6872

[❌] x-pack/test/functional/apps/infra/config.ts: 0/25 tests passed.

see run history

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6875

[❌] x-pack/test/functional/apps/infra/config.ts: 0/25 tests passed.

see run history

@crespocarlos
Copy link
Contributor Author

/ci

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#6876

[✅] x-pack/test/functional/apps/infra/config.ts: 25/25 tests passed.

see run history

@crespocarlos crespocarlos marked this pull request as ready for review September 6, 2024 07:54
@crespocarlos crespocarlos requested review from a team as code owners September 6, 2024 07:54
@crespocarlos crespocarlos added release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.16.0 labels Sep 6, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@crespocarlos crespocarlos added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Sep 6, 2024
Copy link
Contributor

@tonyghiani tonyghiani left a comment

Choose a reason for hiding this comment

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

synthtrace change LGTM

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Sep 6, 2024
Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

@crespocarlos crespocarlos enabled auto-merge (squash) September 6, 2024 09:28
await navigateToNodeDetails('host-1', 'host', {
name: 'host-1',
});

await pageObjects.header.waitUntilLoadingHasFinished();
await pageObjects.timePicker.setAbsoluteRange(
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason why we don't need to navigate to the date with data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're setting the date picker with the same information in the test cases.

Copy link
Contributor

@MiriamAparicio MiriamAparicio 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 fixing the tests, LGTM 🌟

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 6, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: a1905e8
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-192091-a1905e8c32c2

Failed CI Steps

Metrics [docs]

✅ unchanged

History

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

@crespocarlos crespocarlos merged commit 7ad92ba into elastic:main Sep 6, 2024
23 checks passed
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.15 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 192091

Questions ?

Please refer to the Backport tool documentation

@crespocarlos crespocarlos deleted the 191806-unskip-hosts_vew-test branch September 6, 2024 11:48
crespocarlos added a commit to crespocarlos/kibana that referenced this pull request Sep 11, 2024
fixes [elastic#191806](elastic#191806)

## Summary

It was a weird problem that could only be reproduced by running the
host_view test after the node_details test. The cause was that the
synthtrace data was not fully cleaned in between tests
@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 192091 locally

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 30, 2024
@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 192091 locally

2 similar comments
@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 192091 locally

@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 192091 locally

@kibanamachine
Copy link
Contributor

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

1 similar comment
@kibanamachine
Copy link
Contributor

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

@crespocarlos crespocarlos added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.16.0
Projects
None yet
8 participants