sadpandajoe commented on code in PR #33002:
URL: https://github.com/apache/superset/pull/33002#discussion_r2408979483
##########
superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts:
##########
@@ -94,12 +94,15 @@ describe('Dashboards list', () => {
.should('be.checked')
.should('have.length', 6);
cy.getBySel('bulk-select-copy').contains('5 Selected');
- cy.getBySel('bulk-select-action')
- .should('have.length', 2)
- .then($btns => {
- expect($btns).to.contain('Delete');
- expect($btns).to.contain('Export');
- });
+
Review Comment:
I'd convert all these over to a component test.
##########
superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts:
##########
@@ -94,12 +94,57 @@ describe('Charts list', () => {
});
});
+ describe('list mode', () => {
+ before(() => {
+ visitChartList();
+ setGridMode('list');
+ });
+
+ it('should load rows in list mode', () => {
+ cy.getBySel('listview-table').should('be.visible');
+ cy.getBySel('sort-header').eq(1).contains('Name');
+ cy.getBySel('sort-header').eq(2).contains('Type');
+ cy.getBySel('sort-header').eq(3).contains('Dataset');
+ cy.getBySel('sort-header').eq(4).contains('On dashboards');
+ cy.getBySel('sort-header').eq(5).contains('Owners');
+ cy.getBySel('sort-header').eq(6).contains('Last modified');
+ cy.getBySel('sort-header').eq(7).contains('Actions');
+ });
+
+ it('should bulk select in list mode', () => {
+ toggleBulkSelect();
+ cy.get('[aria-label="Select all"]').click();
+ cy.get('input[type="checkbox"]:checked').should('have.length', 26);
+ cy.getBySel('bulk-select-copy').contains('25 Selected');
+ cy.getBySel('bulk-select-action').should('contain', 'Export');
+ cy.getBySel('bulk-select-action').trigger('mouseenter');
+ cy.getBySel('bulk-select-deselect-all').click();
+ cy.get('input[type="checkbox"]:checked').should('have.length', 0);
+ cy.getBySel('bulk-select-copy').contains('0 Selected');
+ cy.getBySel('bulk-select-action').should('not.exist');
+ });
+ });
describe('card mode', () => {
before(() => {
visitChartList();
setGridMode('card');
});
+ it('should load rows in card mode', () => {
+ cy.getBySel('listview-table').should('not.exist');
+ cy.getBySel('styled-card').should('have.length', 25);
+ });
+
+ it('should bulk select in card mode', () => {
Review Comment:
Same as the other tests.
##########
superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts:
##########
@@ -182,7 +188,19 @@ describe('Dashboards list', () => {
cy.getBySel('styled-card').eq(0).contains('1 - Sample
dashboard').click();
cy.getBySel('styled-card').eq(1).contains('2 - Sample
dashboard').click();
- cy.getBySel('bulk-select-action').eq(0).contains('Delete').click();
+
+ // Try to access Delete action (may not be available depending on
permissions)
+ cy.getBySel('bulk-select-action').trigger('mouseenter');
+ // Check if Delete is available in the dropdown
+ cy.get('body').then($body => {
+ if ($body.text().includes('Delete')) {
Review Comment:
Should not use if statements in tests since it may never hit one of the
statements.
##########
superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts:
##########
@@ -94,12 +94,57 @@ describe('Charts list', () => {
});
});
+ describe('list mode', () => {
+ before(() => {
+ visitChartList();
+ setGridMode('list');
+ });
+
+ it('should load rows in list mode', () => {
+ cy.getBySel('listview-table').should('be.visible');
+ cy.getBySel('sort-header').eq(1).contains('Name');
+ cy.getBySel('sort-header').eq(2).contains('Type');
+ cy.getBySel('sort-header').eq(3).contains('Dataset');
+ cy.getBySel('sort-header').eq(4).contains('On dashboards');
+ cy.getBySel('sort-header').eq(5).contains('Owners');
+ cy.getBySel('sort-header').eq(6).contains('Last modified');
+ cy.getBySel('sort-header').eq(7).contains('Actions');
+ });
+
+ it('should bulk select in list mode', () => {
Review Comment:
Same as above statement, you'll probably get more value moving these over to
component tests instead of e2e tests.
##########
superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts:
##########
@@ -94,12 +94,57 @@ describe('Charts list', () => {
});
});
+ describe('list mode', () => {
+ before(() => {
+ visitChartList();
+ setGridMode('list');
+ });
+
+ it('should load rows in list mode', () => {
Review Comment:
These should not be in e2e tests. There are no e2e actions, we're just
checking if headers are showing up.
##########
superset-frontend/cypress-base/cypress/e2e/chart_list/list.test.ts:
##########
@@ -94,12 +94,57 @@ describe('Charts list', () => {
});
});
+ describe('list mode', () => {
+ before(() => {
+ visitChartList();
+ setGridMode('list');
+ });
+
+ it('should load rows in list mode', () => {
+ cy.getBySel('listview-table').should('be.visible');
+ cy.getBySel('sort-header').eq(1).contains('Name');
+ cy.getBySel('sort-header').eq(2).contains('Type');
+ cy.getBySel('sort-header').eq(3).contains('Dataset');
+ cy.getBySel('sort-header').eq(4).contains('On dashboards');
+ cy.getBySel('sort-header').eq(5).contains('Owners');
+ cy.getBySel('sort-header').eq(6).contains('Last modified');
+ cy.getBySel('sort-header').eq(7).contains('Actions');
+ });
+
+ it('should bulk select in list mode', () => {
+ toggleBulkSelect();
+ cy.get('[aria-label="Select all"]').click();
+ cy.get('input[type="checkbox"]:checked').should('have.length', 26);
+ cy.getBySel('bulk-select-copy').contains('25 Selected');
+ cy.getBySel('bulk-select-action').should('contain', 'Export');
+ cy.getBySel('bulk-select-action').trigger('mouseenter');
+ cy.getBySel('bulk-select-deselect-all').click();
+ cy.get('input[type="checkbox"]:checked').should('have.length', 0);
+ cy.getBySel('bulk-select-copy').contains('0 Selected');
+ cy.getBySel('bulk-select-action').should('not.exist');
+ });
+ });
describe('card mode', () => {
before(() => {
visitChartList();
setGridMode('card');
});
+ it('should load rows in card mode', () => {
Review Comment:
Same with this one, we are just loading things on the page but there is no
e2e action.
##########
superset-frontend/cypress-base/cypress/e2e/dashboard_list/list.test.ts:
##########
@@ -197,7 +215,19 @@ describe('Dashboards list', () => {
cy.getBySel('table-row').eq(1).contains('4 - Sample dashboard');
cy.get('[data-test="table-row"] input[type="checkbox"]').eq(0).click();
cy.get('[data-test="table-row"] input[type="checkbox"]').eq(1).click();
- cy.getBySel('bulk-select-action').eq(0).contains('Delete').click();
+
+ // Try to access Delete action (may not be available depending on
permissions)
+ cy.getBySel('bulk-select-action').trigger('mouseenter');
+ // Check if Delete is available in the dropdown
+ cy.get('body').then($body => {
+ if ($body.text().includes('Delete')) {
Review Comment:
Should not use if statements in tests since it may never hit one of the
statements.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
