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.

Sign up for GitHub

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 query enhancement cypress tests to Github CI #8703

Merged
merged 16 commits into from
Nov 2, 2024
Merged

Add query enhancement cypress tests to Github CI #8703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
adb3dfc
Add cypress ciGroup11 to github workflow for discover 2.0 tests
abbyhu2000 Oct 22, 2024
04fb022
modify matrix logic
abbyhu2000 Oct 24, 2024
0a61085
fix start command
abbyhu2000 Oct 24, 2024
5b93291
more test
abbyhu2000 Oct 24, 2024
5b37d74
switch to another way to install sql plugin
abbyhu2000 Oct 24, 2024
4f29a0a
install wget
abbyhu2000 Oct 24, 2024
93f0123
try 1
abbyhu2000 Oct 24, 2024
3ee8452
add job scheduler
abbyhu2000 Oct 24, 2024
4f19024
use opensearch bundle link
abbyhu2000 Oct 25, 2024
1d1b334
get rid of verifying
abbyhu2000 Oct 25, 2024
0d48f93
use 2.17.0
abbyhu2000 Oct 25, 2024
76cfb80
Try another way to run opensearch
abbyhu2000 Oct 25, 2024
a39b324
correct conditions
abbyhu2000 Oct 25, 2024
2688de8
dis-enable workspace
abbyhu2000 Oct 25, 2024
29e1c41
use test location instead of matrix number
abbyhu2000 Oct 25, 2024
80b8eaf
remove ciGroup12 in package.json
abbyhu2000 Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 112 additions & 32 deletions .github/workflows/cypress_workflow.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Run cypress tests
# trigger on every PR for all branches
on:
pull_request:
branches: [ '**' ]
branches: ['**']
paths-ignore:
- '**/*.md'
- '.lycheeignore'
Expand Down Expand Up @@ -32,8 +32,6 @@ env:
TEST_REPO: ${{ inputs.test_repo != '' && inputs.test_repo || 'opensearch-project/opensearch-dashboards-functional-test' }}
TEST_BRANCH: "${{ inputs.test_branch != '' && inputs.test_branch || github.base_ref }}"
FTR_PATH: 'ftr'
START_CMD: 'node ../scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true --csp.warnLegacyBrowsers=false --uiSettings.overrides["query:enhancements:enabled"]=false'
OPENSEARCH_SNAPSHOT_CMD: 'node ../scripts/opensearch snapshot -E cluster.routing.allocation.disk.threshold_enabled=false'
CYPRESS_BROWSER: 'chromium'
CYPRESS_VISBUILDER_ENABLED: true
CYPRESS_DATASOURCE_MANAGEMENT_ENABLED: false
Expand All @@ -42,18 +40,65 @@ env:
COMMENT_TAG: '[MANUAL CYPRESS TEST RUN RESULTS]'
COMMENT_SUCCESS_MSG: ':white_check_mark: Cypress test run succeeded!'
COMMENT_FAILURE_MSG: ':x: Cypress test run failed!'
LATEST_VERSION: '2.17.0'
Copy link
Member

Choose a reason for hiding this comment

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

maybe in an inline comment on why we are using this. i believe it was because of issues trying to use main and install plugins for github virtual env

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 we could also do is actually use to make this dynamic is rely on git tags. every release infra will cut a tag for the release in all the repos post release and then we use that to make a github release. we could potentially just get the highest version tag from our repo and assume that is the highest release version of OpenSearch that we can download


jobs:
cypress-tests:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
group: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
include:
Copy link
Member

Choose a reason for hiding this comment

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

Will this work and help organize?

matrix:
  config: [standard, query_enhanced, dashboard]
  group: [1, 2, 3, 4, 5, 6, 7, 8, 9]
  include:
    - config: query_enhanced
      group: 10
      test_location: ftr
    - config: dashboard
      group: 11
      test_location: source
  exclude:
    - config: query_enhanced
      group: [1,2,3,4,5,6,7,8,9]
    - config: dashboard
      group: [1,2,3,4,5,6,7,8,9]

Copy link
Member

Choose a reason for hiding this comment

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

+1, using matrix to create the right config could make it shorter. but i do see a benefit of making it this granular as it we have more control. as it leave spaces in the future where we migrate the tests from ftr to this repo

# Standard test configs (groups 1-9)
- group: 1
config: standard
test_location: ftr
- group: 2
config: standard
test_location: ftr
- group: 3
config: standard
test_location: ftr
Copy link
Member

Choose a reason for hiding this comment

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

we have a variable FTR_PATH, should we use this/change FTR_PATH to something that makes more sense to usage and add the other locations? or should we get rid of the FTR_PATH variable

- group: 4
config: standard
Copy link
Member

