search

qgis / QGIS-Enhancement-Proposals

QEP's (QGIS Enhancement Proposals) are used in the process of creating and discussing new enhancements for QGIS
118 stars 37 forks source link

Simplify running tests on developers machines #244

Closed strk closed 2 years ago

strk commented 2 years ago

QGIS Enhancement: Simplify running tests on developers machines

Date 2022/01/30 Author Sandro Santilli (@strk )

Contact strk@kbt.io

maintainer @strk

Version QGIS 3.22

Summary

Developers modifying any part of QGIS codebase may introduce regressions on hard-to-anticipate code paths. The number of tests available are of great help but developers need an easy way to run them all. At this moment the only "easy" way to run tests is creating a pull-request on GitHub and let external infrastructure run the tests. This situation results in an overload of the external, shared, infrastructure which becomes slower as the number of proposed changes grow.

Some work has been done toward providing developers a standard way to run tests locally (make check target) but such standard way is currently rarely used as it often fails due to assumptions made by tests about the required test environment in which they are meant to run (often ending up to expect running in GitHub Actions)

Proposed Solution

The proposed solution is to ensure make check rule takes care of ensuring as much as the test environment setup is available and then run all the tests that can be run, while skipping (and warning the developer about that) those tests which can NOT be run for any reason. The performance of tests should be idempotent so that developers can take cycle over tests multiple times.

Given the current expectancy the broadest set of tests (all?) can probably be run by using a docker-based approach as it seems to be what the centralized shared CI is using. Doing so would make docker the only requirement on developers machines.

Backwards Compatibility

Changing make check to start docker containers would for some developers mean they will be unable to run it if they don't have docker. Given the current situation being that NOBODY reported being successful at running successfully make check locally, this seems to be a non-problem.

Votes

(required)

elpaso commented 2 years ago

My gut feeling is that this does not worth the effort (unless it is very small), as a regular QGIS developer I have never had the need to run all tests locally in an environment which is (almost) exactly the one used in CI.

The only use case I see for having this possibility is when a test passes locally but fails on CI, in that case last time it happened I think that I've been using something like this on my machine:

export GH_WORKSPACE=`pwd`
docker-compose -f .docker/docker-compose-testing.yml run qgis-deps /root/QGIS/.docker/docker-qgis-test.sh
strk commented 2 years ago

Effort of embeddding your approach into make check doesn't seem too much does it?

strk commented 2 years ago

@elpaso I tried your command and it failed:

root@docker:/usr/src/qgis# export GH_WORKSPACE=`pwd`
root@docker:/usr/src/qgis# docker-compose -f .docker/docker-compose-testing.yml run qgis-deps /root/QGIS/.docker/docker-qgis-test.sh
Starting docker_httpbin_1 ... done
Starting docker_webdav_1  ... error

ERROR: for docker_webdav_1  Cannot start service webdav: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error mounting "/.docker/webdav/nginx.conf" to rootfs at "/etc/nginx/conf.d/default.conf": mount /.docker/webdav/nginx.conf:/etc/nginx/conf.d/default.conf (via /proc/self/fd/6), flags: 0x5000: not a directory: unknown: Are you trying to mount a directory onto a file (or vice-versa)? Check if the specified host path exists and is the expected type

ERROR: for webdav  Cannot start service webdav: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error mounting "/.docker/webdav/nginx.conf" to rootfs at "/etc/nginx/conf.d/default.conf": mount /.docker/webdav/nginx.conf:/etc/nginx/conf.d/default.conf (via /proc/self/fd/6), flags: 0x5000: not a directory: unknown: Are you trying to mount a directory onto a file (or vice-versa)? Check if the specified host path exists and is the expected type
ERROR: Encountered errors while bringing up the project.

Is a specific version of Docker or docker-compose required ?

strk commented 2 years ago

I shall add: I did use the correct env variable too, but got the same error:

root@docker:/usr/src/qgis# export QGIS_WORKSPACE=/usr/src/qgis
root@docker:/usr/src/qgis# docker-compose -f .docker/docker-compose-testing.yml run qgis-deps /root/QGIS/.docker/docker-qgis-test.sh
Starting docker_httpbin_1 ... done
Starting docker_webdav_1  ... error

ERROR: for docker_webdav_1  Cannot start service webdav: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error mounting "/.docker/webdav/nginx.conf" to rootfs at "/etc/nginx/conf.d/default.conf": mount /.docker/webdav/nginx.conf:/etc/nginx/conf.d/default.conf (via /proc/self/fd/6), flags: 0x5000: not a directory: unknown: Are you trying to mount a directory onto a file (or vice-versa)? Check if the specified host path exists and is the expected type

ERROR: for webdav  Cannot start service webdav: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error mounting "/.docker/webdav/nginx.conf" to rootfs at "/etc/nginx/conf.d/default.conf": mount /.docker/webdav/nginx.conf:/etc/nginx/conf.d/default.conf (via /proc/self/fd/6), flags: 0x5000: not a directory: unknown: Are you trying to mount a directory onto a file (or vice-versa)? Check if the specified host path exists and is the expected type
ERROR: Encountered errors while bringing up the project.
elpaso commented 2 years ago

I'm sorry to hear that it isn't working anymore, I agree it would be nice to have a simple way to replicate locally what we have on CI.

strk commented 2 years ago

Upgrading docker to version 20.10.12 (both client and version) and docker-compose to version 1.29.2 (both coming by default in Ubuntu-22.04 aka Jammy Jellyfish) takes me a little further but halts here:

Pulling qgis-deps (qgis3-build-deps-binary-image:)...
ERROR: pull access denied for qgis3-build-deps-binary-image, repository does not exist or may require 'docker login': denied: requested access to the resource is denied

From .github/workflows/run-tests.yml I now see it takes a secret DOCKER_USERNAME and DOCKER_PASSWORD to fetch that image, which makes it even harder to reconstruct that environment (why a secret?)

elpaso commented 2 years ago

Upgrading docker to version 20.10.12 (both client and version) and docker-compose to version 1.29.2 (both coming by default in Ubuntu-22.04 aka Jammy Jellyfish) takes me a little further but halts here:

Pulling qgis-deps (qgis3-build-deps-binary-image:)...
ERROR: pull access denied for qgis3-build-deps-binary-image, repository does not exist or may require 'docker login': denied: requested access to the resource is denied

From .github/workflows/run-tests.yml I now see it takes a secret DOCKER_USERNAME and DOCKER_PASSWORD to fetch that image, which makes it even harder to reconstruct that environment (why a secret?)

I think the secret is only used for pushing, you are pulling an image that doesn't exist (locally where it's supposed to be), have a look to run-tests.yaml docker-build step (disclaimer: it's not my code so don't ask me the details, I'm just guessing here)

strk commented 2 years ago

I found out there's no secret involved, it's just that docker-compose.yml is EXPECTED that the user of it defined a qgis3-build-deps-binary-image tag on a local image, which is what the github-action does... One additional step to reconstruct the CI environment

strk commented 2 years ago

I filed PR https://github.com/qgis/QGIS/pull/48617 containing a shell script to run the test locally using the CI strategy (building docker and running in docker)

strk commented 2 years ago

Closing this as done as per https://github.com/qgis/QGIS/pull/48617

strk commented 2 years ago

Additional advancement of the work in https://github.com/qgis/QGIS/pull/48752 is pending a review at the time of writing