[Build] Add Github workflows #9490

ktmud · 2020-04-08T09:20:40Z

SUMMARY

Adding GitHub Actions per discussion in #9481. It is a pure addition to current build process for now. Nothing changes for Travis CI, except Cypress is upgraded from 3.6.1 to 4.3.0.

We may evaluate whether to disable Travis CI once GitHub Actions become more stable.

Jobs are split into 3 workflows to maximize parallelization:

superset-python.yml: python lint and unit tests
superset-frontend.yml: frontend lint and unit tests
superset-e2e.yml: end to end tests with Cypress

A custom GitHub Action was created to generalize some of the building steps, but not all of them. There are duplicate code about cache across files, while could potentially be lifted into the GitHub Action like the official Cypress GitHub action.

Some thoughts

GitHub Actions is less powerful than some other CI/CD tools, e.g., CircleCI and GitLab CI/CD, but overall it was a pleasant experience to use. Two major nuances are: 1. limited parallelization support, cannot organize jobs as DAGs; 2. cannot automatically cancel current jobs on new pushes.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

Make sure CI check pass.

ADDITIONAL INFORMATION

REVIEWERS

@john-bodley @dpgaspar @mistercrunch @craig-rueda

ktmud · 2020-04-08T09:26:52Z

See the builds in action: ktmud#25

dpgaspar

First pass

dpgaspar · 2020-04-08T14:38:55Z

.github/workflows/superset.yml

+  pull_request:
+    paths:
+    - superset/**/*.py
+    - tests/**


Should this include superset-frontend/** ?

This file is purely for python code. Frontend code checks are done in other workflows. Maybe I this rename this file to avoid confusion.

craig-rueda · 2020-04-08T15:26:02Z

Looks good - have you thought about Circle?

ktmud · 2020-04-08T15:42:41Z

CircleCI was my first choice, but it seems there are some administration overheads involved in setting it up and no other Apache projects are using it AFAIK.

codecov-io · 2020-04-09T04:46:30Z

Codecov Report

Merging #9490 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #9490   +/-   ##
=======================================
  Coverage   58.77%   58.77%           
=======================================
  Files         385      385           
  Lines       12239    12239           
  Branches     3022     3022           
=======================================
  Hits         7193     7193           
  Misses       4862     4862           
  Partials      184      184

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5565895...282b39e. Read the comment docs.

ktmud

I think this is ready for another review. Everything finally works!

ktmud · 2020-04-09T06:44:30Z

.github/actions/setup-superset/action.yml

+    description: additional commands to run
+  parallel:
+    required: false
+    description: whether to run additional commands in parallel


The parallel mode is not currently in use.

ktmud · 2020-04-09T06:49:44Z

.github/workflows/superset-e2e.yml

+      redis:
+        image: redis:5-alpine
+        ports:
+          - 16379:6379


GitHub-hosted action runners have some softwares pre-installed, including MySQL, Redis, and PostgreSQL.

Here we map all database services to a new port to make sure we don't accidentally use the build-in services that may be running on default ports already.

ktmud · 2020-04-09T07:23:22Z

.github/workflows/superset-e2e.yml

+        path: |
+          ~/.npm
+          superset-frontend/.terser-plugin-cache/
+        key: ${{ runner.os }}-npm-${{ hashFiles('superset-frontend/package-lock.json') }}


Cypress plugin will handle its own cache. So not need to add cypress-base/package-lock.json as hash key.

ktmud · 2020-04-09T07:28:33Z

.github/workflows/superset-e2e.yml

+        wait-on: http://localhost:8081
+        wait-on-timeout: 10
+        spec: cypress/integration/*/*
+        record: true


I'm enabling recording as it's free anyway. Check the recorded logs here.

ktmud · 2020-04-09T07:29:04Z

superset-frontend/cypress-base/.gitignore

@@ -0,0 +1 @@
+cypress/screenshots/


Local builds may generate this folder.

ktmud · 2020-04-09T07:29:35Z

superset-frontend/cypress-base/cypress.json

  "video": false,
  "videoUploadOnPasses": false,
  "viewportWidth": 1280,
  "viewportHeight": 800,
-  "requestTimeout": 10000


The original config has duplicate requestTimeout.

ktmud · 2020-04-09T07:32:31Z

superset-frontend/cypress-base/cypress.json

  "video": false,
  "videoUploadOnPasses": false,
  "viewportWidth": 1280,
  "viewportHeight": 800,
-  "requestTimeout": 10000
+  "projectId": "dk2opw"


Updated to a project under my personal account for testing. Will need to change back to the official project once merged.

Someone with admin access on GitHub please add a new record key to GitHub Actions secrets under the name CYPRESS_RECORD_KEY.

ktmud · 2020-04-09T07:36:18Z

superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.js

@@ -75,7 +75,7 @@ export default () =>
        .trigger('mousedown', { which: 1 })
        .trigger('dragstart', { dataTransfer })
        .trigger('drag', {});
-      cy.get('.grid-content .dragdroppable')
+      cy.get('.grid-content div.grid-row.background--transparent')


The upgraded Cypress will error out because it was dragged to a zero-height element. This fixes the error by dragging to the last visible row instead of the invisible one.

ktmud

Updated the Cypress test cases even more.

ktmud · 2020-04-09T07:54:40Z

superset-frontend/cypress-base/cypress.json

