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

[Security Solution] Fix flaky test: x-pack/test/detection_engine_api_integration/security_and_spaces/update_prebuilt_rules_package/update_prebuilt_rules_package·ts - update_prebuilt_rules_package should allow user to install prebuilt rules from scratch, then install new rules and upgrade existing rules from the new package #163241

Merged
merged 8 commits into from
Aug 11, 2023
Merged

[Security Solution] Fix flaky test: x-pack/test/detection_engine_api_integration/security_and_spaces/update_prebuilt_rules_package/update_prebuilt_rules_package·ts - update_prebuilt_rules_package should allow user to install prebuilt rules from scratch, then install new rules and upgrade existing rules from the new package #163241

Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a25be3f
RUn flaky tets runner
jpdjere Aug 7, 2023
d7472f8
Add indices refresh after installing and upgrading rules
jpdjere Aug 8, 2023
e8395b3
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Aug 8, 2023
ce1cb78
Merge branch 'main' into update-prebuilt-rules-package-flay-test-runner
jpdjere Aug 8, 2023
7ee84f3
Refactor
jpdjere Aug 10, 2023
2d0a116
Merge branch 'main' into update-prebuilt-rules-package-flay-test-runner
jpdjere Aug 10, 2023
4b11128
Fix
jpdjere Aug 10, 2023
4857be9
Merge branch 'main' into update-prebuilt-rules-package-flay-test-runner
jpdjere Aug 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion ...ration/security_and_spaces/update_prebuilt_rules_package/update_prebuilt_rules_package.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import JSON5 from 'json5';
import expect from 'expect';
import { PackageSpecManifest } from '@kbn/fleet-plugin/common';
import { DETECTION_ENGINE_RULES_URL_FIND } from '@kbn/security-solution-plugin/common/constants';
import { ALL_SAVED_OBJECT_INDICES } from '@kbn/core-saved-objects-server';
import { FtrProviderContext } from '../../common/ftr_provider_context';
import {
deleteAllPrebuiltRuleAssets,
Expand Down Expand Up @@ -133,6 +134,8 @@ export default ({ getService }: FtrProviderContext): void => {
// Verify that the _perform endpoint returns the same number of installed rules as the status endpoint
// and the _review endpoint
const installPrebuiltRulesResponse = await installPrebuiltRules(supertest);
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });
Copy link
Contributor

Choose a reason for hiding this comment

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

It's quite easy to forget why es.indices.refresh() is needed and what part of test it's related to. It'd be nicer to have it right before an HTTP call depending on it so anybody reusing it doesn't have to keep in mind es.indices.refresh() has to be called before. For example getPrebuiltRulesStatus() should contain es.indices.refresh() inside.


Copy link
Contributor

Choose a reason for hiding this comment

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

Installed rules fetching chunk looks to be repeated. It could be a reusable function with es.indices.refresh() inside

const { body: rulesResponse } = await supertest
        .get(`${DETECTION_ENGINE_RULES_URL_FIND}?per_page=10000`)
        .set('kbn-xsrf', 'true')
        .send()
        .expect(200);

expect(installPrebuiltRulesResponse.summary.succeeded).toBe(
statusAfterPackageInstallation.stats.num_prebuilt_rules_to_install
);
Expand Down Expand Up @@ -168,9 +171,10 @@ export default ({ getService }: FtrProviderContext): void => {
.type('application/json')
.send({ force: true })
.expect(200);

expect(installLatestPackageResponse.body.items.length).toBeGreaterThanOrEqual(0);

await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });

// Verify status after intallation of the latest package
const statusAfterLatestPackageInstallation = await getPrebuiltRulesStatus(supertest);
expect(
Expand Down Expand Up @@ -198,6 +202,8 @@ export default ({ getService }: FtrProviderContext): void => {
const installPrebuiltRulesResponseAfterLatestPackageInstallation = await installPrebuiltRules(
supertest
);
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });

expect(installPrebuiltRulesResponseAfterLatestPackageInstallation.summary.succeeded).toBe(
statusAfterLatestPackageInstallation.stats.num_prebuilt_rules_to_install
);
Expand Down Expand Up @@ -241,6 +247,8 @@ export default ({ getService }: FtrProviderContext): void => {
const upgradePrebuiltRulesResponseAfterLatestPackageInstallation = await upgradePrebuiltRules(
supertest
);
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });

expect(upgradePrebuiltRulesResponseAfterLatestPackageInstallation.summary.succeeded).toEqual(
statusAfterLatestPackageInstallation.stats.num_prebuilt_rules_to_upgrade
);
Expand Down