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

Cypress v12.8.1 - Fixes for apps #604

Merged
merged 4 commits into from
Jul 17, 2023
Merged

Conversation

TackAdam
Copy link
Collaborator

@TackAdam TackAdam commented Jul 3, 2023

Description

[Describe what this change achieves]
Fixes for cypress testing of apps.

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.

@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Merging #604 (f1cf307) into main (dbb4a8a) will increase coverage by 0.47%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #604      +/-   ##
==========================================
+ Coverage   43.12%   43.59%   +0.47%     
==========================================
  Files         303      312       +9     
  Lines       18008    18581     +573     
  Branches     4434     4479      +45     
==========================================
+ Hits         7766     8101     +335     
- Misses       9709    10438     +729     
+ Partials      533       42     -491     
Flag Coverage Δ
dashboards-observability 43.59% <ø> (+0.47%) ⬆️

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

see 115 files with indirect coverage changes

@TackAdam
Copy link
Collaborator Author

TackAdam commented Jul 6, 2023

Screenshot 2023-07-03 at 10 34 49 AM All 27 test passing

@@ -85,15 +87,15 @@ describe('Creating application', () => {
cy.get('[data-test-subj="createButton"]').should('not.be.disabled');
cy.get('[data-test-subj="createAndSetButton"]').should('be.disabled');
expectMessageOnHover('createAndSetButton', 'Log source is required to set availability.');
cy.get('[data-test-subj="searchAutocompleteTextArea"]').focus().type(baseQuery, {delay: TYPING_DELAY});
cy.get('[data-test-subj="searchAutocompleteTextArea"]').focus().type(' ' + baseQuery);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2023-06-29 at 2 23 47 PM This field seemed to incorrectly type regardless of delay applied. The error always occurred in the first 3-5 letters, adding whitespace allows it to execute correctly and the whitespace is ignored in the application. This greatly increases run-speed as well as preventing the mis-typed log source.

Copy link
Member

Choose a reason for hiding this comment

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

Good to know this hack 😄 Also, did you try looking deeper on why it fails to type correctly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't able to find any specific cause for the mis-typing just that it is a known issue and most people add very long delays to attempt to fix it. This field specifically was only encountering the error at the start after testing a lot of possible fixes / ideas I had this seemed to be the only 100% reliable one.

@TackAdam
Copy link
Collaborator Author

TackAdam commented Jul 6, 2023

Screenshot 2023-07-03 at 12 56 07 PM Occasionally the cypress testing will open an application and the header and breadcrumb will not be present. This causes the test to fail unless manually re-opening the application. This bug seems to be only on the cypress side. Have not found a 100% guaranteed fix for this error but reduced it's frequency through wait actions and additional checks.

.cypress/utils/app_constants.js Outdated Show resolved Hide resolved
@@ -85,15 +87,15 @@ describe('Creating application', () => {
cy.get('[data-test-subj="createButton"]').should('not.be.disabled');
cy.get('[data-test-subj="createAndSetButton"]').should('be.disabled');
expectMessageOnHover('createAndSetButton', 'Log source is required to set availability.');
cy.get('[data-test-subj="searchAutocompleteTextArea"]').focus().type(baseQuery, {delay: TYPING_DELAY});
cy.get('[data-test-subj="searchAutocompleteTextArea"]').focus().type(' ' + baseQuery);
Copy link
Member

Choose a reason for hiding this comment

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

Good to know this hack 😄 Also, did you try looking deeper on why it fails to type correctly?

Signed-off-by: TackAdam <[email protected]>
cy.get(`[data-test-subj="${name}ApplicationLink"]`).click();
cy.wait(delay);
cy.get('.euiTableRow').should('have.length.lessThan', 1);//Replaces Wait
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does less than 1 mean? You are looking for an empty table? Can we replace this to make it more clear?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is checking to see that that the clicking on the Application Link has fully loaded before searching for the name of the application. The table no longer exist on the new page so this ensures the loading completes and delays the next command "find applicationTitle" from occurring till it is on the page and able to be found.

cy.get('[title="Bar"]').click();
cy.wait(delay);
cy.get('[data-test-subj="comboBoxInput"]').click();
cy.focused().type('{downArrow}');
Copy link
Collaborator

Choose a reason for hiding this comment

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

duplicate to line below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was used to select TimeSeries as the third option by going down twice, adding a commit to make this more clear and select it in a better way. It previously selected Horizontal Bar which did not have the availability function.

@@ -372,11 +376,12 @@ describe('Viewing application', () => {
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.get('[data-test-subj="eventExplorer__saveManagementPopover"]').click();
cy.get('[data-test-subj="eventExplorer__querySaveConfirm"]').click();
cy.wait(delay*5);
Copy link
Collaborator

Choose a reason for hiding this comment

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

necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Think its safe to remove, was a problem originally but seems to be working without.


cy.wait(delay);
cy.get('[data-test-subj="comboBoxInput"]').click();
cy.focused().type('{downArrow}');
Copy link
Collaborator

Choose a reason for hiding this comment

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

duplicate to below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was used to select TimeSeries as the third option by going down twice, adding a commit to make this more clear and select it in a better way. It previously selected Horizontal Bar which did not have the availability function.

cy.get('[data-test-subj="superDatePickerShowDatesButton"]').should('contain', 'Last 24 months');
cy.get('.euiTab[id="availability-panel"]').click();
cy.get('[title="Bar"]').click();

cy.wait(delay);
Copy link
Collaborator

Choose a reason for hiding this comment

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

necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I believe a button use to be required to be hit before adding availability that no longer exist.

@@ -325,19 +330,15 @@ describe('Viewing application', () => {

it('Saves visualization #1 to panel', () => {
cy.get('[data-test-subj="app-analytics-panelTab"]').click();
cy.get('[data-test-subj="addVisualizationButton"]').first().click();
cy.wait(delay);
cy.get('[data-test-subj="addVisualizationButton"]').last().click();
Copy link
Collaborator

Choose a reason for hiding this comment

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

why changing from first to last?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both options perform the same, not sure why i switched it, replacing it with first as it was originally

cy.get(`[data-test-subj="${nameTwo}ApplicationLink"]`).click();
cy.get('.euiTableRow').should('have.length.lessThan', 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

replace with 0 or something more clear?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2023-07-17 at 10 57 17 AM

Other objects on the new page are considered table but have nothing in them so below 0 doesn't work.

@TackAdam TackAdam merged commit 4e33b99 into opensearch-project:main Jul 17, 2023
9 of 10 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:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-604-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4e33b99660a0250448cad1d8aede10429df8a313
# Push it to GitHub
git push --set-upstream origin backport/backport-604-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

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

TackAdam added a commit to TackAdam/dashboards-observability that referenced this pull request Jul 19, 2023
Cypress v12.8.1 - Fixes for apps

(cherry picked from commit 4e33b99)
Signed-off-by: TackAdam <[email protected]>
amsiglan pushed a commit to amsiglan/dashboards-observability that referenced this pull request Jun 7, 2024
Cypress v12.8.1 - Fixes for apps

(cherry picked from commit 4e33b99)
Signed-off-by: TackAdam <[email protected]>
(cherry picked from commit b0ced99)
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.

3 participants