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

[Tests] Add Github workflow for Test Orchestrator in FT Repo to run cypress tests within Dashboards repo #5725

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

manasvinibs
Copy link
Member

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

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link

codecov bot commented Jan 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b2d2b26) 67.03% compared to head (ed9ef98) 67.03%.
Report is 11 commits behind head on main.

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           
Flag Coverage Δ
Linux_1 35.23% <ø> (ø)
Linux_2 55.18% <ø> (ø)
Linux_3 43.92% <ø> (-0.02%) ⬇️
Linux_4 35.33% <ø> (ø)
Windows_1 35.26% <ø> (ø)
Windows_2 55.15% <ø> (ø)
Windows_3 43.93% <ø> (ø)
Windows_4 35.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to add this sh file in every plugin if any of them want to onboard the test Orchestrator?

Copy link
Member Author

@manasvinibs manasvinibs Jan 23, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

@SuZhou-Joe
Copy link
Member

Will we pull all of the tests out of test repo to OpenSearch Dashboards repo?

Comment on lines +17 to +26
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,
Copy link
Member

@SuZhou-Joe SuZhou-Joe Jan 23, 2024

Choose a reason for hiding this comment

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

It seems only openSearchUrl is needed based on the migrated test cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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"
Copy link
Member

@SuZhou-Joe SuZhou-Joe Jan 23, 2024

Choose a reason for hiding this comment

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

Should we introduce SECURITY_ENABLED concept into OSD core as security plugin is just a plugin.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

@SuZhou-Joe SuZhou-Joe Jan 26, 2024

Choose a reason for hiding this comment

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

Just feel a little bit weird as for OSD core it has no knowledge on security. @kavilla What's your opinion?

@manasvinibs
Copy link
Member Author

Will we pull all of the tests out of test repo to OpenSearch Dashboards repo?

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.

@kavilla
Copy link
Member

kavilla commented Jan 23, 2024

…ypress tests within Dashboards repo

Signed-off-by: Manasvini B Suryanarayana <manasvis@amazon.com>
cypress-tests:
runs-on: ubuntu-latest
container:
image: docker://opensearchstaging/ci-runner:ci-runner-rockylinux8-opensearch-dashboards-integtest-v2
Copy link
Member

Choose a reason for hiding this comment

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

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.
Copy link
Member

@kavilla kavilla Feb 16, 2024

Choose a reason for hiding this comment

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

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
Copy link
Member

Choose a reason for hiding this comment

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

nit: OpenSearch

source ${{ env.OSD_PATH }}/scripts/common/utils.sh
open_artifact $CWD/${{ env.OPENSEARCH_DIR }} ${{ env.OPENSEARCH }}

- name: Download and extract Opensearch Dashboards artifacts
Copy link
Member

Choose a reason for hiding this comment

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

NIT: OpenSearch

- uses: actions/upload-artifact@v3
if: failure()
with:
name: osd-cypress-screenshots
Copy link
Member

Choose a reason for hiding this comment

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

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
Copy link
Member

Choose a reason for hiding this comment

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

totally unrelated: but does this cache help the re-run of stuff?


const { defineConfig } = require('cypress');

module.exports = defineConfig({
Copy link
Member

Choose a reason for hiding this comment

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

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.

@kavilla
Copy link
Member

kavilla commented Feb 16, 2024

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

@manasvinibs manasvinibs assigned manasvinibs and unassigned kavilla Feb 16, 2024
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

some comments

Comment on lines +17 to +26
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,
Copy link
Member

Choose a reason for hiding this comment

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

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"
Copy link
Member

Choose a reason for hiding this comment

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

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 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

NIT: I think we don't need the core_opensearch_dashboards

Copy link
Member

Choose a reason for hiding this comment

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

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" ];
Copy link
Member

Choose a reason for hiding this comment

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

we should also consider adding this to .js file. Then we can drop some of the hacky stuff.

Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

todo: create an issue and add follow up feedback.

@kavilla kavilla merged commit 55443f7 into opensearch-project:main Feb 16, 2024
72 of 73 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

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.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-5725-to-2.x.

@manasvinibs
Copy link
Member Author

todo: create an issue and add follow up feedback.

Created an issue to address PR comments #5892

manasvinibs added a commit to manasvinibs/OpenSearch-Dashboards that referenced this pull request Mar 8, 2024
…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 <manasvis@amazon.com>
(cherry picked from commit 55443f7)
kavilla pushed a commit that referenced this pull request Mar 13, 2024
…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 <manasvis@amazon.com>
(cherry picked from commit 55443f7)
@manasvinibs
Copy link
Member Author

Manual backport #6095

manasvinibs added a commit to manasvinibs/OpenSearch-Dashboards that referenced this pull request Mar 15, 2024
manasvinibs added a commit to manasvinibs/OpenSearch-Dashboards that referenced this pull request Mar 15, 2024
…to run cypress tests within Dashboards repo (opensearch-project#5725) (opensearch-project#6095)"

This reverts commit 385fce4.

Signed-off-by: Manasvini B Suryanarayana <manasvis@amazon.com>
manasvinibs added a commit that referenced this pull request Mar 15, 2024
…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 <manasvis@amazon.com>

---------

Signed-off-by: Manasvini B Suryanarayana <manasvis@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants