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

[Enhancement/Fix] Refactor datasources cypress tests #1323

Merged
merged 15 commits into from
Jan 10, 2024

Conversation

RyanL1997
Copy link
Contributor

@RyanL1997 RyanL1997 commented Dec 21, 2023

Description

Fix the existing datasources tests, and extend the UI test cases.

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

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.

Copy link

codecov bot commented Dec 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b42aa2c) 52.92% compared to head (dc8b19a) 51.31%.
Report is 1 commits behind head on main.

❗ Current head dc8b19a differs from pull request most recent head b2ccbe6. Consider uploading reports for the commit b2ccbe6 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1323      +/-   ##
==========================================
- Coverage   52.92%   51.31%   -1.62%     
==========================================
  Files         302      303       +1     
  Lines       10587    10586       -1     
  Branches     2777     2778       +1     
==========================================
- Hits         5603     5432     -171     
- Misses       4939     5108     +169     
- Partials       45       46       +1     
Flag Coverage Δ
dashboards-observability 51.31% <ø> (-1.62%) ⬇️

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
Contributor Author

@RyanL1997 RyanL1997 left a comment

Choose a reason for hiding this comment

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

Hi folks, I just left some notes for better understanding of the changes I made.

@@ -18,14 +18,12 @@ module.exports = defineConfig({
env: {
opensearch: 'localhost:9200',
opensearchDashboards: 'localhost:5601',
security_enabled: true,
security_enabled: false,
Copy link
Contributor Author

@RyanL1997 RyanL1997 Dec 22, 2023

Choose a reason for hiding this comment

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

This security config should be disabled by default because:

  • Current testing workflow is fully without security
  • I was checking our commands setup, and I saw that we have overwritten the request command based on this config:
    • source code:

      Cypress.Commands.overwrite('visit', (originalFn, url, options) => {
        // Add the basic auth header when security enabled in the OpenSearch cluster
        // https://github.com/cypress-io/cypress/issues/1288
        if (Cypress.env('security_enabled')) {
          if (options) {
            options.auth = ADMIN_AUTH;
          } else {
            options = { auth: ADMIN_AUTH };
          }
          // Add query parameters - select the default OpenSearch Dashboards tenant
          options.qs = { security_tenant: 'private' };
          return originalFn(url, options);
        } else {
          return originalFn(url, options);
        }
      });
    • when this config enabled, every single call of the request will be assembled with this security_tenant=private option. This is kinda impact the assertion we are having for now. For example, the expected url should not contain the above tenant option.

},
'cypress-watch-and-reload': {
watch: ['common/**', 'public/**', 'server/**'],
},
e2e: {
// We've imported your old cypress plugins here.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a cypress update comment, and we should remove this.

run: |
rm -rf ./config/opensearch_dashboards.yml
cat << 'EOT' > ./config/opensearch_dashboards.yml
server.host: "0.0.0.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this dashboards configuration because the current cypress framework is having trouble determine among localhost, 127.0.0.1, and ::.

This is a known issue for cypress + node version 18+: cypress-io/cypress#25397 (comment)

@RyanL1997 RyanL1997 marked this pull request as ready for review December 22, 2023 01:17
@RyanL1997 RyanL1997 changed the title [Fix] Datasource cypress fix [Enhancement/Fix] Refactor datasources cypress tests Dec 22, 2023
@RyanL1997
Copy link
Contributor Author

@RyanL1997
Copy link
Contributor Author

RyanL1997 commented Jan 4, 2024

Here is the reproduce of linting error on my local:

Oops! Something went wrong! :(

ESLint: 6.8.0.

No files matching the pattern ".cypress/integration/datasources_test/datasources.spec.js" were found.
Please check for typing mistakes in the pattern.

Since we are just fetch the diff of files. In this PR the original name of test file got changed from datasources.spec.js into datasources_basic_ui.spec.js , but the current fetch diff step in the workflow of lint considers the deletion of datasources.spec.js as one of the changes, so we get this "... file is not found" error.

@RyanL1997 RyanL1997 force-pushed the datasource-cypress-fix branch 2 times, most recently from 2dfde77 to 9288754 Compare January 5, 2024 20:24
@RyanL1997 RyanL1997 force-pushed the datasource-cypress-fix branch 3 times, most recently from c7aab98 to dc8b19a Compare January 10, 2024 18:32
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
@RyanL1997 RyanL1997 added the enhancement New feature or request label Jan 10, 2024
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: Ryan Liang <jiallian@amazon.com>
@RyanL1997
Copy link
Contributor Author

Newly changed lint file set is like this: https://github.com/opensearch-project/dashboards-observability/actions/runs/7481679764/job/20363744276?pr=1323#step:10:23

@mengweieric mengweieric merged commit d17ec69 into opensearch-project:main Jan 10, 2024
8 of 18 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 10, 2024
* Fix the datasources cypress tests

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Clean up test setup and setup constants

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Refactor datasources test and disable security config

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Fix some lint

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Switch to capitalized naming

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Fix lint

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* debugging

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* List out all the changed files

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Add output for file changes

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Switch to use env var

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Add echo

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Remove github env setup

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Add msg to check which file is being linted

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Use jq for parsing

Signed-off-by: Ryan Liang <jiallian@amazon.com>

* Lint both modified and added

Signed-off-by: Ryan Liang <jiallian@amazon.com>

---------

Signed-off-by: Ryan Liang <jiallian@amazon.com>
(cherry picked from commit d17ec69)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ps48 pushed a commit that referenced this pull request Jan 11, 2024
* Fix the datasources cypress tests



* Clean up test setup and setup constants



* Refactor datasources test and disable security config



* Fix some lint



* Switch to capitalized naming



* Fix lint



* debugging



* List out all the changed files



* Add output for file changes



* Switch to use env var



* Add echo



* Remove github env setup



* Add msg to check which file is being linted



* Use jq for parsing



* Lint both modified and added



---------


(cherry picked from commit d17ec69)

Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
RyanL1997 pushed a commit to RyanL1997/dashboards-observability that referenced this pull request Apr 18, 2024
…ensearch-project#1323) (opensearch-project#1340)

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Peter Nied <petern@amazon.com>
(cherry picked from commit d5441b2af755806e175a2ed5aa136690e74ba07f)

Co-authored-by: Craig Perkins <cwperx@amazon.com>
amsiglan pushed a commit to amsiglan/dashboards-observability that referenced this pull request Jun 7, 2024
…ect#1323) (opensearch-project#1351)

* Fix the datasources cypress tests

* Clean up test setup and setup constants

* Refactor datasources test and disable security config

* Fix some lint

* Switch to capitalized naming

* Fix lint

* debugging

* List out all the changed files

* Add output for file changes

* Switch to use env var

* Add echo

* Remove github env setup

* Add msg to check which file is being linted

* Use jq for parsing

* Lint both modified and added

---------

(cherry picked from commit d17ec69)

Signed-off-by: Ryan Liang <jiallian@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 3abd951)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants