[Tests] Add Github workflow for Test Orchestrator in FT Repo to run cypress tests within Dashboards repo #5725
Conversation
manasvinibs
requested review from
ananzh,
kavilla,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
kristenTian,
zhongnansu,
ZilongX,
Flyingliuhub,
BSFishy,
curq,
bandinib-amzn and
SuZhou-Joe
as code owners
manasvinibs
force-pushed
the
Issue-5720
branch
from
58b69c5 to
7ade55b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5725 +/- ##
=======================================
Coverage 67.03% 67.03%
=======================================
Files 3296 3296
Lines 63343 63343
Branches 10087 10087
=======================================
Hits 42465 42465
Misses 18429 18429
Partials 2449 2449
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
scripts/cypress_tests.sh
Outdated
There was a problem hiding this comment.
Do we need to add this sh file in every plugin if any of them want to onboard the test Orchestrator?
There was a problem hiding this comment.
I think so. Every plugin has to have their GitHub workflow file which runs cypress tests in their repo along with setup scripts and config files to be able to run cypress tests inside the repo. I can help adding readme doc/example plugin setup once we are done with onboarding Dashboards. Also, these helper scripts and configurations can be hosted in the test-library and can be consumed by all plugins as dependencies.
There was a problem hiding this comment.
Sure, maybe add some helper function such as polling script into the test-library as well so that we can keep the cypress_tests.sh as simple as possible for onboarding.
|
Will we pull all of the tests out of test repo to OpenSearch Dashboards repo? |
| SECURITY_ENABLED: false, | ||
| AGGREGATION_VIEW: false, | ||
| username: 'admin', | ||
| password: 'myStrongPassword123!', | ||
| ENDPOINT_WITH_PROXY: false, | ||
| MANAGED_SERVICE_ENDPOINT: false, | ||
| VISBUILDER_ENABLED: true, | ||
| DATASOURCE_MANAGEMENT_ENABLED: false, | ||
| ML_COMMONS_DASHBOARDS_ENABLED: true, | ||
| WAIT_FOR_LOADER_BUFFER_MS: 0, |
There was a problem hiding this comment.
It seems only openSearchUrl is needed based on the migrated test cases?
There was a problem hiding this comment.
Yes but as the plan is to right away start writing new tests in this repo, I've added/matched mostly what is in FTRepo.
There was a problem hiding this comment.
do we have a link we can provide with the plan and attach it here?
package.json
Outdated
| @@ -77,7 +77,9 @@ | |||
| "docs:acceptApiChanges": "scripts/use_node --max-old-space-size=6144 scripts/check_published_api_changes.js --accept", | |||
| "osd:bootstrap": "scripts/use_node scripts/build_ts_refs && scripts/use_node scripts/register_git_hook", | |||
| "spec_to_console": "scripts/use_node scripts/spec_to_console", | |||
| "pkg-version": "scripts/use_node -e \"console.log(require('./package.json').version)\"" | |||
| "pkg-version": "scripts/use_node -e \"console.log(require('./package.json').version)\"", | |||
| "cypress:run-without-security": "env TZ=America/Los_Angeles NO_COLOR=1 cypress run --headless --env SECURITY_ENABLED=false" | |||
There was a problem hiding this comment.
Should we introduce SECURITY_ENABLED concept into OSD core as security plugin is just a plugin.
There was a problem hiding this comment.
I think so because these are Cypress env variables that needs to be set in order to override some of the configurations/commands based on if security plugin is installed or not.
https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/cypress/utils/commands.js#L75
There was a problem hiding this comment.
Just feel a little bit weird as for OSD core it has no knowledge on security. @kavilla What's your opinion?
I think that's the end goal, but till we reach there the idea was to have two states where in future tests will be written in the components repository while also maintaining the current test setup in Functional Test repo as it is. Once we are comfortable with the new orchestrator setup and plugins starts onboarding to write new tests in their own repo, we can also start pulling out all the old ones into its respective repos. |
manasvinibs
force-pushed
the
Issue-5720
branch
2 times, most recently
from
f3c8982 to
fb9b8f2
Compare
…ypress tests within Dashboards repo Signed-off-by: Manasvini B Suryanarayana <[email protected]>
manasvinibs
force-pushed
the
Issue-5720
branch
from
fb9b8f2 to
87de778
Compare
| cypress-tests: | ||
| runs-on: ubuntu-latest | ||
| container: | ||
| image: docker://opensearchstaging/ci-runner:ci-runner-rockylinux8-opensearch-dashboards-integtest-v2 |
There was a problem hiding this comment.
do we want to consider using this: https://github.com/opensearch-project/dashboards-assistant/blob/main/.github/workflows/unit_test_workflow.yml#L17
which has some pre-setup. One con would be that if we bump a dependency (like Node) the tests will fail since it won't find the required Node version if it isnt already installed
| @@ -0,0 +1,143 @@ | |||
| name: Orchestrator cypress workflow | |||
| run-name: dashboards_cypress_workflow ${{ inputs.UNIQUE_ID != '' && inputs.UNIQUE_ID || '' }} # Unique id number appended to the workflow run-name to reference the run within the orchestrator. | |||
There was a problem hiding this comment.
Is a better name for this file perhaps release_cypress_workflow since it will execute the tests within this repo against a release artifact (something that was built with --release). Because it might be confusing to see on execution that it won't pull the latest source code to run it. If the latest release artifact doesn't contain my code then my tests could still fail even though they were updated to address my changes in a PR.
| cd ${{ env.OSD_PATH }} | ||
| yarn osd bootstrap | ||
|
|
||
| - name: Download and extract Opensearch artifacts |
| source ${{ env.OSD_PATH }}/scripts/common/utils.sh | ||
| open_artifact $CWD/${{ env.OPENSEARCH_DIR }} ${{ env.OPENSEARCH }} | ||
|
|
||
| - name: Download and extract Opensearch Dashboards artifacts |
| - uses: actions/upload-artifact@v3 | ||
| if: failure() | ||
| with: | ||
| name: osd-cypress-screenshots |
There was a problem hiding this comment.
same as the above comment: indicate that these tests were executed against a release artifact so release-osd-cypress-screenshots or something if folks were looking at a glance.
| run: | | ||
| echo "name=cypress_version::$(cat ./${{ env.OSD_PATH }}/package.json | jq '.devDependencies.cypress' | tr -d '"')" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Cache Cypress |
There was a problem hiding this comment.
totally unrelated: but does this cache help the re-run of stuff?
|
|
||
| const { defineConfig } = require('cypress'); | ||
|
|
||
| module.exports = defineConfig({ |
There was a problem hiding this comment.
IMO, I think if we are adding this already we should create a cypress folder in test folder, move this spec to that folder.
Sort of how like selenium does it where it defines the config at the functional folder level and adds subdirectories below that.
|
We should also update this: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/.github/workflows/cypress_workflow.yml to execute the cypress tests within this repo. This way we can get the release testing verification added in this PR that is triggered with a dispatch event with a specified build. While the existing cypress test workflow will address the code within opened pull requests and run the tests. While we are at it, is there an easy way to code cov from cypress and add the diff to PRs: https://app.codecov.io/gh/opensearch-project/OpenSearch-Dashboards |
| SECURITY_ENABLED: false, | ||
| AGGREGATION_VIEW: false, | ||
| username: 'admin', | ||
| password: 'myStrongPassword123!', | ||
| ENDPOINT_WITH_PROXY: false, | ||
| MANAGED_SERVICE_ENDPOINT: false, | ||
| VISBUILDER_ENABLED: true, | ||
| DATASOURCE_MANAGEMENT_ENABLED: false, | ||
| ML_COMMONS_DASHBOARDS_ENABLED: true, | ||
| WAIT_FOR_LOADER_BUFFER_MS: 0, |
There was a problem hiding this comment.
do we have a link we can provide with the plan and attach it here?
| @@ -226,7 +229,9 @@ | |||
| "type-detect": "^4.0.8", | |||
| "uuid": "3.3.2", | |||
| "whatwg-fetch": "^3.0.0", | |||
| "yauzl": "^2.10.0" | |||
| "yauzl": "^2.10.0", | |||
| "@opensearch-dashboards-test/opensearch-dashboards-test-library": "https://github.com/opensearch-project/opensearch-dashboards-test-library/archive/refs/tags/1.0.6.tar.gz" | |||
There was a problem hiding this comment.
i'm fine for this for now. but the refs was a painful thing anytime we need to make a change and if we plan on adopting this more i'd imagine it would be helpful to build out the library within this repo and other folks can rely on it being available in OSD instead of an external dependency that needs to be re-installed.
For example, if we just take what's in that package and dump it here: https://github.com/opensearch-project/OpenSearch-Dashboards/tree/main/packages/osd-test in a clean folder structure then we can fix it as we go. or add stuff that can be reusable.
| @@ -0,0 +1,164 @@ | |||
| /* | |||
There was a problem hiding this comment.
NIT: I think we don't need the core_opensearch_dashboards
There was a problem hiding this comment.
One thing I think will be very important that will be easier to do starting off is to build in the concept of CiGroups like the selenium tests have. For the ftrepo I did something like this: https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/package.json#L28. If we define a folder structure that makes that a little bit easier to do that might be helpful but if this in code solution that would be great too.
Cypress tests support parallelization but I don't think we are built out to run the tests like that.
| CWD=$(pwd) | ||
| CREDENTIAL="admin:myStrongPassword123!" | ||
|
|
||
| if [ $SECURITY_ENABLED == "false" ]; |
There was a problem hiding this comment.
we should also consider adding this to .js file. Then we can drop some of the hacky stuff.
|
The backport to To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-5725-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 55443f7fe3c22afe541c7225f0d25207f1b03a77
# Push it to GitHub
git push --set-upstream origin backport/backport-5725-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.xThen, create a pull request where the |
Created an issue to address PR comments #5892 |
…ypress tests within Dashboards repo (opensearch-project#5725) * Adds Github workflow which gets triggered on dispatch event sent from FT Repo Orchestrator. Currently this workflow defaults to using release bundle artifact for Opensearch and Dashboards. * In this iteration, pulling out few of the dashboards sanity test from FT repo into the Dashboards repo - https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/cypress/integration/common/dashboard_sample_data_spec.js * Introduces Cypress dependency into the package json to run cypress tests within repo. Currently, I'm pulling the version which matches the one in FT repo. * Adds cypress config file. ToDo: Add fallback mechanism for using snapshot URL when release bundle url is not accessible. Enable this workflow for each PR/push event. opensearch-project#5720 Signed-off-by: Manasvini B Suryanarayana <[email protected]> (cherry picked from commit 55443f7)
…ypress tests within Dashboards repo (#5725) (#6095) * Adds Github workflow which gets triggered on dispatch event sent from FT Repo Orchestrator. Currently this workflow defaults to using release bundle artifact for Opensearch and Dashboards. * In this iteration, pulling out few of the dashboards sanity test from FT repo into the Dashboards repo - https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/cypress/integration/common/dashboard_sample_data_spec.js * Introduces Cypress dependency into the package json to run cypress tests within repo. Currently, I'm pulling the version which matches the one in FT repo. * Adds cypress config file. ToDo: Add fallback mechanism for using snapshot URL when release bundle url is not accessible. Enable this workflow for each PR/push event. #5720 Signed-off-by: Manasvini B Suryanarayana <[email protected]> (cherry picked from commit 55443f7)
|
Manual backport #6095 |
…to run cypress tests within Dashboards repo (opensearch-project#5725) (opensearch-project#6095)" This reverts commit 385fce4.
…to run cypress tests within Dashboards repo (opensearch-project#5725) (opensearch-project#6095)" This reverts commit 385fce4. Signed-off-by: Manasvini B Suryanarayana <[email protected]>
…to run cypress tests within Dashboards repo and Rename cypress config file to its version supported convention (#6166) * Revert "Rename cypress config file to its version supported convention (#6137) (#6141)" This reverts commit ed2e38d. * Revert "[Tests] Add Github workflow for Test Orchestrator in FT Repo to run cypress tests within Dashboards repo (#5725) (#6095)" This reverts commit 385fce4. Signed-off-by: Manasvini B Suryanarayana <[email protected]> --------- Signed-off-by: Manasvini B Suryanarayana <[email protected]>
Description
ToDo:
Add fallback mechanism for using snapshot URL when release bundle url is not accessible.
Enable this workflow for each PR/push event.
Issues Resolved
#5720
Screenshot
https://github.com/manasvinibs/OpenSearch-Dashboards/actions/runs/7618260248/job/20749040514
Check List
yarn test:jestyarn test:jest_integration