Repository navigation
Improve test setup - #4569
Improve test setup#4569andrew8er wants to merge 7 commits into
Conversation
6baa8d6 to
25c8765
Compare
8828cb3 to
a8bc29f
Compare
|
I'm a bit unsure if I know how the testing should run 😅 My setup is with VSCode and a devcontainer. The testing functionality of VSCode doesn't work correctly for me, none of the tests aren passing so something does not work for that yet. I did get |
|
The test script is currently only meant to run "as is", i.e. directly from a local shell, not within a devcontainer. I have not looked into this yet, but I guess, in a devcontainer setup, your IDE's shell is running in a container itself. That means running Docker is theoretically possible ("docker in docker") but a lot of features, such as volume mounts are not present. I will see, how I can integrate this, but it should actually be not very different from the state before. The previous shell script started "docker compose" as well, just with different settings. |
|
@jarlhengstmengel I improved the argument forwarding of |
|
It's building the container, but I'm encountering the following error when running |
|
This is probably a UID mapping issue with Docker and bind mounts on Linux. These steps may help:
|
LLM PR Review (verdigado-code)Analyzed diff, but not the whole repository. Important context might be missing. ScopeThe PR matches its description: Docker Compose test setup, Correctness
The
Conventions
Minor
|
|
Ok, I have a different error now 😅 When I look into the logs for integreat-cms-test-web-1: |
|
Ok, I finally got around to testing this. I also run Apple Silicon, but the performance difference is not as dramatic for me (M3 Pro). Still an improvement, though 👍
I needed to add
I also encountered this when I tried to compare the times to a linux system. It's a permission issue with the shared volume. Temporary fix is to change the permissions of The permanent fix should be in the Dockerfile. I'll follow up with a more thorough review in the coming days |
Thank you @jonbulz, that helped! I can run |
|
@jonbulz @jarlhengstmengel Please give it another run and report any problems. |
Short description
Run tests in a Docker Compose environment, on dev machines as well as CI. The proposed changes result in a speedup for local development (a major speedup on ARM machines anyway) and comparable runtime on CI, but with an 8-fold reduction in parallelism (no parallelism used actually), using the same
resource_class: large. This translates into a substantial reduction in spent credits per run.We could continue to use parallelism on CI, but the speedup would not scale linearly while complicating the test setup, and there are a few optimizations still on the table that we should try first. I therefore decided to not use parallelism for now.
Proposed changes
The current test setup tries to make the tests' Docker environment match the CircleCi environment. This is misguided IMHO. The dev and test environments (including the CI environment) should instead strive to match the production environment.
Pinning the Docker images' architectures on the other hand takes a huge performance toll when there is a mismatch, and in fact, running the tests on Arm MacBooks is currently practically impossible (see table below). Also, the actual benefit of pinning is dubious. The chances of encountering a significant behavioral difference between architectures is pretty small anyway, and if we do encounter one, it will be immediately obvious with the proposed setup.
Main changes
Move pytest settings to
pytest.toml. I really think it is not a good idea to overload thepyproject.tomlwith all kinds of settings as it becomes unwieldy. It also has the side effect that the Docker images get rebuilt on every settings change, even when the settings do not affect the build at all.pyproject.tomlshould only contain project metadata, dependencies and build settings. Anything that you might want to ship to pypi and that affects the package build process basically.Refactor the
test.shshell script to a Python script (test.py), which is more portable and hopefully easier to understand. The script offers some predefined presets – sets of pytest arguments – that will be passed to the pytest runner. The currently available presets are:changed,ci,coverageanddefault.It also has a
--localoption for running tests locally (please test this). All other arguments will be passed to pytest as is.The
webpack-stats.jsonfile was moved tointegreat_cms/static/shared. I think it is the more appropriate place, and it needs to be in a separate directory for reasons explained in the next section.Improve the Docker Compose setup. A few special settings worth mentioning:
test_entrypoint.shis now included intest.py, since theDockerfile.devimage is supposed to be universally usable in the future. Also, this makes the setup less complicated.docker-compose.test.yamlat the project root and use official images for our 3rd-party services as well as a special custom image built fromDockerfile.dev, that inherits fromdebian:trixieto match our production setup.Dockerfile.devcontains all Python as well as NPM dependencies compiled in and will be rebuilt when any of those changes. In the case of a change, rebuilds should still be relatively fast, since there are cache mounts in place forapt,uvandnpmduring the image building process.webservice that compiles the web assets via webpack.cmsandwebservices share a shared directory in/app/integreat_cms/static/sharedthat contains the webpack stats file. This is a tmpfs mount, so the local files stay unaffected.cmsservice/app/integreat_cms/pdfdirectory also gets a tmpfs mount, so it is always clean and local files are unaffected.dbservice gets a tmpfs mount on/var/lib/postgresql, so Postgres effectively becomes an in-memory DB. This gives a major speedup.Minor changes
qltysh/qlty-orb.Side effects
There should be no side effects.
Faithfulness to issue description and design
There are no intended deviations from the issue and design.
How to test
Note
Usage with Podman works now, but the default machine VM size created by Podman is too small to run tests in parallel. Run the following commands to resize your machine:
podman machine stop podman machine set --memory 8192 podman machine startThen prepend
CONTAINER_ENGINE=podmanto the commands below.tools/test.py.tools/test.py --preset ci.tools/test.py --preset changed.tools/test.py --preset coverage.tools/test.py --local(if possible).tools/test.pywith any arguments you usually need.🟢 I would really like to know, if you experience any speed-ups or downs with your setup!