Test Dash table against head of master and release-v1 #309
Conversation
Marc-Andre-Rivet
changed the title
|
These checks do not exist anymore and have been supplanted. Will remove them from the master branch checks and add the new ones in once this PR is completed. |
| paths: | ||
| - "venv" |
There was a problem hiding this comment.
Not keeping the venv in cache to make sure the latest is installed from git ?
| name: Run tests | ||
| command: | | ||
| . venv/bin/activate | ||
| npm run test-v0 |
There was a problem hiding this comment.
test v0 stuff -- we only run server tests
| @@ -54,7 +102,7 @@ jobs: | |||
| name: Run tests | |||
| command: | | |||
| . venv/bin/activate | |||
| npm run test | |||
| npm run test-v1 | |||
There was a problem hiding this comment.
for v1 we run everything (server, standalone w/ React 16, unit tests)
|
|
||
| - restore_cache: | ||
| key: deps1-{{ .Branch }}-{{ checksum "requirements.txt" }} | ||
| echo 'export PERCY_TOKEN="$PERCY_PYTHON_TOKEN_V0"' >> $BASH_ENV |
There was a problem hiding this comment.
v0 and v1 visual tests end up in https://percy.io/plotly/dash-table-python-v0 and https://percy.io/plotly/dash-table-python-v1
| @@ -13,7 +13,8 @@ | |||
| ID_PREFIX = "app_dropdown" | |||
| IDS = { | |||
| "dropdown": ID_PREFIX, | |||
| "dropdown-by-cell": '{}-row-by-cell'.format(ID_PREFIX) | |||
| "dropdown-by-cell": '{}-row-by-cell'.format(ID_PREFIX), | |||
| "dropdown-by-cell-deprecated": '{}-deprecated-row-by-cell'.format(ID_PREFIX) | |||
There was a problem hiding this comment.
Not certain what happened but I think the recent change to renderer (keys/ids) is responsible for the regression -- both table NotifyObserver had the same key -- apparently causing the second one to disappear from the DOM entirely.
There was a problem hiding this comment.
There was a problem hiding this comment.
release-v1 did not have this problem because the latest master changes haven't been merged in yet -- process yet to be put in place :)
There was a problem hiding this comment.
Oh interesting - that's probably going to bite us and our users a lot of places. Should we make it an exception on the Python side if you ever provide multiple components with matching id?
There was a problem hiding this comment.
make it an exception on the Python side if you ever provide multiple components with matching id?
There is a duplicate ids check: plotly/dash#320
There was a problem hiding this comment.
There is a duplicate ids check: plotly/dash#320
why wasn't that triggered here?
There was a problem hiding this comment.
Layout is dynamic, it doesn't work when that is the case.
There was a problem hiding this comment.
Ah got it, didn't notice that. More reason to make a dynamic test -> plotly/dash#475 (comment)
| Werkzeug==0.14.1 | ||
| wrapt==1.10.11 | ||
| -r requirements-base.txt | ||
| -r requirements-v1.txt |
There was a problem hiding this comment.
@alexcjohnson So interestingly Heroku does not seem super flexible about its setup procedure. The project has been tagged as Python and as far as I understand, that means it will look for setup.py, runtime.txt and requirements.txt -- so whatever weird multi version setup we have, we must play around that limitation.
There was a problem hiding this comment.
Annoying, but nice solution!
Fixes https://github.com/plotly/dash-core/issues/20.
The table modification is a bit different from what will happen for the other repos as the master branch will support both the v0 and v1 of Dash.