Skip to content
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

[Build] Collect frontend code coverage from Cypress tests #9555

Merged
merged 3 commits into from
Apr 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 118 additions & 31 deletions .github/workflows/bashlib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
set -e

GITHUB_WORKSPACE=${GITHUB_WORKSPACE:-.}
ASSETS_MANIFEST="$GITHUB_WORKSPACE/superset/static/assets/manifest.json"

# Echo only when not in parallel mode
say() {
if [[ ${INPUT_PARALLEL^^} != 'TRUE' ]]; then
if [[ $(echo "$INPUT_PARALLEL" | tr '[:lower:]' '[:upper:]') != 'TRUE' ]]; then
echo "$1"
fi
}
Expand Down Expand Up @@ -67,41 +71,27 @@ build-assets() {
say "::endgroup::"
}

npm-build() {
if [[ $1 = '--no-cache' ]]; then
build-assets
build-assets-cached() {
cache-restore assets
if [[ -f "$ASSETS_MANIFEST" ]]; then
echo 'Skip frontend build because static assets already exist.'
else
cache-restore assets
if [[ -f $GITHUB_WORKSPACE/superset/static/assets/manifest.json ]]; then
echo 'Skip frontend build because static assets already exist.'
else
build-assets
cache-save assets
fi
build-assets
cache-save assets
fi
}

cypress-install() {
cd "$GITHUB_WORKSPACE/superset-frontend/cypress-base"

cache-restore cypress

say "::group::Install Cypress"
npm ci
say "::endgroup::"

cache-save cypress
}
build-instrumented-assets() {
cd "$GITHUB_WORKSPACE/superset-frontend"

testdata() {
cd "$GITHUB_WORKSPACE"
say "::group::Load test data"
# must specify PYTHONPATH to make `tests.superset_test_config` importable
export PYTHONPATH="$GITHUB_WORKSPACE"
superset db upgrade
superset load_test_users
superset load_examples --load-test-data
superset init
say "::group::Build static assets with JS instrumented for test coverage"
cache-restore instrumented-assets
if [[ -f "$ASSETS_MANIFEST" ]]; then
echo 'Skip frontend build because instrumented static assets already exist.'
else
npm run build-instrumented -- --no-progress
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a new build-instrumented npm script to build instrumented JS for Cypress: https://docs.cypress.io/guides/tooling/code-coverage.html#Instrumenting-code

cache-save instrumented-assets
fi
say "::endgroup::"
}

Expand Down Expand Up @@ -131,3 +121,100 @@ setup-mysql() {
EOF
say "::endgroup::"
}

testdata() {
cd "$GITHUB_WORKSPACE"
say "::group::Load test data"
# must specify PYTHONPATH to make `tests.superset_test_config` importable
export PYTHONPATH="$GITHUB_WORKSPACE"
superset db upgrade
superset load_test_users
superset load_examples --load-test-data
superset init
say "::endgroup::"
}

codecov() {
say "::group::Upload code coverage"
local codecovScript="${HOME}/codecov.sh"
# download bash script if needed
if [[ ! -f "$codecovScript" ]]; then
curl -s https://codecov.io/bash > "$codecovScript"
fi
bash "$codecovScript" "$@"
say "::endgroup::"
}

cypress-install() {
cd "$GITHUB_WORKSPACE/superset-frontend/cypress-base"

cache-restore cypress

say "::group::Install Cypress"
npm ci
say "::endgroup::"

cache-save cypress
}

# Run Cypress and upload coverage reports
cypress-run() {
cd "$GITHUB_WORKSPACE/superset-frontend/cypress-base"

local page=$1
local group=${2:-Default}
local cypress="./node_modules/.bin/cypress run"
local browser=${CYPRESS_BROWSER:-chrome}

say "::group::Run Cypress for [$page]"
if [[ -z $CYPRESS_RECORD_KEY ]]; then
Copy link
Member Author

@ktmud ktmud Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we only record Cypress runs when CYPRESS_RECORD_KEY is set in GitHub secretes.

$cypress --spec "cypress/integration/$page" --browser "$browser"
else
# additional flags for Cypress dashboard recording
$cypress --spec "cypress/integration/$page" --browser "$browser" --record \
--group "$group" --tag "${GITHUB_REPOSITORY},${GITHUB_EVENT_NAME}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must add quotes to allow spaces in parameters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, are you using shellcheck to lint these scripts? I recommend it:

brew install shellcheck
shellcheck script.sh

You could even add a lint scripts CI step ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried it out and it works great! Previously I was relying on Codacy.

fi

# don't add quotes to $record because we do want word splitting
say "::endgroup::"
}

cypress-run-all() {
# Start Flask and run it in background
# --no-debugger means disable the interactive debugger on the 500 page
# so errors can print to stderr.
local flasklog="${HOME}/flask.log"
local port=8081

nohup flask run --no-debugger -p $port > "$flasklog" 2>&1 < /dev/null &
local flaskProcessId=$!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pipe Flask logs to background, so the outputs can be nicely grouped:

image


cypress-run "*/*"

# Upload code coverage separately so each page can have separate flags
# -c will clean existing coverage reports, -F means add flags
codecov -cF "cypress"

# After job is done, print out Flask log for debugging
say "::group::Flask log for default run"
cat "$flasklog"
say "::endgroup::"

# Rerun SQL Lab tests with backend persist enabled
export SUPERSET_CONFIG=tests.superset_test_config_sqllab_backend_persist

# Restart Flask with new configs
kill $flaskProcessId
nohup flask run --no-debugger -p $port > "$flasklog" 2>&1 < /dev/null &
local flaskProcessId=$!

cypress-run "sqllab/*" "Backend persist"
codecov -cF "cypress"

say "::group::Flask log for backend persist"
cat "$flasklog"
say "::endgroup::"

# make sure the program exits
kill $flaskProcessId
}
28 changes: 16 additions & 12 deletions .github/workflows/caches.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@
const workspaceDirectory = process.env.GITHUB_WORKSPACE;
const homeDirectory = process.env.HOME;

const assetsConfig = {
path: [`${workspaceDirectory}/superset/static/assets`],
hashFiles: [
`${workspaceDirectory}/superset-frontend/src/**/*`,
`${workspaceDirectory}/superset-frontend/*.js`,
`${workspaceDirectory}/superset-frontend/*.json`,
],
// dont use restore keys as it may give an invalid older build
restoreKeys: '',
};

// Multi-layer cache definition
module.exports = {
pip: {
Expand All @@ -31,18 +42,11 @@ module.exports = {
path: [`${homeDirectory}/.npm`],
hashFiles: ['superset-frontend/package-lock.json'],
},
assets: {
path: [
`${workspaceDirectory}/superset/static/assets`,
],
hashFiles: [
`${workspaceDirectory}/superset-frontend/src/**/*`,
`${workspaceDirectory}/superset-frontend/*.json`,
`${workspaceDirectory}/superset-frontend/*.js`,
],
// dont use restore keys as it may give an invalid older build
restoreKeys: ''
},
assets: assetsConfig,
// use separate cache for instrumented JS files and regular assets
// one is built with `npm run build`,
// another is built with `npm run build-instrumented`
'instrumented-assets': assetsConfig,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use separate cache keys for regular assets and instrumented JS files that supports code coverage. The regular assets are not currently in use, though. We can potentially add a sanity check in superset-frontend.yml just to make sure npm run build passes. But I don't think that's necessary.

cypress: {
path: [`${homeDirectory}/.cache/Cypress`],
hashFiles: [
Expand Down
42 changes: 9 additions & 33 deletions .github/workflows/superset-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
name: Cypress
runs-on: ubuntu-18.04
strategy:
fail-fast: false
fail-fast: true
matrix:
browser: ['chrome']
env:
Expand All @@ -17,7 +17,6 @@ jobs:
postgresql+psycopg2://superset:superset@127.0.0.1:15432/superset
PYTHONPATH: ${{ github.workspace }}
REDIS_PORT: 16379
CI: github-actions
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
services:
postgres:
Expand All @@ -40,42 +39,19 @@ jobs:
python-version: '3.6'

- name: Install dependencies
uses: apache-superset/cached-dependencies@ddf7d7f
uses: apache-superset/cached-dependencies@adc6f73
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This upgrade fixes a bug related to cache validation.

with:
# Run commands in parallel does help initial installation without cache
parallel: true
run: |
npm-install && npm-build
npm-install && build-instrumented-assets
pip-install && setup-postgres && testdata
cypress-install

- name: Cypress run all
- name: Run Cypress
uses: apache-superset/cached-dependencies@adc6f73
env:
CYPRESS_GROUP: Default
CYPRESS_PATH: 'cypress/integration/*/*'
run: |
# Start Flask and run Cypress

# --no-debugger means disable the interactive debugger on the 500 page
# so errors can print to stderr.
flask run --no-debugger --with-threads -p 8081 &

sleep 3 # wait for the Flask app to start

cd ${{ github.workspace }}/superset-frontend/cypress-base/
npm run cypress -- run --browser ${{ matrix.browser }} --spec "${{ env.CYPRESS_PATH }}" --record false

- name: Cypress run SQL Lab (with backend persist)
env:
SUPERSET_CONFIG: tests.superset_test_config_sqllab_backend_persist
CYPRESS_GROUP: Backend persist
CYPRESS_PATH: 'cypress/integration/sqllab/*'
run: |
# Start Flask with alternative config and run Cypress

killall python # exit the running Flask app
flask run --no-debugger --with-threads -p 8081 &
sleep 3 # wait for the Flask app to start

cd ${{ github.workspace }}/superset-frontend/cypress-base/
npm run cypress -- run --browser ${{ matrix.browser }} --spec "${{ env.CYPRESS_PATH }}" --record false
CYPRESS_BROWSER: ${{ matrix.browser }}
CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
with:
run: cypress-run-all
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to bashlib.sh.

6 changes: 2 additions & 4 deletions .github/workflows/superset-frontend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@ jobs:
frontend-build:
name: build
runs-on: ubuntu-18.04
env:
CI: github-actions
steps:
- name: Checkout code
uses: actions/checkout@v2
- name: Install dependencies
uses: apache-superset/cached-dependencies@ddf7d7f
uses: apache-superset/cached-dependencies@adc6f73
with:
run: npm-install
- name: eslint
Expand All @@ -26,4 +24,4 @@ jobs:
- name: Upload code coverage
working-directory: ./superset-frontend
run: |
bash <(curl -s https://codecov.io/bash) -cF unittest,javascript
bash <(curl -s https://codecov.io/bash) -cF javascript
15 changes: 7 additions & 8 deletions .github/workflows/superset-python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ jobs:
python-version: [3.6]
env:
PYTHON_LINT_TARGET: setup.py superset tests
CI: github-actions
steps:
- name: Checkout code
uses: actions/checkout@v2
Expand All @@ -19,7 +18,7 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
uses: apache-superset/cached-dependencies@ddf7d7f
uses: apache-superset/cached-dependencies@adc6f73
- name: black
run: black --check $(echo $PYTHON_LINT_TARGET)
- name: mypy
Expand Down Expand Up @@ -62,7 +61,7 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
uses: apache-superset/cached-dependencies@ddf7d7f
uses: apache-superset/cached-dependencies@adc6f73
with:
run: |
pip-install
Expand All @@ -75,7 +74,7 @@ jobs:
./scripts/python_tests.sh
- name: Upload code coverage
run: |
bash <(curl -s https://codecov.io/bash) -cF unittest,python,postgres
bash <(curl -s https://codecov.io/bash) -cF python

test-mysql:
runs-on: ubuntu-18.04
Expand Down Expand Up @@ -105,7 +104,7 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
uses: apache-superset/cached-dependencies@ddf7d7f
uses: apache-superset/cached-dependencies@adc6f73
with:
run: |
pip-install
Expand All @@ -118,7 +117,7 @@ jobs:
./scripts/python_tests.sh
- name: Upload code coverage
run: |
bash <(curl -s https://codecov.io/bash) -cF unittest,python,mysql
bash <(curl -s https://codecov.io/bash) -cF python

test-sqlite:
runs-on: ubuntu-18.04
Expand All @@ -141,7 +140,7 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
uses: apache-superset/cached-dependencies@ddf7d7f
uses: apache-superset/cached-dependencies@adc6f73
with:
run: |
pip-install
Expand All @@ -154,4 +153,4 @@ jobs:
./scripts/python_tests.sh
- name: Upload code coverage
run: |
bash <(curl -s https://codecov.io/bash) -cF unittest,python,sqlite
bash <(curl -s https://codecov.io/bash) -cF python
7 changes: 7 additions & 0 deletions superset-frontend/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ module.exports = {
},
plugins: ['prettier', 'react'],
overrides: [
{
files: ['cypress-base/**/*'],
rules: {
'import/no-unresolved': 0,
'global-require': 0,
}
},
{
files: ['*.ts', '*.tsx'],
parser: '@typescript-eslint/parser',
Expand Down
4 changes: 4 additions & 0 deletions superset-frontend/babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,9 @@ module.exports = {
],
plugins: ['babel-plugin-dynamic-import-node'],
},
// build instrumented code for testing code coverage with Cypress
instrumented: {
plugins: ['istanbul'],
},
},
};
Loading