mtest: fix building of all test deps for --suite

[maj0e]

Fix building all test targets for meson test --suite

This PR fixes a bug, where all targets are executed, when meson test --suite <suite> is used.
This bug was first observed in meson-1.7.0, but affects all version above. The last correct version was meson-1.6.1.

I created a small example project, which demonstrates the bug: https://github.com/maj0e/meson-test-suite-bug-example

Expected behaviour

When running meson test --suite <suite> only the targets, that are necessary for the executed tests should be built by meson.

Observed behaviour

Meson builds all targets, that depend on any of the tests, even when they are not in the selected test suite.

Implementation

The previous commit eb1e52a introduced a variable rebuild_only_tests to fix various edge cases in rebuild_deps.

In particular, if all tests are selected anyway, rebuild_deps prior to the introduction of rebuild_only_tests would potentially create a huge list of targets, which may overflow the ARG_MAX limit.

For that case rebuild_only_tests was introduced and is set to an empty list, which means that rebuild_deps falls back to the meson-test-prereq ninja target, that already contains the necessary targets.

This is wrong, though, when meson test is executed with the "--suite" option. In that case, the user requested a particular subset of the tests, but rebuild_only_tests will be set to an empty list anyway, which means the meson-test-prereq target is executed, which contains the targets for all tests.

Instead, rebuild_only_tests should only be set to an empty list, when the selected tests are identical to the complete list of available tests: tests == self.tests

Since after this commit we compare directly to the result of self.get_tests(), this will do the correct thing for all other options, which change or filter the list of selected tests (e.g. self.options.args, self.options.slice).

[bonzini]

Not sure why allplatformstests.py::AllPlatformTests::test_slice fails but it does seem related to the change.

[maj0e]

Not sure why allplatformstests.py::AllPlatformTests::test_slice fails but it does seem related to the change.

Thanks, for having a look at this PR.
I investigated the failing test and there is a subtle difference in the raw output for this test:

Output master:

ninja: Entering directory `/home/lbcjom1/repos/meson/tmpwbxu2_6q'
ninja: no work to do.
1/5 test_slice:test-1 OK              0.02s
2/5 test_slice:test-3 OK              0.02s
3/5 test_slice:test-5 OK              0.01s
4/5 test_slice:test-7 OK              0.01s
5/5 test_slice:test-9 OK              0.01s

Output from this branch

1/5 test_slice:test-1 OK              0.03s
2/5 test_slice:test-3 OK              0.03s
3/5 test_slice:test-5 OK              0.03s
4/5 test_slice:test-7 OK              0.03s
5/5 test_slice:test-9 OK              0.03s

The selected tests are actually still correct, but on master, there are the two additional lines from ninja.
The output is parsed with this regex for verifying the test results:

tests = sorted([ int(x) for x in re.findall(r'\n[ 0-9]+/[0-9]+ test_slice:test-([0-9]*)', output) ])

Note, that a match has to start with a newline. Since the first two lines are not there anymore, we don't get a match for the first item (test-1 in this case) and thus the test fails.

This leads to the question, why we don't get a message from ninja anymore:
On master an empty string is based to the rebuild_deps function (since the "--slice" argument is not considered there at all). This means that the meson-test-prereq target is invoked. Since there no deps, ninja correctly informs us, that there is "No work to do".

On this branch, it is correctly detected, that not all tests are selected, but only a subset. Instead of an empty list we pass the actual list of tests to rebuild_deps. Inside rebuild_deps the list of targets is filled, which ends up to be empty (because there are no targets). In line 2250, we then skip ninja entirely:

    if not targets:
        # We want to build minimal deps, but if the subset of targets have no
        # deps then ninja falls back to 'all'.
        return True

I think the latter behaviour is correct and I therefore propose to change the regex in this testcase:

tests = sorted([ int(x) for x in re.findall(r'[ 0-9]+/[0-9]+ test_slice:test-([0-9]*)', output) ])

Let me know, if you agree, then I push a new commit.

And sorry for the wall of text...

[bonzini]

I didn't review the regex but the debugging and the consequent wall of text makes me inclined to trust you that it's ok. :) go ahead

[bonzini]

Ah, please use rebase -i to place the new regex before your bug fix! Thanks!

[maj0e]

Ah, please use rebase -i to place the new regex before your bug fix! Thanks!

Done.
In the end I substituted "\n" with "^" in the regex, that's probably more robust than removing "\n" altogether.