@kavilla kavilla Oct 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

nit: default has been used for this type of situation or undefined and we automatically apply the default config

test_location: ftr
- group: 5
config: standard
test_location: ftr
- group: 6
config: standard
test_location: ftr
- group: 7
config: standard
test_location: ftr
- group: 8
config: standard
test_location: ftr
- group: 9
config: standard
test_location: ftr
# Query enhanced tests
- group: 10
config: query_enhanced
test_location: ftr
# Dashboard tests
- group: 11
config: dashboard
test_location: source
container:
image: docker://opensearchstaging/ci-runner:ci-runner-rockylinux8-opensearch-dashboards-integtest-v2
options: --user 1001
env:
START_CMD: ${{ matrix.config == 'query_enhanced' &&
Copy link
Member

Choose a reason for hiding this comment

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

i really like this concept of config because it actually follows the format well with how OSD functional tests are structured!

only nit I have here is that this actually is a little bit hard to read. and any fixes or additional updates to this will require us to know to update both places.

so i think minimum we should consider the logic being where we append extra configurations if the matrix config is query enhanced instead of having two.

if that is not good enough then it might be time to start sourcing a .env config file that stores this information so that we can modify this stuff and make this file more approachable.

'node ../scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true --csp.warnLegacyBrowsers=false --uiSettings.overrides["query:enhancements:enabled"]=true --uiSettings.overrides[''home:useNewHomePage'']=true --data_source.enabled=true --opensearch.ignoreVersionMismatch=true' ||
matrix.config == 'dashboard' &&
'node scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true' ||
'node ../scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true --csp.warnLegacyBrowsers=false --uiSettings.overrides["query:enhancements:enabled"]=false' }}
OPENSEARCH_SNAPSHOT_CMD: ${{ matrix.config == 'query_enhanced' &&
'/bin/bash -c "./opensearch-2.17.0/opensearch-tar-install.sh &"' ||
matrix.config == 'dashboard' &&
'node scripts/opensearch snapshot -E cluster.routing.allocation.disk.threshold_enabled=false' ||
'node ../scripts/opensearch snapshot -E cluster.routing.allocation.disk.threshold_enabled=false' }}
# prevents extra Cypress installation progress messages
CI: 1
# avoid warnings like "tput: No value for $TERM and no -T specified"
Expand Down Expand Up @@ -117,7 +162,7 @@ jobs:

# Setup spec files for existing Functional Test repo cypress tests
- name: Setup spec files
if: ${{ inputs.specs == '' && matrix.group < 10 }}
if: ${{ inputs.specs == '' && matrix.test_location == 'ftr' }}
working-directory: ${{ env.FTR_PATH }}
shell: bash
run: |
Expand All @@ -128,14 +173,26 @@ jobs:
done
echo "SPEC=${FORMATTED_SPEC}" >> $GITHUB_ENV

# Setup spec files for Dashboards in-house cypress tests
- name: Setup spec files for Dashboards tests
if: ${{ inputs.specs == '' && matrix.test_location == 'source' }}
shell: bash
run: |
IFS="," read -a SPEC_ARRAY <<< $(yarn --silent osd:ciGroup${{ matrix.group }})
DASHBOARDS_SPEC=''
for i in "${SPEC_ARRAY[@]}"; do
DASHBOARDS_SPEC+="cypress/integration/core_opensearch_dashboards/${i},"
Copy link
Member

Choose a reason for hiding this comment

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

we desperately need to be able to tag cypress test so that when we write them inhouse the test can decide which workflow it will run in

done
echo "DASHBOARDS_SPEC=${DASHBOARDS_SPEC}" >> $GITHUB_ENV

- name: Get Cypress version
if: ${{ matrix.group < 10 }}
if: ${{ matrix.test_location == 'ftr' }}
id: cypress_version
run: |
echo "name=cypress_version::$(cat ./${{ env.FTR_PATH }}/package.json | jq '.devDependencies.cypress' | tr -d '"')" >> $GITHUB_OUTPUT

- name: Cache Cypress
if: ${{ matrix.group < 10 }}
if: ${{ matrix.test_location == 'ftr' }}
id: cache-cypress
uses: actions/cache@v1
with:
Expand All @@ -146,70 +203,93 @@ jobs:
- run: npx cypress cache list
- run: npx cypress cache path

# Setup spec files for Dashboards in-house cypress tests
- name: Setup spec files for Dashboards tests
if: ${{ inputs.specs == '' && matrix.group > 9 }}
# Run tests based on configuration
- name: Run FT repo tests with no query enhancements
Copy link
Member

Choose a reason for hiding this comment

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

nit: historically we called this FTRepo.

