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

[SecuritySolution] Revert defend-workflows integration tests #187257

Merged
merged 31 commits into from
Jul 11, 2024

Conversation

angorayc
Copy link
Contributor

@angorayc angorayc commented Jul 1, 2024

Summary

#183611

I moved x-pack/test/security_solution_endpoint to x-pack/test/security_solution_api_integration in #183611 as I thought all the tests regarding Security Solution should live there.

However security_solution_endpoint are not api tests , they are UI tests. After discussions, we decided to move security_solution_endpoint back to x-pack/test/

The two files below are shared between x-pack/test/security_solution_api_integration/test_suites/security_solution_endpoint_api_int and x-pack/test/security_solution_endpoint, moved them to services in this PR to avoid type check confusion.

  • x-pack/test/common/services/security_solution/endpoint_data_stream_helpers.ts
  • x-pack/test/common/services/security_solution/endpoint_registry_helpers.ts

@angorayc
Copy link
Contributor Author

angorayc commented Jul 1, 2024

buildkite test this

@angorayc angorayc added Team:Defend Workflows “EDR Workflows” sub-team of Security Solution release_note:skip Skip the PR/issue when compiling release notes labels Jul 1, 2024
@angorayc
Copy link
Contributor Author

angorayc commented Jul 1, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 1, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 1, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 2, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 2, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 2, 2024

buildkite test this

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jul 2, 2024

💔 Build Failed

Failed CI Steps

Metrics [docs]

✅ unchanged

History

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

@angorayc
Copy link
Contributor Author

angorayc commented Jul 2, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 2, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 3, 2024

/ci

@@ -0,0 +1,145 @@
/*
Copy link
Contributor Author

@angorayc angorayc Jul 3, 2024

Choose a reason for hiding this comment

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

Move the shared utils to x-pack/test/common/services/security_solution/ to avoid type check confusion:

Screenshot 2024-07-03 at 11 47 32 Screenshot 2024-07-03 at 16 40 01

@angorayc
Copy link
Contributor Author

angorayc commented Jul 5, 2024

/ci

@angorayc
Copy link
Contributor Author

angorayc commented Jul 5, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 5, 2024

buildkite test this

@angorayc
Copy link
Contributor Author

angorayc commented Jul 8, 2024

buildkite test this

@angorayc angorayc marked this pull request as ready for review July 8, 2024 14:33
@angorayc angorayc requested review from a team as code owners July 8, 2024 14:33
@angorayc angorayc requested review from joeypoon and szwarckonrad July 8, 2024 14:33
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

@angorayc angorayc requested a review from MadameSheema July 10, 2024 11:53

export default function (providerContext: FtrProviderContext) {
const { loadTestFile, getService, getPageObjects } = providerContext;

// Flaky: https://github.com/elastic/kibana/issues/186089
describe('@skipInServerless endpoint', function () {
describe('endpoint', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

@skipInServerless should stay

Comment on lines 54 to 57
// It's flaky only in Serverless
// Failing: See https://github.com/elastic/kibana/issues/187314
// Failing: See https://github.com/elastic/kibana/issues/187383
describe.skip('@ess @serverless For each artifact list under management', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe linked failures are from ESS not Serverless? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll remove the It's flaky only in Serverless comment. This test is currently unskipped , and only running on ESS. Should I add @serverless to this test?

// Failing: See https://github.com/elastic/kibana/issues/187314
// Failing: See https://github.com/elastic/kibana/issues/187383
describe.skip('@ess @serverless For each artifact list under management', function () {
describe('For each artifact list under management', function () {
targetTags(this, ['@ess', '@skipInServerless']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: is targetTags something we are switching to from inline tags?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

security_solution_api_integration supports inline tags; while x-pack/test supports only targetTags.
Given that we are moving tests back to x-pack/test, we should revert the way we apply tags as well.

@@ -23,6 +25,8 @@ export const services = {
endpointPolicyTestResources: EndpointPolicyTestResourcesProvider,
endpointArtifactTestResources: EndpointArtifactsTestResources,
rolesUsersProvider: RolesUsersProvider,
endpointDataStreamHelpers: SecuritySolutionEndpointDataStreamHelpers,
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Is there any upside to adding those as services instead of regular imports in tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my perspective, it's mainly for maintenance. I tried to imported it straight away in the code, e.g.: in security_solution_endpoint folder,

import {something} from '{somewhere}/security_solution_api_integration/test_suites/security_solution_endpoint_api_int/registry.ts'

It gave me a coupe of type check error: #187257 (comment)
Therefore, I moved them to the services folder and follow the getService pattern to apply them in the code.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

Copy link
Contributor

@szwarckonrad szwarckonrad left a comment

Choose a reason for hiding this comment

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

LGTM

@angorayc angorayc merged commit 756e9c1 into elastic:main Jul 11, 2024
19 checks passed
@kibanamachine kibanamachine added v8.16.0 backport:skip This commit does not require backporting labels Jul 11, 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 release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants