Extend autoconf tests #2975
Extend autoconf tests #2975
Conversation
vmcj
force-pushed
the
configure_test_build_gha_subjobs
branch
4 times, most recently
from
223665f to
8a60b1b
Compare
vmcj
force-pushed
the
configure_test_build_gha_subjobs
branch
from
8a60b1b to
a6668ba
Compare
|
The PR should be fine without the Not sure why https://github.com/DOMjudge/domjudge/actions/runs/14092975984/job/39473975481#step:4:1010 fails, @eldering shouldn't it find the target because of the Makefile.global? Other files also don't have an explicit dependency for distclean to depend on clean so what is it complaining about? |
It uses wwwrun (or www group), it's added as last so it shouldn't fail our normal installs.
vmcj
force-pushed
the
configure_test_build_gha_subjobs
branch
from
9e26204 to
9538de9
Compare
sphinx-build adds it's own Makefile which we need to work around or we can decide to just remove all dirs. This would also remove custom files someone might have put there.
It's always run before every testcase, which makes that we would always install certain tools which breaks the tests where we assume to not have those tools.
Bats testing now works from another directory
We'll need to start with multiple clean images in future tests so we now skip certain tests to keep a "clean" image for certain tests and test in other matrix jobs. The intent was to both extend the tests and test with different distrobutions: - Alpine fails on running bats, - Fedora has issues with building `runpipe` so fails on `make judgehost` - Arch needs another package manager to install certain libraries (so is left for another PR). - OpenSUSE lacks some libraries. The pre-work is already done to start using those images. make clean should remove what was created by make all make distclean should remove what was created by configure (plus what make clean does)
vmcj
force-pushed
the
configure_test_build_gha_subjobs
branch
from
bbc03f5 to
5f873f8
Compare
| # Debian/Ubuntu need 2 packages, fedora some more. As we always install those together we just | ||
| # add them as replacement for and skip the other. |
There was a problem hiding this comment.
This sentence is not written correctly:
| # Debian/Ubuntu need 2 packages, fedora some more. As we always install those together we just | |
| # add them as replacement for and skip the other. | |
| # Debian/Ubuntu need 2 packages, fedora some more. As we always install those together we just | |
| # add them as replacement and skip the second. |
| # This test is ran with multiple containers, as we only care for the C/C++ test | ||
| # a bogus value is provided for the webserver as for example arch doesn't have | ||
| # a default www-data like group. |
There was a problem hiding this comment.
| # This test is ran with multiple containers, as we only care for the C/C++ test | |
| # a bogus value is provided for the webserver as for example arch doesn't have | |
| # a default www-data like group. | |
| # This test is ran with multiple containers. Since we only care for the C/C++ test, | |
| # a bogus value is provided for the webserver group, as for example arch doesn't | |
| # have a default www-data like group. |
| } | ||
|
|
||
| multiple_compilers() { | ||
| # all of the tests ran before will have a subset of thos already |
There was a problem hiding this comment.
Typo in thos, but more importantly, it's not really cleat to me what that refers to.
| } | ||
|
|
||
| run_configure () { | ||
| su $u -c "./configure $*" | ||
| } | ||
|
|
||
| compiler_assertions () { |
There was a problem hiding this comment.
It would help to rename this function to clearly indicate what it does (and for example include a verb in the function name). Same with multiple_compilers. Probably something starting with assert_...
|
|
||
| run_configure () { | ||
| su $u -c "./configure $*" | ||
| echo "$args" | ||
| } | ||
|
|
||
| repo-install () { |
There was a problem hiding this comment.
Maybe clearer to rename this to install-package or so, same with repo-remove below.
| @@ -203,258 +433,334 @@ compile_assertions_finished () { | |||
| assert_line " * webserver group.....: nginx" | |||
| } | |||
|
|
|||
| @test "Run as root discouraged" { | |||
There was a problem hiding this comment.
I think various test descriptions could be written a bit more clearly/explicitly. For example here:
| @test "Run as root discouraged" { | |
| @test "A warning is shown when you implicitly configure with domjudge user=root" { |
So say what is tested for.
| assert_success | ||
| # Cleanup for next runs | ||
| rm -rf /opt/domjudge &>/dev/zero | ||
| make clean &>/dev/zero |
There was a problem hiding this comment.
Can this be a test setup/teardown action? Adding it here manually makes this code fragile.
| @@ -0,0 +1,303 @@ | |||
| #!/usr/bin/env bats | |||
There was a problem hiding this comment.
It looks like this file shares a lot of duplicate code with all.bats. Can common functionality be extracted?
This tries to split tests where extra packages get installed to different runs.
It does add some small improvements to the configure script to make testing easier.