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 Test] Add and Refactor TESTID-140 sidebar spec and TESTID-46,47,49 sharing spec #9154

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Jan 8, 2025

Description

Issues Resolved

Partially #8946
#8954
#8952
#8953

Screenshot

Changelog

  • test: [Cypress Test] Add and Refactor TESTID-140 sidebar spec and clean up

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 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.70%. Comparing base (6c83d4e) to head (88d64b5).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9154      +/-   ##
==========================================
- Coverage   61.70%   61.70%   -0.01%     
==========================================
  Files        3816     3816              
  Lines       91824    91824              
  Branches    14542    14542              
==========================================
- Hits        56664    56662       -2     
- Misses      31506    31507       +1     
- Partials     3654     3655       +1     
Flag Coverage Δ
Linux_1 28.99% <ø> (ø)
Linux_2 56.46% <ø> (ø)
Linux_3 39.18% <ø> (-0.01%) ⬇️
Linux_4 28.91% <ø> (ø)
Windows_1 29.00% <ø> (ø)
Windows_2 56.41% <ø> (ø)
Windows_3 39.18% <ø> (ø)
Windows_4 28.91% <ø> (ø)

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.

@ananzh ananzh force-pushed the refactor-testid-140 branch 2 times, most recently from bd60d37 to c6e7933 Compare January 21, 2025 20:41
cy.get('.view-line').type(query);
// Send query
cy.getElementByTestId('querySubmitButton').click();
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

removed

*/

/**
* Returns the SideBarTestConfig for the provided dataset, datasetType, and language
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Returns the SideBarTestConfig for the provided dataset, datasetType, and language
* Returns the FieldDisplayFilteringTestConfig for the provided dataset, datasetType, and language

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

* @param {string} dataset - the dataset name
* @param {QueryEnhancementDataset} datasetType - the type of the dataset
* @param {QueryEnhancementLanguageData} language - the relevant data for the query language to use
* @returns {SideBarTestConfig}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @returns {SideBarTestConfig}
* @returns {FieldDisplayFilteringTestConfig}

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

* @property {string} dataset - the dataset name to use
* @property {QueryEnhancementDataset} datasetType - the type of dataset
* @property {QueryEnhancementLanguage} language - the name of query language as it appears in the dashboard app
* @property {boolean} isFilterButtonsEnabled - whether filter button is enabled for this permutation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @property {boolean} isFilterButtonsEnabled - whether filter button is enabled for this permutation

Copy link
Member Author

Choose a reason for hiding this comment

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

done

* @returns {object[]}
*/
export const generateAllTestConfigurations = (generateTestConfigurationCallback) => {
export const generateAllTestConfigurations = (generateTestConfigurationCallback, options = {}) => {
const { indexPattern = INDEX_PATTERN_WITH_TIME, index = INDEX_WITH_TIME_1 } = options;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with this change, but looking at it's usage in this PR, it looks like the only change is:

indexPattern: 'data_logs_small_time_1*',

Can't we just change this to data_logs_small_time_*?

Copy link
Contributor

Choose a reason for hiding this comment

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

But irreglardless of my questions, lets keep this change (i found an issue when migrating that'll make this useful!)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I want it to be more general. So you can define what is index patten and index.

config
) => {
// Helper functions
const getDocTableHeaderByIndex = (index) =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to move these helper functions into the sidebar.js

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 am considering it. But it should be a function in table canvas which is TESTID-147. Will update when raise the PR for TESTID-147.

};

describe('sidebar spec', () => {
before(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should change the before into beforeEach and consolidate the after and afterEach blocks

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 will remove before. I have kept one in config which should not affect.

@ananzh ananzh force-pushed the refactor-testid-140 branch from c6e7933 to c42e778 Compare January 27, 2025 19:29
@ananzh ananzh changed the title [Cypress Test] Add and Refactor TESTID-140 sidebar spec and clean up [Cypress Test] Add and Refactor TESTID-140 sidebar spec and TESTID-46,47,49 sharing spec Jan 27, 2025
opensearch-changeset-bot bot added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Jan 27, 2025
@ananzh ananzh force-pushed the refactor-testid-140 branch 2 times, most recently from 98f25c0 to 38fb073 Compare January 28, 2025 00:53
@ananzh ananzh mentioned this pull request Jan 28, 2025
7 tasks
@ananzh ananzh force-pushed the refactor-testid-140 branch from 38fb073 to 44819bf Compare January 28, 2025 22:08
LDrago27
LDrago27 previously approved these changes Jan 29, 2025
@ananzh ananzh force-pushed the refactor-testid-140 branch 2 times, most recently from c5e094a to c4193be Compare January 29, 2025 23:30
opensearch-changeset-bot bot added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Jan 29, 2025
@ananzh ananzh force-pushed the refactor-testid-140 branch from c58b86f to b1378d4 Compare January 29, 2025 23:36
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

Approving the PR but lets look into why we need to tear down and add data every time to OpenSearch. Seems very inefficient

interval: 'w',
filter: ['category', 'Network'],
};
beforeEach(() => {
Copy link
Member

Choose a reason for hiding this comment

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

This seems really inefficient! Why do we need to do this for every test? Our test suite will blow up!

Copy link
Member Author

Choose a reason for hiding this comment

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

this is for @AMoo-Miki 's future test plan design to isolate each test cases and run parallelly.

@ananzh ananzh merged commit a50e0c7 into opensearch-project:main Jan 30, 2025
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-9154-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a50e0c74f5bf87903ec6b6d82c131d1fc20b4a29
# Push it to GitHub
git push --set-upstream origin backport/backport-9154-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-9154-to-2.x.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.19 failed:

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

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.19 2.19
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.19
# Create a new branch
git switch --create backport/backport-9154-to-2.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a50e0c74f5bf87903ec6b6d82c131d1fc20b4a29
# Push it to GitHub
git push --set-upstream origin backport/backport-9154-to-2.19
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.19

Then, create a pull request where the base branch is 2.19 and the compare/head branch is backport/backport-9154-to-2.19.

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.

4 participants