You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/09/05 15:16:09 UTC

[GitHub] [superset] geido opened a new pull request, #21332: chore: Cypress runtime enhancements

geido opened a new pull request, #21332:
URL: https://github.com/apache/superset/pull/21332

   ### SUMMARY
   This PR is part of a tech debt initiative. It proposes several enhancements to the Cypress suite and runtime improvements, as follows:
   
   - It reduces at the minimum the number of page reloads. Before this PR, Cypress would take X locally. With this PR it takes Y.
   - Introduces a new pattern to create sample charts and dashboards on the fly via fixtures
   - It introduces certain Cypress best standards, such as preferring `data-test` and provides utils such as `getBySel` 
   - It create several util functions that can be used across the entire suite or for specific areas
   - It adds the `_skip` prefix to those files that had all their tests skipped
   - It reorganizes the directory structure and places tests that fall under the same category within the same test files
   
   This PR enhances the dashboard, dashboard list and chart list suites.
   
   ### WHAT'S NEXT
   - All the `_skip` files should be fixed and their tests enhanced following the new patterns
   - Implement the same patterns for the remaining tests, such as directories database, dataset, explore, sqllab, alerts_and_reports
   
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #21332: chore: Cypress runtime enhancements

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #21332:
URL: https://github.com/apache/superset/pull/21332#issuecomment-1237235055

   # [Codecov](https://codecov.io/gh/apache/superset/pull/21332?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#21332](https://codecov.io/gh/apache/superset/pull/21332?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ac2219a) into [master](https://codecov.io/gh/apache/superset/commit/c3f841713989634ef4ba522b6a89e04ff89e2c0d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3f8417) will **decrease** coverage by `11.39%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #21332       +/-   ##
   ===========================================
   - Coverage   66.56%   55.16%   -11.40%     
   ===========================================
     Files        1790     1790               
     Lines       68393    68393               
     Branches     7279     7279               
   ===========================================
   - Hits        45526    37732     -7794     
   - Misses      20990    28784     +7794     
     Partials     1877     1877               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `52.94% <ø> (ø)` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `52.84% <ø> (ø)` | |
   | python | `57.81% <ø> (-23.54%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `50.86% <ø> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/21332?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...frontend/src/components/Chart/DrillDetailModal.tsx](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnQvRHJpbGxEZXRhaWxNb2RhbC50c3g=) | `100.00% <ø> (ø)` | |
   | [...end/src/components/PageHeaderWithActions/index.tsx](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUGFnZUhlYWRlcldpdGhBY3Rpb25zL2luZGV4LnRzeA==) | `90.00% <ø> (ø)` | |
   | [...ashboard/components/BuilderComponentPane/index.tsx](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0J1aWxkZXJDb21wb25lbnRQYW5lL2luZGV4LnRzeA==) | `76.47% <ø> (ø)` | |
   | [...frontend/src/dashboard/components/Header/index.jsx](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0hlYWRlci9pbmRleC5qc3g=) | `60.29% <ø> (ø)` | |
   | [...src/dashboard/components/PropertiesModal/index.tsx](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC9pbmRleC50c3g=) | `62.19% <ø> (ø)` | |
   | [...dashboard/components/SliceHeaderControls/index.tsx](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyQ29udHJvbHMvaW5kZXgudHN4) | `70.21% <ø> (ø)` | |
   | [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-88.89%)` | :arrow_down: |
   | [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
   | [superset/key\_value/commands/delete\_expired.py](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZV9leHBpcmVkLnB5) | `0.00% <0.00%> (-80.77%)` | :arrow_down: |
   | ... and [289 more](https://codecov.io/gh/apache/superset/pull/21332/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on a diff in pull request #21332: chore: Cypress runtime enhancements

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #21332:
URL: https://github.com/apache/superset/pull/21332#discussion_r967262764


##########
superset-frontend/cypress-base/cypress/integration/alerts_and_reports/reports.test.ts:
##########
@@ -16,31 +16,26 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { REPORT_LIST } from './alert_report.helper';
+import { REPORT_LIST } from 'cypress/utils/urls';
 
-describe('report list view', () => {
-  beforeEach(() => {
-    cy.login();
+describe('Report list view', () => {
+  before(() => {
+    cy.visit(REPORT_LIST);
   });
 
-  afterEach(() => {
-    cy.eyesClose();
+  beforeEach(() => {
+    cy.preserveLogin();
   });
 
   it('should load report lists', () => {
-    cy.visit(REPORT_LIST);
-
-    cy.get('[data-test="listview-table"]').should('be.visible');
-    // check report list view header
-    cy.get('[data-test="sort-header"]').eq(1).contains('Last run');
-    cy.get('[data-test="sort-header"]').eq(2).contains('Name');
-    cy.get('[data-test="sort-header"]').eq(3).contains('Schedule');
-    cy.get('[data-test="sort-header"]').eq(4).contains('Notification method');
-    cy.get('[data-test="sort-header"]').eq(5).contains('Created by');
-    cy.get('[data-test="sort-header"]').eq(6).contains('Owners');
-    cy.get('[data-test="sort-header"]').eq(7).contains('Modified');
-    // TODO: this assert is flaky, we need to find a way to make it work consistenly
-    // cy.get('[data-test="sort-header"]').eq(7).contains('Active');
-    // cy.get('[data-test="sort-header"]').eq(8).contains('Actions');

Review Comment:
   Looks like you fixed the `Active` one. Is `Actions` a lost cause, or should we leave the commented line in for now?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] geido merged pull request #21332: chore: Cypress runtime enhancements

Posted by GitBox <gi...@apache.org>.
geido merged PR #21332:
URL: https://github.com/apache/superset/pull/21332


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] geido commented on a diff in pull request #21332: chore: Cypress runtime enhancements

Posted by GitBox <gi...@apache.org>.
geido commented on code in PR #21332:
URL: https://github.com/apache/superset/pull/21332#discussion_r968193145


##########
superset-frontend/cypress-base/cypress/integration/alerts_and_reports/reports.test.ts:
##########
@@ -16,31 +16,26 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { REPORT_LIST } from './alert_report.helper';
+import { REPORT_LIST } from 'cypress/utils/urls';
 
-describe('report list view', () => {
-  beforeEach(() => {
-    cy.login();
+describe('Report list view', () => {
+  before(() => {
+    cy.visit(REPORT_LIST);
   });
 
-  afterEach(() => {
-    cy.eyesClose();
+  beforeEach(() => {
+    cy.preserveLogin();
   });
 
   it('should load report lists', () => {
-    cy.visit(REPORT_LIST);
-
-    cy.get('[data-test="listview-table"]').should('be.visible');
-    // check report list view header
-    cy.get('[data-test="sort-header"]').eq(1).contains('Last run');
-    cy.get('[data-test="sort-header"]').eq(2).contains('Name');
-    cy.get('[data-test="sort-header"]').eq(3).contains('Schedule');
-    cy.get('[data-test="sort-header"]').eq(4).contains('Notification method');
-    cy.get('[data-test="sort-header"]').eq(5).contains('Created by');
-    cy.get('[data-test="sort-header"]').eq(6).contains('Owners');
-    cy.get('[data-test="sort-header"]').eq(7).contains('Modified');
-    // TODO: this assert is flaky, we need to find a way to make it work consistenly
-    // cy.get('[data-test="sort-header"]').eq(7).contains('Active');
-    // cy.get('[data-test="sort-header"]').eq(8).contains('Actions');

Review Comment:
   Good catch. I think I just forgot to keep that comment. Cypress won't recognize that one. There is something weird going on with it. For now, I'll bring the comment back.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org