Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Update toolchain #878

Merged
merged 32 commits into from
Mar 22, 2021
Merged

Update toolchain #878

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ab2de02
update toolchain to use webpack 5 amongst other things and remove Cyp…
Mar 18, 2021
4c1b4df
move last cypress test to selenium
Mar 19, 2021
0fc5749
clean up cypress related code
Mar 19, 2021
6a8b0a9
remove unused parameter `run`
Mar 19, 2021
2063cc7
parallelism 2
Mar 19, 2021
50b9a97
undo parallelism
Mar 19, 2021
993f653
probably safe updates + prettier formatting changes
Mar 19, 2021
bce90ed
update additional depedencies (d3-format, husky, less)
Mar 19, 2021
ef38957
d3-format v2 changes the minus character (https://github.com/d3/d3-fo…
Mar 19, 2021
2840f03
d3-format: fix additional test entry
Mar 19, 2021
0c60032
- webpack-dev-server -> webpack serve
Mar 19, 2021
c289908
webpack-dev-server (after all it's required by webpack serve)
Mar 19, 2021
1f4d705
remove standalone configuration
Mar 19, 2021
873508f
add back restore/save cache
Mar 19, 2021
75b6ae1
remove virtualenv usage
Mar 19, 2021
21910c3
save path
Mar 19, 2021
381e49d
switch out deprecated tslint for typescript/eslint
Mar 19, 2021
c8985e3
eslint fixes
Mar 19, 2021
6c086e3
add a tsconfig for linting
Mar 19, 2021
4378ec0
remove time.sleep in test
Mar 19, 2021
eef129b
try parallelism again..
Mar 20, 2021
c7d7a5d
parallelism=4
Mar 20, 2021
c6ac560
percy-finalize step
Mar 21, 2021
7bb2f85
remove last virtualenv
Mar 21, 2021
c37de12
runs
Mar 21, 2021
bdce3fb
inject
Mar 21, 2021
1f334e9
quotes?
Mar 21, 2021
721c3e8
.
Mar 21, 2021
4a64b1c
venv venv
Mar 21, 2021
1a6c639
finalize
Mar 21, 2021
449245b
slice up szng003 into multiple functions so it can be time sliced in CI
Mar 21, 2021
fa79934
sizing into files
Mar 21, 2021
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
80 changes: 6 additions & 74 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,15 @@ version: 2.1
jobs:
"server-test":
docker:
- image: circleci/python:3.7.9-node-browsers
- image: circleci/python:3.9.2-buster-node-browsers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating to use py3.9 as per https://github.com/plotly/dash-core/issues/203

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, can't use the new CircleCI cimg because CHROME_BIN is missing and expected by Karma

environment:
PERCY_PARALLEL_TOTAL: -1
- image: cypress/base:10

steps:
- checkout
- run:
name: Inject Percy Environment variables
command: |
echo 'export PERCY_TOKEN="$PERCY_TOKEN_E2E"' >> $BASH_ENV

- restore_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
- run:
Expand All @@ -24,18 +21,15 @@ jobs:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
paths:
- node_modules
alexcjohnson marked this conversation as resolved.
Show resolved Hide resolved

- run:
name: Install requirements
command: |
sudo pip install --upgrade virtualenv
python -m venv venv || virtualenv venv
python -m venv venv
. venv/bin/activate
pip install -r dev-requirements.txt --quiet
git clone --depth 1 git@github.com:plotly/dash.git dash-main
pip install -e ./dash-main[dev,testing] --quiet
cd dash-main/dash-renderer && npm ci && npm run build && pip install -e . && cd ./../..

- run:
name: Build
command: |
Expand All @@ -45,7 +39,6 @@ jobs:
python setup.py sdist
cd dist
find . -name "*.gz" | xargs pip install --no-cache-dir --ignore-installed && cd ..

- run:
name: Run tests
command: |
Expand All @@ -56,101 +49,50 @@ jobs:
command: npx percy finalize --all
when: always


"standalone-test":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests have been ported to test-server / Selenium

Copy link
Collaborator

Choose a reason for hiding this comment

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

test-server is awfully long now, nearly an hour. Can we parallelize it, shoot for <10min?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexcjohnson I've tried but for some reason one of the test consistently fails with parallelism turned on. I've got no idea why. Can try and figure it out before we consider this PR ready.

Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Mar 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can we parallelize it, shoot for <10min?

There's one set of tests that is extremely long to run with Selenium (lots of DOM checks). The minimal run time with per-file slicing is probably not lower than 15-20 minutes because of that test and its variations. https://github.com/plotly/dash-table/blob/dev/tests/selenium/test_sizing.py

Out of scope for this PR: it should be possible to rewrite that test to run the checks directly from the browser (Karma) instead and improve its speed by at least an order of magnitude.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexcjohnson Figured out what I did wrong. Also, broke down the large test with lots of permutations into many smaller tests in their own files. The last run took ~28 minutes but I expect the CI time splitting algorithm to be able to optimize this down to below 20 minutes soon enough.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Figured out what I did wrong

Excellent, which change fixed this?

I expect the CI time splitting algorithm to be able to optimize this down to below 20 minutes soon enough.

OK, that's reasonable for now, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which change fixed this?

When I updated the CI yaml to run tests in parallel, I copied it over from the DCC CI yaml which runs pytest with the --headless flag. For some reason tbmu001, specifically, doesn't pass when it's run headless. Not absolutely sure why as I didn't investigate it further but headless Chrome is known for sometimes causing different behaviors

docker:
- image: circleci/python:3.6.7-node-browsers
- image: cypress/base:10

steps:
- checkout
- restore_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
- run:
name: Install npm packages
command: npm ci
- run:
name: Cypress Install
command: |
$(npm bin)/cypress install

- save_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
paths:
- node_modules
- /home/circleci/.cache/Cypress

- run:
name: Run tests
command: |
rm -rf node_modules/cypress
npm i cypress@3.4.1
npm run test.standalone


"unit-test":
docker:
- image: circleci/python:3.7.5-node-browsers
- image: cypress/base:10

- image: circleci/python:3.9.2-buster-node-browsers
steps:
- checkout
- restore_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
- run:
name: Install npm packages
command: npm ci
- run:
name: Cypress Install
command: |
$(npm bin)/cypress install

- save_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}-{{ checksum ".circleci/config.yml" }}
paths:
- node_modules
- /home/circleci/.cache/Cypress

- run:
name: Install requirements
command: |
sudo pip install --upgrade virtualenv
python -m venv venv || virtualenv venv
python -m venv venv
. venv/bin/activate
pip install -r dev-requirements.txt --quiet
pip install --progress-bar off -e git+https://github.com/plotly/dash.git@dev#egg=dash[dev,testing]

- run:
name: Run tests
command: |
. venv/bin/activate
npm run build
npm run test.unit


"visual-test":
docker:
- image: circleci/node:10-browsers

- image: circleci/node:14-browsers
Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Mar 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Node14 is the current active LTS and some npm packages are starting to drop v10. Preventive more than anything.

steps:
- checkout

- restore_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}

- run:
name: Install package.json
command: npm ci

- save_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}
paths:
- node_modules

- run:
name: Run build:js
command: npm run private::build:js

- run:
name: Run visual tests
command: npm run test.visual
Expand All @@ -159,34 +101,25 @@ jobs:

"node":
docker:
- image: circleci/python:3.7.5-node

- image: circleci/python:3.9.2-node
steps:
- checkout

- run:
name: Create virtual env
command: python -m venv || virtualenv venv

- restore_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}

- run:
name: Install package.json
command: npm ci

- save_cache:
key: dep-{{ .Branch }}-{{ checksum "package-lock.json" }}-{{ checksum "package.json" }}
paths:
- node_modules

- run:
name: Install requirements
command: |
. venv/bin/activate
pip install -r dev-requirements.txt --quiet
pip install --progress-bar off -e git+https://github.com/plotly/dash.git@dev#egg=dash[dev,testing]

- run:
name: Run eslint
command: |
Expand All @@ -200,6 +133,5 @@ workflows:
jobs:
- "node"
- "server-test"
- "standalone-test"
- "unit-test"
- "visual-test"
13 changes: 7 additions & 6 deletions .config/webpack/base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ module.exports = (options = {}) => {
mode: mode,
output: {
path: path.resolve(__dirname, `./../../${dashLibraryName}`),
chunkFilename: '[name].js',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed from Wepback4 -> 5 -- see optimization section

filename: '[name].js',
library: dashLibraryName,
libraryTarget: 'window'
Expand All @@ -40,15 +39,18 @@ module.exports = (options = {}) => {
rules: [
{
test: /demo[\\\/]index.html?$/,
loader: 'file-loader?name=index.[ext]'
loader: 'file-loader',
options: {
name: 'index.[ext]'
}
},
{
test: /\.csv$/,
loader: 'raw-loader'
},
{
test: /\.ts(x?)$/,
include: /node_modules[\\\/](highlight[.]js)[\\\/]/,
include: /node_modules[\\\/](highlight[.]js|d3-format)[\\\/]/,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

d3-format v2 is now bundled in ES6, we need to transpile it to ES5 for our IE11 support.

use: [
{ loader: 'babel-loader', options: babel },
{ loader: 'ts-loader', options: ts },
Expand All @@ -65,7 +67,7 @@ module.exports = (options = {}) => {
},
{
test: /\.js$/,
include: /node_modules[\\\/](highlight[.]js)[\\\/]/,
include: /node_modules[\\\/](highlight[.]js|d3-format)[\\\/]/,
use: [
{ loader: 'babel-loader', options: babel }
]
Expand Down Expand Up @@ -97,7 +99,6 @@ module.exports = (options = {}) => {
},
resolve: {
alias: {
cypress: path.resolve('./tests/cypress/src'),
'dash-table': path.resolve('./src/dash-table'),
demo: path.resolve('./demo'),
core: path.resolve('./src/core'),
Expand All @@ -108,7 +109,7 @@ module.exports = (options = {}) => {
optimization: {
splitChunks: {
chunks: 'async',
name: true,
name: '[name].js',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed from Wepback4 -> 5

cacheGroups: {
async: {
chunks: 'async',
Expand Down
2 changes: 1 addition & 1 deletion .flake8
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[flake8]
ignore = C901, E203, E266, E501, E731, W503
ignore = C901, E203, E231, E266, E501, E731, W503
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flask / Black spacing and trailing ,, deactivating in Flask and respecting Black formatting instead

select = B,C,E,F,W,T4
per-file-ignores =
tests/*: E722, F811
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ Project.toml

# testing
/coverage
/tests/cypress/screenshots/**
/storybook-static/**

# misc
Expand Down
4 changes: 4 additions & 0 deletions .storybook/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@ const baseConfig = require('./../.config/webpack/base.js')({
});

module.exports = {
core: { builder: 'webpack5' },
stories: ['./../tests/visual/percy-storybook/**/*.percy.tsx'],
webpackFinal: async (config, { configType }) => {
// jerry rig everything
config.resolve.alias.core = path.resolve(__dirname, './../src/core'),
config.resolve.alias['dash-table'] = path.resolve(__dirname, './../src/dash-table')

config.module = baseConfig.module;
config.stats = {
warnings: true
};

return config;
}
Expand Down
10 changes: 0 additions & 10 deletions cypress.json
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions demo/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
</head>
<body>
<div id='root'></div>
<script src='https://unpkg.com/react@16.13.0/umd/react.production.min.js'></script>
<script src='https://unpkg.com/react-dom@16.13.0/umd/react-dom.production.min.js'></script>
<script src='https://unpkg.com/react@17.0.1/umd/react.production.min.js'></script>
<script src='https://unpkg.com/react-dom@17.0.1/umd/react-dom.production.min.js'></script>

<script src="./demo.js"></script>
</body>
Expand Down
23 changes: 23 additions & 0 deletions karma.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module.exports = config => {
config.set({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Karma replaces Cypress as the unit test runner. Configuration to run in the desired env the desired tests correctly bundled.

frameworks: ['mocha', 'webpack'],

plugins: [
'karma-webpack',
'karma-mocha',
'karma-chrome-launcher'
],
preprocessors: {
// add webpack as preprocessor
'tests/js-unit/**/*.ts': ['webpack']
},
files: [
'node_modules/babel-polyfill/dist/polyfill.js',
'node_modules/regenerator-runtime/runtime.js',
'tests/js-unit/**/*.ts' // *.tsx for React Jsx
],
reporters: ["progress"],
browsers: ["Chrome"],
webpack: require('./webpack.test.config')
});
}
Loading