and I think the specification with no query enhancements might be unnecessary. we should go with the actual one run the query enhancements to say with query enhancements incase we want to follow this format in the future and add different type of situations.

if: matrix.test_location == 'ftr' && matrix.config == 'standard'
uses: cypress-io/github-action@v2
Copy link
Member

Choose a reason for hiding this comment

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

did we want to bump the cypress github action if available and works? might have improvements https://github.com/cypress-io/github-action/blob/master/CHANGELOG.md. but i see somethings in our file will need to change so maybe not worht it at this time

with:
working-directory: ${{ env.FTR_PATH }}
start: ${{ env.OPENSEARCH_SNAPSHOT_CMD }}, ${{ env.START_CMD }}
wait-on: 'http://localhost:9200, http://localhost:5601'
command: yarn cypress:run-without-security --browser ${{ env.CYPRESS_BROWSER }} --spec ${{ env.SPEC }}

- name: Download OpenSearch
if: matrix.test_location == 'ftr' && matrix.config == 'query_enhanced'
uses: suisei-cn/[email protected]
with:
url: https://artifacts.opensearch.org/releases/bundle/opensearch/${{ env.LATEST_VERSION }}/opensearch-${{ env.LATEST_VERSION }}-linux-x64.tar.gz

- name: Extract OpenSearch
if: matrix.test_location == 'ftr' && matrix.config == 'query_enhanced'
run: |
tar -xzf opensearch-*.tar.gz
Copy link
Member

@kavilla kavilla Oct 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we should be able to untar this file and pass the param to call it a specific name so line 238 doesn't have to be a specific version

rm -f opensearch-*.tar.gz
shell: bash

- name: Remove security plugin
if: matrix.test_location == 'ftr' && matrix.config == 'query_enhanced'
run: |
IFS="," read -a SPEC_ARRAY <<< $(yarn --silent osd:ciGroup${{ matrix.group }})
DASHBOARDS_SPEC=''
for i in "${SPEC_ARRAY[@]}"; do
DASHBOARDS_SPEC+="cypress/integration/core_opensearch_dashboards/${i},"
done
echo "DASHBOARDS_SPEC=${DASHBOARDS_SPEC}" >> $GITHUB_ENV
/bin/bash -c "yes | ./opensearch-${{ env.LATEST_VERSION }}/bin/opensearch-plugin remove opensearch-security"
shell: bash

# Run FT repo tests
- name: Run FT repo tests
if: ${{ matrix.group < 10 }}
- name: Run OpenSearch
if: matrix.test_location == 'ftr' && matrix.config == 'query_enhanced'
run: |
/bin/bash -c "./opensearch-2.17.0/opensearch-tar-install.sh &"
sleep 30
Copy link
Member

Choose a reason for hiding this comment

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

confused about this one.

wait-on should be actually a better resolution to this: https://github.com/cypress-io/github-action?tab=readme-ov-file#wait-on

are you seeing issues with that?

shell: bash

- name: Run FT repo tests with query enhancements
if: matrix.test_location == 'ftr' && matrix.config == 'query_enhanced'
uses: cypress-io/github-action@v2
with:
working-directory: ${{ env.FTR_PATH }}
start: ${{ env.OPENSEARCH_SNAPSHOT_CMD }}, ${{ env.START_CMD }}
start: ${{ env.START_CMD }}
wait-on: 'http://localhost:9200, http://localhost:5601'
command: yarn cypress:run-without-security --browser ${{ env.CYPRESS_BROWSER }} --spec ${{ env.SPEC }}

# Clear Cypress Cache before running Dashboards tests
- name: Clear Cypress Cache
if: ${{ matrix.group > 9 }}
if: matrix.test_location == 'source'
run: npx cypress cache clear

# Run Dashboards Cypress tests within the source repo
- name: Run Dashboards Cypress tests
if: ${{ matrix.group > 9 }}
if: matrix.test_location == 'source'
uses: cypress-io/github-action@v6
with:
install-command: npx cypress install --force
start: node scripts/opensearch snapshot -E cluster.routing.allocation.disk.threshold_enabled=false, node scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true
start: ${{ env.OPENSEARCH_SNAPSHOT_CMD }}, ${{ env.START_CMD }}
wait-on: 'http://localhost:9200, http://localhost:5601'
command: yarn cypress:run-without-security --browser ${{ env.CYPRESS_BROWSER }} --spec ${{ env.DASHBOARDS_SPEC }}

# Screenshots are only captured on failure, will change this once we do visual regression tests
- name: Upload FT repo screenshots
uses: actions/upload-artifact@v3
if: failure() && (matrix.group < 10)
if: failure() && matrix.test_location == 'ftr'
with:
name: ftr-cypress-screenshots
path: ${{ env.FTR_PATH }}/cypress/screenshots
retention-days: 1

