-
Notifications
You must be signed in to change notification settings - Fork 937
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
TESTID-66: histogram tests #9290
base: main
Are you sure you want to change the base?
Conversation
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
@silvaf-dev why skip changelog? |
@silvaf-dev could you upload a video? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9290 +/- ##
=======================================
Coverage 61.71% 61.71%
=======================================
Files 3816 3816
Lines 91829 91829
Branches 14543 14543
=======================================
Hits 56668 56668
Misses 31506 31506
Partials 3655 3655
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I tried to update but it seems only for author |
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
cy.wait(1000); | ||
cy.getElementByTestId('discoverIntervalSelect').should('have.value', 'auto'); | ||
cy.getElementByTestId('discoverIntervalDateRange').should('be.visible'); | ||
// TO DO: check histogram visualization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you missed two aspects in https://github.com/opensearch-project/OpenSearch-Dashboards/pull/9290/files for
Select index pattern with time fieldSet time range to cover multiple monthsVerify the default auto interval displays autoTest with different time intervals (Weekly, Day) and verify histogram displaySwitch to PPL and verify visualization remains consistent across languages
- we need to switch to different intervals
- we need to switch language to verify the state persist like if you are using DQL - INDEX PATTERN and week, you need to switch to PPL and Lucence and check it is still week
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I switch to week/day only?
I'm already checking persistence for week on it(`check histogram visibility for ${config.testName}`)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to switch and verify all the intervals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don;t think you verify the interval state is persistent in that test right? That is for visibility, like show or no show histogram.
When you change the interval, e.g. from auto to day and you switch language the interval should be day and should not go back to default auto.
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
Done! |
❌ Invalid Prefix For Manual Changeset CreationInvalid description prefix. Found "add histogram tests". Only "skip" entry option is permitted for manual commit of changeset files. If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description. |
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
...earch_dashboards/opensearch_dashboards/apps/query_enhancements/histogram_interaction.spec.js
Outdated
Show resolved
Hide resolved
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
❌ Invalid Prefix For Manual Changeset CreationInvalid description prefix. Found "test". Only "skip" entry option is permitted for manual commit of changeset files. If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description. |
cy.wait(1000); | ||
cy.getElementByTestId('discoverIntervalSelect').should('have.value', 'auto'); | ||
cy.getElementByTestId('discoverIntervalDateRange').should('be.visible'); | ||
// TO DO: check histogram visualization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don;t think you verify the interval state is persistent in that test right? That is for visibility, like show or no show histogram.
When you change the interval, e.g. from auto to day and you switch language the interval should be day and should not go back to default auto.
Signed-off-by: Federico Silva <[email protected]>
Signed-off-by: Federico Silva <[email protected]>
Signed-off-by: Federico Silva <[email protected]>
Description
This PR incorporates all four test suites from TESTID-66, save for the interactions with the inner elements of the histogram.
Issues Resolved
Screenshot
Screen.Recording.2025-01-28.at.5.1.mp4
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration