[Logs Explorer] Fix flaky test - Number of installed integrations do not match #188723
Conversation
…e provided initial packages.
awahab07
added
release_note:skip
|
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs) |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
Thanks for looking into this, I don't think that will fix the problem here. Maybe the problem is related to that one instead, something is happening with was package? |
| @@ -188,6 +188,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { | |||
| ); | |||
| cleanupIntegrationsSetup = | |||
| await PageObjects.observabilityLogsExplorer.setupInitialIntegrations(); | |||
|
|
|||
| // Ensure that number of installed packages equals the initial packages | |||
| await retry.try(async () => { | |||
There was a problem hiding this comment.
Note
This change adds a new assertion without modifying the one that makes the test flaky. How does this resolve the test flakiness?
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6591[✅] x-pack/test/functional/apps/observability_logs_explorer/config.ts: 25/25 tests passed. |
💚 Build Succeeded
Metrics [docs]
|
|
Thanks for your input @yngrdyn and @tonyghiani. Any suggestions to make it deterministic, so that we always have the intended packages installed? Or if we can't control it, does it make sense to only check the number of packages installed e.g. instead of: expect(integrations.length).to.be(3);
expect(integrations).to.eql(initialPackagesTexts);we use expect(integrations.length).to.be(installedPackagesLength);
expect(integrations).to.eql(installedPackagesTexts); |
…e2e-number-of-integrations-test-failure
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6600[✅] x-pack/test/functional/apps/observability_logs_explorer/config.ts: 100/100 tests passed. |
…e2e-number-of-integrations-test-failure
| ); | ||
|
|
||
| // Skip the suite if Apache HTTP Server integration is not available | ||
| if (!installedPackagesTexts.includes(initialPackageMap.apache)) { |
There was a problem hiding this comment.
I don't get this part.
The test says clicking on Apache HTTP Server integration and moving into the second navigation level
This means Apache HTTP must be present, why are we skipping the test if its not present. If its not present it means we have not set the environment properly ?
There was a problem hiding this comment.
Apache integration is installed as a prerequisite, but once in a while the package installation could fail due to unprecedented reasons e.g. as mentioned here.
So either we need a way which guarantees that the package will always be installed, or explicitly check if the installation is present in dependent tests (which is done here).
I am open for suggestions if there's a better way.
There was a problem hiding this comment.
In that case that error should not be handled and it should fail the entire test suite.
This is a clear case of Install Package feature not working correctly. We should not handle such failures on our side, infact escalate them to Fleet if they keep on happening
There was a problem hiding this comment.
When we are doing Package Installation, we are actually testing EPR's Install function and if that function is broken we must report it rather than swallowing it on our side
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: |
|
Closing the PR as these failures should be handled in the upper level where package installation fails. See. |
Fixes #188676
Fixes #187556
Should also fix #182017
Summary
The PR attempts to fix the flaky behavior reported in the parent issue.