Skip to content

[Defend Workflows] Add tags for mocked and real Endpoint Cypress tests for Serverless#165094

Merged
gergoabraham merged 37 commits intoelastic:mainfrom
gergoabraham:test/olm-7133-reuse-tests-for-serverless
Sep 20, 2023
Merged

[Defend Workflows] Add tags for mocked and real Endpoint Cypress tests for Serverless#165094
gergoabraham merged 37 commits intoelastic:mainfrom
gergoabraham:test/olm-7133-reuse-tests-for-serverless

Conversation

@gergoabraham
Copy link
Contributor

@gergoabraham gergoabraham commented Aug 29, 2023

Summary

  • introduces tags for [Defend Workflows] cypress tests (similarly to [Security Solution][Serverless] Reusing Cypress tests for Serverless infrastructure #162698)
  • adds scripts to Security Solution:
    • cypress:dw:serverless:open and :run
    • cypress:dw:endpoint:serverless:open and :run
  • adds CI jobs to run these scripts
  • so far most of the expected tests got both @serverless and @brokenInServerless tests, because of other issues to be solved,
  • one test is able to run against serverless: x-pack/plugins/security_solution/public/management/cypress/e2e/mocked_data/policy_details.cy.ts

@gergoabraham gergoabraham added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Defend Workflows “EDR Workflows” sub-team of Security Solution labels Aug 29, 2023
@gergoabraham gergoabraham self-assigned this Aug 29, 2023
@gergoabraham gergoabraham force-pushed the test/olm-7133-reuse-tests-for-serverless branch 2 times, most recently from 2279454 to f6e7a5c Compare September 4, 2023 08:51
@gergoabraham gergoabraham force-pushed the test/olm-7133-reuse-tests-for-serverless branch 6 times, most recently from 0b52f93 to b4d1246 Compare September 14, 2023 10:20
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

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

awesome job here. Thansk for taking this on and for all the changes to enable this.

Left a round of feedback - let me know if you have any questions.

Once thing I wanted to follow up on - perhaps best in our team channel or during our weekly eng. hour:

  • What are all of the approved tags to use? are they documented somewhere? Also - might be good to add documentation it to our cypress README file.
  • What happens to a test that has no tags? I had (incorrectly?) assumed that all tests would continue to be picked up and ran in ESS, only those with @serverless would be executed in serverless. but given this PR, it leads me to belive that tags are always required? If so, is there a way to catch and error a test that has no tags?

const parseRevNumber = (revString: string) => Number(revString.match(/\d+/)?.[0]);

describe('Artifact pages', () => {
describe('Artifact pages', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if we could type this new property (tags) so that we get content assist. Can you add that to our cypress.d.ts file here:

interface SuiteConfigOverrides {
env?: {
ftrConfig: SecuritySolutionDescribeBlockFtrConfig;
};
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually - looks like tags is already defined via @cypress types so need to add to the our cypress.d.ts file. I do wonder if there is a way to make the type more specific in that it can suggest our "known" 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.

I did some experiments with types to be able to have suggestions, but didn't work so far, exactly because tags are already defined in cypress.d.ts, and therefore it cannot be overriden by a more exact type.

one solution would be to use our own security_tags or something, but I'm not sure if grep can pick it up - I'll look into this, but this is definitely an open question.

Copy link
Contributor

Choose a reason for hiding this comment

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

no big deal. it would just help with DX. Don't hold this PR because of this.

- exit_status: '*'
limit: 1

- command: .buildkite/scripts/steps/functional/defend_workflows_serverless.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the question! I've added it to our tracking issue for serverless tests, to make a team decision - until then, for this PR, I'd keep it this way to merge this as soon as possible.

@gergoabraham gergoabraham removed the request for review from a team September 19, 2023 19:32
Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

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

👍

import { dataLoaders } from './cypress/support/data_loaders';

export default defineCypressConfig({
export const CY_BASE_CONFIG = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: turn this into a function that returns the config object so that there is no concern around someone accidentally mutating this object. Or, use Object.freeze() on it (although, tht is also not a deep freeze)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the suggestion!
75cde75

@gergoabraham gergoabraham requested a review from jbudz September 19, 2023 20:33
@kibana-ci
Copy link

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
securitySolution 66 67 +1

ESLint disabled line counts

id before after diff
securitySolution 453 458 +5

Total ESLint disabled count

id before after diff
securitySolution 519 525 +6

History

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

cc @gergoabraham

@gergoabraham gergoabraham merged commit aa36fe6 into elastic:main Sep 20, 2023
@gergoabraham gergoabraham deleted the test/olm-7133-reuse-tests-for-serverless branch September 20, 2023 08:36
delanni added a commit to delanni/kibana that referenced this pull request Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR 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.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants