-
Notifications
You must be signed in to change notification settings - Fork 113
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
Add test for aggregation view feature #334
Add test for aggregation view feature #334
Conversation
looks like many tests share the same code like creating user/role/etc, to make the code clean and reusable, one thing you could do is to create custom commands, like |
do you need to backport the tests to other branches like |
cypress/integration/plugins/security-dashboard-plugin/aggregation_view.js
Outdated
Show resolved
Hide resolved
cypress/integration/plugins/security-dashboard-plugin/aggregation_view.js
Outdated
Show resolved
Hide resolved
cypress/integration/plugins/security-dashboard-plugin/aggregation_view.js
Outdated
Show resolved
Hide resolved
cypress/integration/plugins/security-dashboard-plugin/aggregation_view.js
Outdated
Show resolved
Hide resolved
Hi @CCongWang , thank you for asking. Yes, I believe we need to backport to 2.x. |
bc2fb11
to
3e538e8
Compare
Hi @CCongWang, thank you for the review! I just restructured the entire test with your suggestion of adding API call commands, such as, |
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.
Hey Ryan, just have some comments to reduce some redundancy.
d6ac8b6
to
75a39c9
Compare
75d6771
to
2a6974b
Compare
cypress/fixtures/plugins/security-dashboards-plugin/indexpatterns/indexPatternTenantHeader.json
Outdated
Show resolved
Hide resolved
2a6974b
to
2d7c303
Compare
Please add a label called "backport 2.x" to this PR so that our backport workflow can automatically generate a new PR to backport once the current PR is merged. |
Any reason why we need |
Hi @tianleh, thanks for the review! Yes, the feature is disabled by default, so that the UI behaviors will be different. The test will only run when both of |
Done. Thanks for the instruction. |
In this case, do you have plan to enable the feature when Github workflow runs https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/.github/workflows/security-release-e2e-workflow.yml ? We have another feature which is disabled by default. https://github.com/opensearch-project/opensearch-dashboards-functional-test/pull/325/files The PR updates the Github workflow to start OSD with that feature enabled. I am not sure whether your feature can be enabled as simple as that or has to modify the OSD yml file. The current run https://github.com/opensearch-project/opensearch-dashboards-functional-test/actions/runs/3286221746/jobs/5414100963 doesn't run your tests. Let me know your plan. |
@tianleh We don't have plan to enable the feature when the release e2e workflow runs. The feature gets enabled by setting a feature flag to |
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
c71f27a
to
08de7d0
Compare
Signed-off-by: Ryan Liang <jiallian@amazon.com>
08de7d0
to
fadc917
Compare
Thanks for all the reviews! I have resolved all the comments, and this PR is ready to be merged now. |
(cherry picked from commit 4ebfa3d)
Signed-off-by: Jakub Przybylski <jakub.przybylski@eliatra.com>
Signed-off-by: Ryan Liang jiallian@amazon.com
Description
Add test for security dashboard plugin aggregation view feature
visit()
command to support non-admin authentication and tenant switching when security feature is enabled.createIndexPattern()
command to support index pattern creation with tenant header when security feature is enabledIssues Resolved
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.