compiletest: ./x test <filter> is a substring match

[jieyouxu]

This is very non-intuitive...

I tried on native x86_64-pc-windows-msvc:

./x test tests/run-make/exp/ --stage 1

This showed me 8 tests were ignored (incl. a local tests/run-make/exp/ test I added for testing) even though I would've expected an exact match for only the local test I added:

[23:37] Joe:rust (return-adjustment-target) | ls -d tests/run-make/*exp*
tests/run-make/export-executable-symbols/             tests/run-make/share-generics-export-again/
tests/run-make/extern-fn-explicit-align/              tests/run-make/wasm-export-all-symbols/
tests/run-make/mingw-export-call-convention/          tests/run-make/wasm-symbols-not-exported/
tests/run-make/rustdoc-scrape-examples-invalid-expr/

I almost never want this substring match behavior, at least not unless I explicitly write some kind of --test-name="exp", for example.

I suspect this is some behavior related to the test name constructed by compiletest, which is fed to libtest which probably handles the test filtering... Ah, it might be the test name constructed from the test path...

[jieyouxu]

I have not investigated if this is true for run-make tests only, or if this is a generic compiletetst behavior.

[jieyouxu]

This behavior is indeed applicable generally, it seems like. E.g. necessary and sufficient condition to reproduce:

tests/
    ui/
        unknown/ # <- just that dir must exist
        unknown-1.rs
        unknown-2.rs
        meow-unknown-bar.rs

A folder must exists that shares the same "prefix" name, e.g. unknown/, and some other tests must contain unknown. Writing ./x test tests/ui/unknown/ will run all of these tests.

There's a bootstrap check that asserts the filter must correspond to an existing directory or file, but that's actually kind of misleading...

[jieyouxu]

Note that the combined effect of compiletest handling + underlying libtest means that ./x test <test_folder_path> -- -- --exact does what I expect ./x test <test_folder_path> to do, but I find this very confusing...

[Shunpoco]

I investigated this problem and got some findings:

1. This substring match causes not only run unnecessary tests in the same directory, but also run unnecessary tests in completely different directory:
   Let's say, when I kick ./x ./tests/ui/unknown/sample.rs

tests/
    ui/
        unknown/
            sample.rs # <- This is executed as expected
        dir1/
            dir2/
                unknown/
                    sample.rs # <- This is also executed unexpectedly

2. This problem is caused because compiletest passes a filter string to libtest without test suite prefix. In the example above, compiletest passes unknown/sample.rs as a filter, so libtest picks both tests/ui/unknown/sample.rs and tests/ui/dir1/dir2/unknown/sample.rs. In your example, because the filter is only unknown, all tests which contains the substring unknown are executed.
   compiletest picks this filter string from args given by bootstrap's run() function (/bootstrap/src/core/build_steps/test.rs), and in the function the test suite prefix is stripped from the original filter string (./tests/ui/unknown/sample.rs -> unknown/sample.rs).

We can fix this problem by modifying the run() function's behaviour slightly. If it modifies the original filter just removing the leading dot . then passes it to compiletest, it should work appropriately. I think the path is used only for a filter in compiletest, the change will not affect other parts (I ran ./x test on my local, and there was no regression).

[jieyouxu]

I'm a bit wary of fixing it like that, because that doesn't account for platform path separator differences nor UNC paths, right? Also relative paths but from different directories? I'm inclined to say that the actual problem (or rather, the deeper problem) is that compiletest doesn't properly normalize out the test path name during test discovery and collection before sending it off to libtest. This is further complicated because as far as I can recall bootstrap has another layer of filtering logic before handing the filter string to compiletest.

[Shunpoco]

Thanks.

that doesn't account for platform path separator differences nor UNC paths, right?

I lacked of this point of view, I should investigate more.

that compiletest doesn't properly normalize out the test path name during test discovery and collection

Currently compiletest use a relative path to the test file as the test name, but do you mean that there is a better way than that?

[jieyouxu]

Currently compiletest use a relative path to the test file as the test name, but do you mean that there is a better way than that?

I'm not too sure. Because I think what compiletest does currently is that it uses the test path as a test name, and that test name is passed to libtest. However libtest does funny things like, using the test name verbatim, as a test filter. compiletest may need to properly handle the filtering during test discovery and not rely on libtest's filtering behavior, which is not easy to implement.

[jieyouxu]

See also discussion at https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Mismatch.20between.20arg-filter.20vs.20.60--exclude.60.20filter

[Shunpoco]

Thanks! the discussion is very interesting.
I agree with you that compiletest should have its own filtering system rather than relying on libtest's filter.