Skip to content

Cypress Test fixes for event-analytics#456

Merged
derek-ho merged 12 commits intoopensearch-project:mainfrom
TackAdam:main
May 15, 2023
Merged

Cypress Test fixes for event-analytics#456
derek-ho merged 12 commits intoopensearch-project:mainfrom
TackAdam:main

Conversation

@TackAdam
Copy link
Copy Markdown
Collaborator

@TackAdam TackAdam commented May 8, 2023

Description

Fixed the re-naming of event-analytics links causing the testing to fail.
Fixed existing bugs in some of the test so each run successfully

Issues Resolved

[List any issues this PR will resolve]

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.

});
});

describe('Saves a query on explorer page', () => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why this test was moved?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Order was impacting an error occurring on cypress's side labeled as a "node_modules/async-wait-until/src/waitUntil.js:53:1" putting this test at the end / commenting it out allowed the rest of the test to run successfully.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We may want to know the exact error here to fix it without changing the order.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I want to know why order should matter?

TLDR; The test suite should run in any order. This is imperative.
IF there is a test that is order-dependent, then it is an erroneous test and is high/critical on the list of things-to-fix.

In the course of debugging, this describe() was definitively causing the test-suite to fail.
Moving this test simplified debugging and will simplify fixing it in a subsequent PR.

Tackett added 3 commits May 9, 2023 09:22
Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
package.json Outdated
"@types/react-test-renderer": "^16.9.1",
"antlr4ts-cli": "^0.5.0-alpha.4",
"cypress": "^6.0.0",
"cypress": "12.11.0",
Copy link
Copy Markdown
Collaborator

@mengweieric mengweieric May 9, 2023

Choose a reason for hiding this comment

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

Maybe better to upgrade it in a separate PR after we have all-green status for all the tests, and you may also use ^ as it will pick up the latest compatible version

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are some test failures that dont' seem to be fixable in Cypress 6.
Cypress has had significant refactors (6 Majors!). The internal browser-automation has been changed. (Investigating 6 majors worth the code-changes is infeasible).
Some browser-automation techinques from Cypress 6 -> 12.x have changed in ways that are difficult to document. The APIs have changed, which provides a small clue. Other changes are simply observed and trial-and-error + intuition is necessary to determine the "correct" repeatable automation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we use consistent cypress version with functional test repo?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why cypress.json removed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Has to be removed in order to run Cypress version 12, replaced with cypress.config.js (Pending commit depending which version we end up using)

Tackett and others added 5 commits May 9, 2023 11:19
Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
Signed-off-by: TackAdam <navytackett@hotmail.com>
Signed-off-by: TackAdam <navytackett@hotmail.com>
@TackAdam
Copy link
Copy Markdown
Collaborator Author

Removed version upgrade as it will be a separate PR pending discussion on #411.
This PR now just fixes the url naming conventions and wait commands.

@codecov
Copy link
Copy Markdown

codecov bot commented May 11, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@0a015f8). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #456   +/-   ##
=======================================
  Coverage        ?   42.82%           
=======================================
  Files           ?      299           
  Lines           ?    17782           
  Branches        ?     4353           
=======================================
  Hits            ?     7616           
  Misses          ?    10125           
  Partials        ?       41           
Flag Coverage Δ
dashboards-observability 42.82% <0.00%> (?)

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

Signed-off-by: TackAdam <navytackett@hotmail.com>
TackAdam added 2 commits May 12, 2023 09:54
Signed-off-by: TackAdam <navytackett@hotmail.com>
Signed-off-by: TackAdam <navytackett@hotmail.com>
Copy link
Copy Markdown
Collaborator

@derek-ho derek-ho left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Copy Markdown
Collaborator

@kavithacm kavithacm left a comment

Choose a reason for hiding this comment

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

LGTM!

@derek-ho derek-ho merged commit 2f518c3 into opensearch-project:main May 15, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 18, 2023
* Cypress fixes for EventAnalytics

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Cypress fixes for EventAnalytics

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed Skip from timestamp, modified config to work for Cypress v12

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed all waits and replaced with timeouts, per Eric's guidance

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Added back cypress.json and added ^ to version

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Added back cypress.json and added ^ to version

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed version upgrade, changed order back

Signed-off-by: TackAdam <navytackett@hotmail.com>

* Removed version upgrade from package.json

Signed-off-by: TackAdam <navytackett@hotmail.com>

* Yarn lock regenerated

Signed-off-by: TackAdam <navytackett@hotmail.com>

* yarn.lock reset to origins file

Signed-off-by: TackAdam <navytackett@hotmail.com>

* cypress.json  reset to origins file

Signed-off-by: TackAdam <navytackett@hotmail.com>

---------

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
Signed-off-by: TackAdam <navytackett@hotmail.com>
Co-authored-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
(cherry picked from commit 2f518c3)
TackAdam added a commit to TackAdam/dashboards-observability that referenced this pull request Jul 18, 2023
* Cypress fixes for EventAnalytics

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Cypress fixes for EventAnalytics

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed Skip from timestamp, modified config to work for Cypress v12

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed all waits and replaced with timeouts, per Eric's guidance

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Added back cypress.json and added ^ to version

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Added back cypress.json and added ^ to version

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed version upgrade, changed order back

Signed-off-by: TackAdam <navytackett@hotmail.com>

* Removed version upgrade from package.json

Signed-off-by: TackAdam <navytackett@hotmail.com>

* Yarn lock regenerated

Signed-off-by: TackAdam <navytackett@hotmail.com>

* yarn.lock reset to origins file

Signed-off-by: TackAdam <navytackett@hotmail.com>

* cypress.json  reset to origins file

Signed-off-by: TackAdam <navytackett@hotmail.com>

---------

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
Signed-off-by: TackAdam <navytackett@hotmail.com>
Co-authored-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
(cherry picked from commit 2f518c3)
Signed-off-by: TackAdam <navytackett@hotmail.com>
amsiglan pushed a commit to amsiglan/dashboards-observability that referenced this pull request Jun 7, 2024
* Cypress fixes for EventAnalytics

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Cypress fixes for EventAnalytics

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed Skip from timestamp, modified config to work for Cypress v12

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed all waits and replaced with timeouts, per Eric's guidance

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Added back cypress.json and added ^ to version

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Added back cypress.json and added ^ to version

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>

* Removed version upgrade, changed order back

Signed-off-by: TackAdam <navytackett@hotmail.com>

* Removed version upgrade from package.json

Signed-off-by: TackAdam <navytackett@hotmail.com>

* Yarn lock regenerated

Signed-off-by: TackAdam <navytackett@hotmail.com>

* yarn.lock reset to origins file

Signed-off-by: TackAdam <navytackett@hotmail.com>

* cypress.json  reset to origins file

Signed-off-by: TackAdam <navytackett@hotmail.com>

---------

Signed-off-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
Signed-off-by: TackAdam <navytackett@hotmail.com>
Co-authored-by: Tackett <tackadam@6cb133a01186.ant.amazon.com>
(cherry picked from commit 2f518c3)
(cherry picked from commit 2b22732)
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.

6 participants