GNOME Bugzilla – Bug 795636
gitlab-ci: generate test coverage reports
Last modified: 2018-05-02 12:58:39 UTC
Created attachment 371500 [details] [review] ci: collect test coverage and deploy a html report through gitlab pages This reports something like https://creiter.pages.gitlab.gnome.org/glib/coverage/index.html Feedback welcome. btw, I get regular test timeouts on my glib fork with this and I'm not sure if it is related to coverage data generation. meson test has a "--timeout-multiplier" option which we should use there imo.
Review of attachment 371500 [details] [review]: Overall, I think this is a great addition. I’m not a fan of all the little scripts that are starting to accumulate in the .gitlab-ci directory, but I don’t know of an alternative. Emmanuele, what do you think? ::: .gitlab-ci/run-docker.sh @@ +8,2 @@ --file "Dockerfile" . +sudo docker run --rm --security-opt label=disable \ Why disable security labelling? ::: .gitlab-ci/test-msys2.sh @@ +36,3 @@ + +cd .. +curl -O -J -L "https://github.com/linux-test-project/lcov/releases/download/v1.13/lcov-1.13.tar.gz" Can we have some kind of a crypto signature or SHA512 sum check on the downloaded tarball please? Otherwise if the LTP gets compromised, so does GLib.
(In reply to Philip Withnall from comment #1) > Review of attachment 371500 [details] [review] [review]: > > Overall, I think this is a great addition. I’m not a fan of all the little > scripts that are starting to accumulate in the .gitlab-ci directory, but I > don’t know of an alternative. > > Emmanuele, what do you think? > > ::: .gitlab-ci/run-docker.sh > @@ +8,2 @@ > --file "Dockerfile" . > +sudo docker run --rm --security-opt label=disable \ > > Why disable security labelling? oh, right, that's a bit unrelated. this makes the command work on fedora with selinux enabled, otherwise mounting of the glib sources fails there. > ::: .gitlab-ci/test-msys2.sh > @@ +36,3 @@ > + > +cd .. > +curl -O -J -L > "https://github.com/linux-test-project/lcov/releases/download/v1.13/lcov-1. > 13.tar.gz" > > Can we have some kind of a crypto signature or SHA512 sum check on the > downloaded tarball please? Otherwise if the LTP gets compromised, so does > GLib. ok
(In reply to Christoph Reiter (lazka) from comment #2) > (In reply to Philip Withnall from comment #1) > > ::: .gitlab-ci/run-docker.sh > > @@ +8,2 @@ > > --file "Dockerfile" . > > +sudo docker run --rm --security-opt label=disable \ > > > > Why disable security labelling? > > oh, right, that's a bit unrelated. this makes the command work on fedora > with selinux enabled, otherwise mounting of the glib sources fails there. Please split that out into a separate commit then. Howcome we didn’t need this before?
Created attachment 371545 [details] [review] ci: collect test coverage and deploy a html report through gitlab pages
(In reply to Christoph Reiter (lazka) from comment #2) > (In reply to Philip Withnall from comment #1) > > Can we have some kind of a crypto signature or SHA512 sum check on the > > downloaded tarball please? Otherwise if the LTP gets compromised, so does > > GLib. > > ok done (In reply to Philip Withnall from comment #3) > (In reply to Christoph Reiter (lazka) from comment #2) > > (In reply to Philip Withnall from comment #1) > > > ::: .gitlab-ci/run-docker.sh > > > @@ +8,2 @@ > > > --file "Dockerfile" . > > > +sudo docker run --rm --security-opt label=disable \ > > > > > > Why disable security labelling? > > > > oh, right, that's a bit unrelated. this makes the command work on fedora > > with selinux enabled, otherwise mounting of the glib sources fails there. > > Please split that out into a separate commit then. Howcome we didn’t need > this before? This script is only needed for debugging the docker container locally and not used by gitlab. I'm usually on debian, which is why I didn't add it initially. I've just removed it for now.
Review of attachment 371545 [details] [review]: Thanks, this looks good to me. I’d like to get Emmanuele’s opinion on it before it’s pushed, though, since he set up the Docker stuff.
Review of attachment 371545 [details] [review]: Nice :thumbsup:
Fixed a trivial merge conflict and pushed to master, thanks.
Created attachment 371610 [details] [review] ci: use timeout-multiplier=2 for running the tests It looks like the coverage generation makes the tests a bit slower and some are now hitting timeouts. Flaky tests are always more annoying than slow ones, and we don't know how much resources we get under CI, so increase the timeout.
Review of attachment 371610 [details] [review]: yes please