[Bug] rbpf-tests attempt to run non-test generated interface move files

[brson]

After running rbpf-tests once, a second run will fail with errors like:

thread 'run_test::entry-point04-build/generated_interface_files/mv_interfaces/00959839231632418319/0x0000000000000000000000000000000000000000000000000000000000000001/bit_vector.move' panicked at 'called `Result::unwrap()` on an `Err` value: "Loading executable failed: InvalidAccountData"', language/tools/move-mv-llvm-compiler/tests/rbpf-tests.rs:320:6
test run_test::entry-point04-build/generated_interface_files/mv_interfaces/00959839231632418319/0x0000000000000000000000000000000000000000000000000000000000000001/bit_vector.move ... FAILED

This is because of an interaction between the test_harness macro and tests that compile the stdlib: compiling the standard library generates "interface files" that have a .move extension, and they are generated under subdirectories of the rbpf-tests directory.

The test_harness regex looks like:

datatest_stable::harness!(run_test, TEST_DIR, r"rbpf-tests/[a-z0-9-_/]*\.move$");

This regex allows tests to be located in subdirs, so it pickes up the generated files and fails them.

[brson]

The regex allows subdirectories since commit 5fe4195b64, which adds tests that are contained in subdirectiories.

The easiest solution at the moment is probably just to create a more complicated regex that rejects paths containing something like -build/.

[jcivlin]

Why don't we have a clean-up before running so each run will be like the first run?

[brson]

Why don't we have a clean-up before running so each run will be like the first run?

The primary difficulty is that the datatest_stable::harness macro is responsible for both discovering tests and generating the main function, so there is no opportunity to run custom code prior to the test harness.

[brson]

I'm finding that it is actually hard to write a regex that says "allow all strings except one's that contain '-build/'". This isn't something the Rust regex engine can do in an obvious way.

[brson]

We can possibly have the tests that link to the stdlib clean up after themselves to delete the .move interface files, though that feels pretty ugly to me.

[brson]

ok I have a regex that works. I'll post it shortly.

[jcivlin]

Maybe regex is good for now, but if the starting positions for different runs are not the same it will always remain a legit source of suspicion.

[brson]

Maybe regex is good for now, but if the starting positions for different runs are not the same it will always remain a legit source of suspicion.

Good point. It looks like each individual test does clean up their build directories prior to doing their individual builds, so the tests shouldn't be building into dirty build directories; but there is no single step to clean up all build directories at once.

[dmakarov]

For CI checks this shouldn't matter, because they always run in a new clone of the repository.

[jcivlin]

For CI checks this shouldn't matter, because they always run in a new clone of the repository.

For CI indeed, but before CI we do a local test run and it may come out misleadingly correct.