CI - Fix smoketests running twice

[bfops]

Description of Changes

Smoketests were running twice because tests/foo.rs automatically gets turned into a test binary, but we also had a mod.rs that listed each file, so they were getting tested via that as well.

I fixed this by removing everything from mod.rs except for the cli subdirectory (because tests in subdirs are not automatically discovered).

API and ABI breaking changes

None.

Expected complexity level and risk

1

Testing

• CI passes
• If I run cargo ci smoketests locally, I still see the CLI tests running
• If I run cargo ci smoketests locally, I do not see a particular CLI test name appearing more than once (e.g. cli_cannot_publish_breaking_change_without_flag)

[kim]

I would very much prefer to do the opposite (keep mod.rs and move test files into a subdirectory). The reason is that a) it is faster (linker runs only once) and b) it provides slightly nicer test names to filter on. See also #4102 (comment)

[bfops]

I would very much prefer to do the opposite (keep mod.rs and move test files into a subdirectory). The reason is that a) it is faster (linker runs only once) and b) it provides slightly nicer test names to filter on. See also #4102 (comment)

@kim I've made the changes, but I'm not convinced.

In terms of linker time, how much time do you think this is saving?

What's the extra power we get in terms of filtering? I haven't experienced trouble running subsets of tests.

[kim]

Well, why do you prefer the many test binaries variant?

In terms of linker time, how much time do you think this is saving?

It's about 5s on my laptop that has 16 cores. The difference is most likely more pronounced the less cores there are -- the linker does more or less the same for each test binary, but it has to do it for each test binary.

What's the extra power we get in terms of filtering? I haven't experienced trouble running subsets of tests.

If you do cargo test -p spacetimedb-smoketests -- --list or cargo nextest list -p spacetimedb-smoketests, you'll see that the mod.rs variant lists test cases that are in each module, e.g.:

    permissions::test_call
    permissions::test_cannot_delete_others_database
    permissions::test_describe
    permissions::test_lifecycle_reducers_cant_be_called
    permissions::test_logs
    permissions::test_private_table
    permissions::test_publish
    permissions::test_replace_names

but the many-files variant lists only the name of the binary.

I find it quite valuable to be able to run both a group of tests (e.g. permissions::) or one or more sub-tests (e.g. permissions::test_publish). The python version also had tests that had the same name, but were in different modules, which yields two ways one might want to run them.

[bfops]

Well, why do you prefer the many test binaries variant?

In terms of linker time, how much time do you think this is saving?

It's about 5s on my laptop that has 16 cores. The difference is most likely more pronounced the less cores there are -- the linker does more or less the same for each test binary, but it has to do it for each test binary.

I can imagine this is part of what slows down the CI. Sad.

What's the extra power we get in terms of filtering? I haven't experienced trouble running subsets of tests.

If you do cargo test -p spacetimedb-smoketests -- --list or cargo nextest list -p spacetimedb-smoketests, you'll see that the mod.rs variant lists test cases that are in each module, e.g.:

    permissions::test_call
    permissions::test_cannot_delete_others_database
    permissions::test_describe
    permissions::test_lifecycle_reducers_cant_be_called
    permissions::test_logs
    permissions::test_private_table
    permissions::test_publish
    permissions::test_replace_names

but the many-files variant lists only the name of the binary.

I find it quite valuable to be able to run both a group of tests (e.g. permissions::) or one or more sub-tests (e.g. permissions::test_publish). The python version also had tests that had the same name, but were in different modules, which yields two ways one might want to run them.

That's not the behavior I see with cargo nextest:

cargo nextest list -p spacetimedb-smoketests | head
    Finished `test` profile [optimized + debuginfo] target(s) in 0.16s
spacetimedb-smoketests modules::tests::test_module_name_derivation
spacetimedb-smoketests tests::test_normalize_whitespace
spacetimedb-smoketests::add_remove_index test_add_then_remove_index
spacetimedb-smoketests::auto_inc test_autoinc_basic
spacetimedb-smoketests::auto_inc test_autoinc_unique
spacetimedb-smoketests::auto_migration test_add_table_auto_migration
spacetimedb-smoketests::auto_migration test_add_table_columns
spacetimedb-smoketests::auto_migration test_reject_schema_changes
spacetimedb-smoketests::call test_call_empty_errors
spacetimedb-smoketests::call test_call_errors

and to run them I would do:

cargo nextest run -p spacetimedb-smoketests --test auto_migration

or

cargo ci smoketests -- --test auto_migration

My main resistance is that we then need to list all the test files again in a mod.rs which seems like unneeded friction for adding new tests, and a way to forget them. I can add a CI check to ensure that all files are listed in mod.rs, it's just.. extra dancing.

The link time is compelling either way, though. Sigh.