-
Notifications
You must be signed in to change notification settings - Fork 115
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
security analytics dashboard plugin cypress tests update #657
base: main
Are you sure you want to change the base?
security analytics dashboard plugin cypress tests update #657
Conversation
Signed-off-by: Jovan Cvetkovic <[email protected]>
I don't seem to have permissions to apply labels, or run the checks for PRs in this repo. Would someone mind adding the |
@CCongWang @kavilla I also noticed that the |
} | ||
); | ||
|
||
export const createDetector = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a customized command with the same name createDetector
in https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/cypress/utils/plugins/security-analytics-dashboards-plugin/detectors.js#L9, not sure if it's confusing to use the same name but have different function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this file has both customized commands and a util function, and this file is imported in commands.js, should we just have commands in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CCongWang The util function is removed. I'll refactor it in some next PR.
@@ -3,14 +3,119 @@ | |||
|
|||
declare namespace Cypress { | |||
interface Chainable<Subject> { | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have much context on this file, but looks like most of the customized commands are not added in this file? I see there are three ways to add customized commands:
- just add a command in cypress/utils/plugins/{plugin}/commands.js
- add a command in cypress/utils/plugins/{plugin}/commands.js and then declare the command in cypress/utils/plugins/{plugin}/index.d.ts, example: https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/cypress/utils/plugins/security-dashboards-plugin/index.d.ts
- add a command in cypress/utils/plugins/{plugin}/commands.js and then declare the command in cypress/utils/index.d.ts
Do we have a preference? @kavilla @ashwin-pc @ohltyler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CCongWang I have refactored commands, all are now in the commands file and declared in index.d.ts within the plugins folder.
Hi @AWSHurneyt , for failed CodeQL workflow, looks like the deprecation message is just a warning, see this successful example: https://github.com/opensearch-project/opensearch-dashboards-functional-test/actions/runs/4636082170/jobs/8249870977?pr=608
I'm gonna rerun to see if it's something transient |
@AWSHurneyt for the other failed workflows with |
Signed-off-by: Jovan Cvetkovic <[email protected]>
Signed-off-by: Jovan Cvetkovic <[email protected]>
Would someone re-run the tests? I don't have permission to run the tests. |
Security analytics team is working on a problem with the plugin build, the fix is in the PR but will have to wait for it to be merged before the tests for the security analytics can be run successfully. |
Description
Syncs new rule tests of the security analytics dashboard plugin with functional tests
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.