- name: Upload FT repo videos
uses: actions/upload-artifact@v3
if: always() && (matrix.group < 10)
if: always() && matrix.test_location == 'ftr'
with:
name: ftr-cypress-videos
path: ${{ env.FTR_PATH }}/cypress/videos
retention-days: 1

- name: Upload FT repo results
uses: actions/upload-artifact@v3
if: always() && (matrix.group < 10)
if: always() && matrix.test_location == 'ftr'
with:
name: ftr-cypress-results
path: ${{ env.FTR_PATH }}/cypress/results
retention-days: 1

- name: Upload Dashboards screenshots
if: failure() && (matrix.group > 9)
if: failure() && matrix.test_location == 'source'
uses: actions/upload-artifact@v3
with:
name: dashboards-cypress-screenshots
Expand All @@ -218,15 +298,15 @@ jobs:

- name: Upload Dashboards repo videos
uses: actions/upload-artifact@v3
if: always() && (matrix.group > 9)
if: always() && matrix.test_location == 'source'
with:
name: dashboards-cypress-videos
path: cypress/videos
retention-days: 1

- name: Upload Dashboards repo results
uses: actions/upload-artifact@v3
if: always() && (matrix.group > 9)
if: always() && matrix.test_location == 'source'
with:
name: dashboards-cypress-results
path: cypress/results
Expand All @@ -245,7 +325,7 @@ jobs:
with:
issue-number: ${{ inputs.pr_number }}
comment-author: 'github-actions[bot]'
body-includes: "${{ env.COMMENT_TAG }}"
body-includes: '${{ env.COMMENT_TAG }}'

- name: Add comment on the PR
uses: peter-evans/create-or-update-comment@v3
Expand Down
3 changes: 1 addition & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@
"release_note:generate": "scripts/use_node scripts/generate_release_note",
"cypress:run-without-security": "env TZ=America/Los_Angeles NO_COLOR=1 cypress run --headless --env SECURITY_ENABLED=false",
"cypress:run-with-security": "env TZ=America/Los_Angeles NO_COLOR=1 cypress run --headless --env SECURITY_ENABLED=true,openSearchUrl=https://localhost:9200,WAIT_FOR_LOADER_BUFFER_MS=500",
"osd:ciGroup10": "echo \"dashboard_sanity_test_spec.js\"",
"osd:ciGroup11": "echo \"apps/vis_builder/*.js\"",
Copy link
Member Author

@abbyhu2000 abbyhu2000 Oct 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

removed "osd:ciGroup11": "echo \"apps/vis_builder/*.js\"", because vis_builder tests are already running in ciGroup1 as defined here https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/package.json#L28

"osd:ciGroup11": "echo \"dashboard_sanity_test_spec.js\"",
"generate:opensearchsqlantlr": "./node_modules/antlr4ng-cli/index.js -Dlanguage=TypeScript -o ./src/plugins/data/public/antlr/opensearch_sql/.generated -visitor -no-listener -Xexact-output-dir ./src/plugins/data/public/antlr/opensearch_sql/grammar/OpenSearchSQLLexer.g4 ./src/plugins/data/public/antlr/opensearch_sql/grammar/OpenSearchSQLParser.g4",
"generate:opensearchpplantlr": "./node_modules/antlr4ng-cli/index.js -Dlanguage=TypeScript -o ./src/plugins/data/public/antlr/opensearch_ppl/.generated -visitor -no-listener -Xexact-output-dir ./src/plugins/data/public/antlr/opensearch_ppl/grammar/OpenSearchPPLLexer.g4 ./src/plugins/data/public/antlr/opensearch_ppl/grammar/OpenSearchPPLParser.g4"
},
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/public/ui/dataset_selector/configurator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export const Configurator = ({
setLanguage(e.target.value);
setDataset({ ...dataset, language: e.target.value });
}}
data-test-subj="advancedSelectorLanguageSelect"
/>
</EuiFormRow>
{!languageService.getLanguage(language)?.hideDatePicker &&
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data/public/ui/dataset_selector/dataset_selector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ export const DatasetSelector = ({
appearance={appearance}
{...buttonProps}
className="datasetSelector__button"
data-test-subj="datasetSelectorButton"
iconType="arrowDown"
iconSide="right"
onClick={togglePopover}
Expand Down Expand Up @@ -253,6 +254,7 @@ export const DatasetSelector = ({
<EuiPopoverFooter paddingSize="none" className="datasetSelector__footer">
<EuiButton
className="datasetSelector__advancedButton"
data-test-subj="datasetSelectorAdvancedButton"
iconType="gear"
iconSide="right"
iconSize="s"
Expand Down
Loading