Integration testing mechanics. #36
Conversation
Apparently selenium and chromedriver are already installed on the GH runners. So just need "dash[testing]" as a dev dependency and everything should just work™️. https://community.plotly.com/t/dash-integration-testing-with-selenium-and-github-actions/43602
Codecov Report
@@ Coverage Diff @@
## main #36 +/- ##
==========================================
+ Coverage 9.38% 40.39% +31.00%
==========================================
Files 4 9 +5
Lines 373 458 +85
==========================================
+ Hits 35 185 +150
+ Misses 338 273 -65
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
The Windows build fails because Either set |
|
@sfmig let me open this for review now in the spirit of short PRs and finding our team's zen 🧘♂️ (This PR does almost nothing).
One boring train ride later: 0fac8d4 does a (still minimal but) meaningful test of the |
samcunliffe
added
task
A first integration test that starts a WAZP app server in a thread, and checks that it can be started error-free.
Merge remote-tracking branch 'origin/main' into end2end-test-attempt-v0
samcunliffe
force-pushed
the
end2end-test-attempt-v0
branch
from
e07aa27
to
5c2a8e8
Compare
| @@ -14,6 +14,8 @@ dynamic = ["version"] | |||
| license = {text = "BSD-3-Clause"} | |||
|
|
|||
| dependencies = [ | |||
| "tables", | |||
| "blosc2", | |||
There was a problem hiding this comment.
☝️ After merging main into this branch in 4ff5406 (then I fixed a trivial error), it seems the smoke test actually flagged up a missing dependency (👍).
It's an error about missing optional pytables. So this also fixes that.
I think this makes sense as we've added some HDF5 input since I made this branch.
There was a problem hiding this comment.
The problem about pytables is that pip install doesn't work on many machines (Adam mentioned this might be due to missing hdf5 library, which pip cannot install).
That's why I have recommended creating a conda environment with pytables in install instructions (conda install works).
Does "blosc2" solve the above issue? If yes, we can simply get rid of the whole conda thing
There was a problem hiding this comment.
No. blocsc2 is some compression for hdf5. It won't solve any (py)tables installation woes.
Trouble is, for the tests we'll need tables (it's fine for the tests because hdf5 is on the runners).
Options that come to mind:
- Leave it like this. (I think I prefer this option because it's "pytables' fault" if it won't install.)
- Perhaps we add a "you might have errors from pytables, here's where to go for help"-type sentence to the docs.
- Move
tablesto a dev dependency (which it really isn't, so I don't like this). But could then add an explicitcondainstallation step of tables to the instructions. - @dstansby showed me this which might help.
There was a problem hiding this comment.
That's annoying.
Tagging @adamltyson because we have discussed this before, and it's also relevant to other NIU projects.
Question: if we add tables as a regular (pip) dependency, will pip install wazp fail when users run it? Or will it just install wazp without pytables and let them know that the latter is missing?
There was a problem hiding this comment.
Sort of dodging the question, but if tables is going to be necessary for proper functioning of the app, then I would target conda-forge as the main release mechanism.
There was a problem hiding this comment.
My (univited but I've been tagged so sort of invited) two cents here is it would be best specify pandas[hdf5] if you're only loading HDF5 via pandas, and not explicitly have tables as a dependency. This way if pandas change the library they're using to load HDF5 files you don't have to change anything.
re. installing a hdf5 library:
- For testing, I've used
tox-condabefore and would recommend. - For users I agree that targeting
conda-forgeas the main release channel is a good way to go. This way you can specify a hdf5 dependency in the conda-forge recipe.
And finally, FWIW, if you try and install tables on a platform that doesn't have wheels and tries to build from source, and don't have a HDF5 library you get:
Collecting tables
Using cached tables-3.8.0.tar.gz (8.0 MB)
Installing build dependencies ... done
Getting requirements to build wheel ... error
error: subprocess-exited-with-error
× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> [11 lines of output]
cpuinfo failed, assuming no CPU features: 'flags'
* Using Python 3.11.0 | packaged by conda-forge | (main, Jan 14 2023, 12:26:40) [Clang 14.0.6 ]
* Found cython 0.29.33
* USE_PKGCONFIG: True
* Found conda env: ``/Users/dstansby/mambaforge/envs/pytables``
ld: library not found for -lhdf5
clang: error: linker command failed with exit code 1 (use -v to see invocation)
.. ERROR:: Could not find a local HDF5 installation.
You may need to explicitly state where your local HDF5 headers and
library can be found by setting the ``HDF5_DIR`` environment
variable or by using the ``--hdf5`` command-line option.
[end of output]
note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error
× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> See above for output.
note: This error originates from a subprocess, and is likely not a problem with pip.Looking at the PyPI files for tables this just looks like it's because there's no wheel for macOS arm, so here I'd say asking for one of those upstream in the tables repo might be a good thing to do.
There was a problem hiding this comment.
it would be best specify pandas[hdf5] if you're only loading HDF5 via pandas
[...] This way if pandas change the library they're using to load HDF5 files you don't have to change anything.
That would be nice but I don't think we can have nice things...
(wazp-env) ➤ grep dependencies\ = pyproject.toml
dependencies = ["pandas[hdf5]", "dash", "dash-bootstrap-components"]
(wazp-env) ➤ tox -e py310 -vv
eventually you get a:
WARNING: pandas 1.5.3 does not provide the extra 'hdf5'
About the pyTables wheel, seems like they did make it: PyTables/PyTables#887 and PyTables/PyTables#944.
There was a problem hiding this comment.
I have a silicon Mac, so I created a fresh conda environment, ran pip install . on this branch, and got the error that David posted. I tried the same on an ubuntu machine and it worked (presumably because there is a system-level install of hdf5 ?).
Anyhow, at least some users will get the error when running pip install wazp.
So how about this action plan:
- We release on pip as is, but users have to follow the current installation instructions:
(alternatively we could create a conda
conda create -n wazp-env -c conda-forge python=3 pytables conda activate wazp-env pip install wazp
environment.ymlfile) - In parallel we start the process of releasing on
conda-forge - Re testing, Sam decides how to handle it, because I don't understand it well enough to offer an opinion
There was a problem hiding this comment.
The test job should be as-close-as-possible environment to the users. So if we install tables via conda-force, then the GH workflow ideally does that.
I'd suggest merging this PR as-is. I claim that installing via conda as per the instructions, and then reinstalling as a dependency via pip works. Even though it's double-work.
We can fix pyproject.toml accordingly later if we fix the GH workflow or release some other way or...
|
Codecov |
Both coverage.py and @sfmig seem to need it.
As discussed in #27, we also want "end-to-end" tests/integrations tests that actually start the web app and ... do things.
I'm not sure about the doing things part yet, but I can get the mechanics of dash's basic example to work.
"dash[testing]"as a dev dependency and everything will just work™️.But doesn't work on Windows (because of course not... ).Fixed windows, see below.To run locally:
python -m pip install "dash[testing]"toxorpytestas you normally would.