@@ -1,13 +1,14 @@
 {
  "baseUrl": "http://localhost:8081",
  "chromeWebSecurity": false,
-  "defaultCommandTimeout": 20000,


No need to actually wait for 20s for default commands (e.g., click triggers).

ktmud · 2020-04-09T09:05:40Z

superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.js

-      cy.wait('@boxplotRequest');
-      cy.get('.grid-container .box_plot').should('be.exist');
+      // wait for box plot to appear
+      cy.get('.grid-container .box_plot');


A simple get will wait for the element to appear. No need to wait for the additional request.

ktmud · 2020-04-09T09:06:34Z

superset-frontend/cypress-base/cypress/integration/dashboard/load.js

+            const responseBody = await readResponseBlob(xhr.response.body);
+            expect(responseBody).to.have.property('error', null);
+            const sliceId = responseBody.form_data.slice_id;
+            cy.get(`#chart-id-${sliceId}`).should('be.visible');


Has to return a Promise for the test cases to be captured.

ktmud · 2020-04-09T09:07:58Z

superset-frontend/cypress-base/cypress/integration/dashboard/save.js

-      cy.route('POST', boxplotRequest).as('boxplotRequest');
-      cy.wait('@boxplotRequest');
-      cy.get('.grid-container .box_plot').should('be.exist');
+      cy.get('.grid-container .box_plot', { timeout: 5000 }); // wait for 5 secs


Again, a simple get will wait. Also, dashboardId may be out of sync when test cases are running in parallelized mode.

ktmud · 2020-04-09T09:08:33Z

superset-frontend/cypress-base/cypress/integration/dashboard/tabs.js

      cy.on('fail', err => {
-        expect(err.message).to.include('Timed out retrying');
+        expect(err.message).to.include('timed out waiting');


The error message changed in Cypress v4.

ktmud · 2020-04-09T22:00:37Z

superset-frontend/cypress-base/cypress/integration/explore/visualizations/big_number.js

-      cy.get('.chart-container .header_line');
-      cy.get('.chart-container .subheader_line');
+      cy.get('.chart-container .header-line');
+      cy.get('.chart-container .subheader-line');


explore/visualizations/index.test.js was probably unintentioally disabled by #6241 .

It has since stopped running for over a year now. A lot of these tests are now broken. It's easy to add them back, but we may want to do the fix in another PR. Keep the changes for BigNumber here just for reference.

As an replacement for Travis CI.

ktmud · 2020-04-09T22:39:20Z

Woking on further improvements, closing for now to avoid excessive notification.

willbarrett · 2020-04-10T17:03:18Z

Hey @ktmud - let us know if there's anything we at Preset can do to support you on this work. We're really feeling the Travis pain and want to help get to a solution.

ktmud force-pushed the github-actions branch from ab89723 to d6290d7 Compare April 8, 2020 09:26

dpgaspar requested a review from craig-rueda April 8, 2020 11:00

dpgaspar reviewed Apr 8, 2020

View reviewed changes

ktmud force-pushed the github-actions branch from 299d664 to 3689439 Compare April 9, 2020 04:05

ktmud force-pushed the github-actions branch 5 times, most recently from 04fd288 to fd5c4ec Compare April 9, 2020 07:39

ktmud commented Apr 9, 2020

View reviewed changes

ktmud force-pushed the github-actions branch from fd5c4ec to c10349b Compare April 9, 2020 07:53

ktmud commented Apr 9, 2020

View reviewed changes

ktmud force-pushed the github-actions branch 14 times, most recently from d25aed5 to 5114064 Compare April 9, 2020 20:32

ktmud force-pushed the github-actions branch 4 times, most recently from 288e466 to bf3288e Compare April 9, 2020 21:54

ktmud commented Apr 9, 2020

View reviewed changes

[Build] Add Github workflows

a714914

As an replacement for Travis CI.

ktmud force-pushed the github-actions branch from bf3288e to a714914 Compare April 9, 2020 22:16

ktmud closed this Apr 9, 2020

This was referenced Apr 12, 2020

[Build] Add Github workflows ktmud/superset#27

Closed

[Build] Add Github workflows #9510

Closed

[Build] Add Github workflows #9515

Closed

[Build] Add Github workflows #9517

Merged

ktmud mentioned this pull request Jun 25, 2020

build: upgrade Cypress and re-enable visualization tests #10158

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Build] Add Github workflows #9490

[Build] Add Github workflows #9490

ktmud commented Apr 8, 2020 •

edited

Loading

ktmud commented Apr 8, 2020

dpgaspar left a comment

dpgaspar Apr 8, 2020

ktmud Apr 8, 2020

craig-rueda commented Apr 8, 2020

ktmud commented Apr 8, 2020

codecov-io commented Apr 9, 2020 •

edited

Loading

ktmud left a comment

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020 •

edited

Loading

ktmud Apr 9, 2020

ktmud left a comment

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud Apr 9, 2020

ktmud commented Apr 9, 2020

willbarrett commented Apr 10, 2020

		@@ -0,0 +1 @@
		cypress/screenshots/

[Build] Add Github workflows #9490

[Build] Add Github workflows #9490

Conversation

ktmud commented Apr 8, 2020 • edited Loading

CATEGORY

SUMMARY

Some thoughts

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

REVIEWERS

ktmud commented Apr 8, 2020

dpgaspar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

craig-rueda commented Apr 8, 2020

ktmud commented Apr 8, 2020

codecov-io commented Apr 9, 2020 • edited Loading

Codecov Report

ktmud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktmud Apr 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktmud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktmud commented Apr 9, 2020

willbarrett commented Apr 10, 2020

ktmud commented Apr 8, 2020 •

edited

Loading

codecov-io commented Apr 9, 2020 •

edited

Loading

ktmud Apr 9, 2020 •

edited

Loading