Add Discover Table Tests #1182
Add Discover Table Tests #1182
Conversation
LDrago27
requested review from
xluo-aws,
Hailong-am,
SuZhou-Joe,
ruanyl,
wanglam and
raintygao
as code owners
| cy.waitForLoader(); | ||
| }); | ||
|
|
||
| after(() => {}); |
There was a problem hiding this comment.
@Hailong-am Removed it in the latest version of the PR.
| // Setting up the page | ||
| describe('discover_table', () => { | ||
| beforeEach(() => { | ||
| if (Cypress.env('SECURITY_ENABLED')) { |
There was a problem hiding this comment.
I don't think we need this if (Cypress.env('SECURITY_ENABLED')) { since this path cypress/integration/core-opensearch-dashboards/opensearch-dashboards/apps/data_explorer is different from cypress/integration/core-opensearch-dashboards/opensearch-dashboards/dashboard_share_copy_link_test.js which will be tested in component and Integ in release pipeline. Most security related ones are in cypress/integration/plugins/security. For data_explorer, we don't need to test security.
Seems our current tests are not consistency. I see one test has check this cypress/integration/core-opensearch-dashboards/opensearch-dashboards/apps/data_explorer/discover.spec.js but all the rest don't. Maybe we should clean this one as well.
There was a problem hiding this comment.
I have updated both the tests to remove the check specific to security.
| cy.waitForSearch(); | ||
| }); | ||
|
|
||
| describe('no line wrapping in legacy table', () => { |
There was a problem hiding this comment.
no line wrapping in legacy table --> auto line wrapping in legacy table
There was a problem hiding this comment.
Updated the description.
| }); | ||
|
|
||
| describe('no line wrapping in legacy table', () => { | ||
| it('checks that there is no line wrapping by default in the legacy table', function () { |
There was a problem hiding this comment.
same here. Legacy table can auto do line wrapping with additional setting.
There was a problem hiding this comment.
Updated the description.
| // last element is _scrore if there is wrapping this field won't be present | ||
| // So we check for the presence of the _score element in the legacy table | ||
|
|
||
| cy.get('.euiDescriptionList__title').should('contain.text', '_score'); |
| }); | ||
| }); | ||
|
|
||
| describe('maxHeight With No truncation', () => { |
There was a problem hiding this comment.
might be cleaner if put maxHeight With No truncation and maxHeight With truncation together.
describe('maxHeight truncation behavior', () => {
it('checks if the table respects maxHeight setting with truncation', function () {
cy.setAdvancedSetting({
'truncate:maxHeight': 130,
});
cy.get('.truncate-by-height')
.first()
.should('have.css', 'max-height', '130px');
});
it('checks if the table respects maxHeight setting without truncation', function () {
cy.setAdvancedSetting({
'truncate:maxHeight': 0,
});
cy.get('.truncate-by-height')
.first()
.should('not.have.css', 'max-height', '130px');
});
});
There was a problem hiding this comment.
Consolidated both the tests into one.
| cy.get('[data-test-subj="dataGridRowCell"]') | ||
| .first() | ||
| .invoke('height') | ||
| .should('be.gt', res); // Check if the new height is greater than previus height |
There was a problem hiding this comment.
Does this test include all table related issues? Cuz I see you only mentioned on in the description about multi document but it seems to me that this test file has covered at least two issues. It is better to include all of them in the description.
There was a problem hiding this comment.
All the tests related to the table will be housed within this file. This PR only covers the tests corresponding to opensearch-project/OpenSearch-Dashboards#6280. There will be follow up PR addressing other table related issues.
LDrago27
force-pushed
the
ftr
branch
7 times, most recently
from
7c03294
to
ebb903d
Compare
LDrago27
requested review from
tianleh,
kavilla,
ohltyler,
CCongWang,
ashwin-pc and
peterzhuamazon
as code owners
LDrago27
force-pushed
the
ftr
branch
4 times, most recently
from
b3f373f
to
86088bb
Compare
Signed-off-by: Suchit Sahoo <suchsah@amazon.com>
|
@SuZhou-Joe I have updated the tests to fix the failures. The newly added tests are now passing for both with and without security enabled. |
|
@LDrago27 Thanks for addressing all the failed cases. |
Signed-off-by: Suchit Sahoo <suchsah@amazon.com> (cherry picked from commit a964ed8)
Signed-off-by: Suchit Sahoo <suchsah@amazon.com>
Description
This adds functional tests for the issues described in opensearch-project/OpenSearch-Dashboards#6280.
Issues Resolved
opensearch-project/OpenSearch-Dashboards#5713
Issues Still Pending (Sub Issues within the Test Suite are mentioned within brackets)
The test suites are divided across two files discover_table and discover_advanced_settings. All the functional tests related to the Advanced Settings are grouped in discover_advanced_setting while the rest are part of discover_table settings.
Tests when run without security
discover_advanced_settings_no_security.spec.js.mp4
discover_table_no_security.spec.js.mp4
Tests when run with security
discover_advanced_settings_with_security.spec.js.mp4
discover_table_with_security.spec.js.mp4
Check List
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.