Use of environment variables is confusing with tests

[gustavovalverde]

I don't know if there's a better way to filter tests or execute specific tests we want to run isolated, but the combination of variables, annotations like #[ignore] and configuration parameters can get very confusing to know which tests are actually being run.

A good example is: https://cs.github.com/ZcashFoundation/zebra?q=TEST_FAKE_ACTIVATION_HEIGHTS

I'm not sure if there's a better way to handle this, by filtering, excluding or selecting specific test using --test --skip --ignored --exclude or any other variation to exclude or include specific tests could be more "standard" as shown here: https://stackoverflow.com/questions/66755529/run-cargo-test-workspace-and-exclude-one-test

Maybe a naming convention for tests names, to filter group of tests.

[conradoplg]

The fake-activation-heights test is an exception, the test requires changing some constants and we can only do that at build time. Our build.rs reads the env variable and makes the constants change.

We could change the constants to be just variables so that we can change them at runtime. There was an issue for that but we recently closed it because it didn't seem required. But if you think it's getting in the way we can revisit that decision.

The #[ignore] tests are usually tests that take too long, or tests that are actually tasks (like rebuilding the cache). We could transform that into an executable, but again, not sure if it's worth it.

Maybe we can fix this just by better documenting everything. Let's see what the rest of the team thinks.

[gustavovalverde]

Now that you say this, I've been trying to execute the all_upgrades_and_wrong_commitments_with_fake_activation_heights test, but I'm changing the environment variable at runtime not at build time. And this might be the reason why this tests is not running with Docker.

Is this also the case with PROPTEST_CASES? Or is there any other cases with environment variables needed at build time (build args)

[conradoplg]

Now that you say this, I've been trying to execute the all_upgrades_and_wrong_commitments_with_fake_activation_heights test, but I'm changing the environment variable at runtime not at build time. And this might be the reason why this tests is not running with Docker.

Yep, that's probably it.

Is this also the case with PROPTEST_CASES? Or is there any other cases with environment variables needed at build time (build args)

That one is read at runtime. I think TEST_FAKE_ACTIVATION_HEIGHTS is the only one that is build-time.

[gustavovalverde]

If we agree to build a image specifically to run tests with TEST_FAKE_ACTIVATION_HEIGHTS, I can do that. From my side, I'd prefer to use runtime variables.

We can also use this approach (which will be merged with the full sync PR): #3597

This way we can even change the configuration file on the fly, before the service starts

[teor2345]

Testing Goals

These goals make it easier for developers to work on Zebra:

• the default set of tests covers most of the code, and passes on most machines
• there is a single consistent way of changing test behaviour
• compiling tests is fast

Moving to runtime environmental variables can help us achieve all these goals.
(But we don't have to do it straight away - the lightwalletd work is more important.)

Modifying Test Behaviour

Sometimes we want to skip tests by default, but activate them when we know we have the required setup. Runtime environmental variables can be set for the test command, and propagate to sub-commands. So they are the easiest way to change test behaviour.

(We also use them to disable tests in unusually restricted environments, like no network, or no IPv6.)

But using both #[ignore] and environmental variables can be really confusing.

So can test filters and environmental variables.

Let's use runtime environmental variables when we write new tests, or change old tests.

Modifying zebrad Behaviour

Configuration files change zebrad's behaviour. They are used by zebrad users, developers, and zebrad's acceptance tests. Most tests with configuration files should just work by default. Because that's one of our test goals.

In the acceptance tests, the configuration files are created by the test harness. So they can't be changed by the CI config, or manually by users.

So the test harnesses use runtime environmental variables to change important configuration options.

Let's avoid directly modifying configuration files in the CI config - they're much easier to change in the Rust test harness code.

Compiling Different Test Code

Each different compile-time configuration costs us a lot of CI time. So I'd like to avoid compile-time test features and compile-time environmental variables.

Let's use runtime environmental variables when we write new tests, or change old tests.

[teor2345]

I'm not sure how to avoid "run 0 tests" CI bugs like #3673.

Here are some ideas:

• avoid test name filters. Instead, filter tests by crate for quick compiles. Then run all the tests in the crate, with the extra tests activated by the environmental variables.
• check the CI output for "run 0 tests"
• log a message when a test is skipped, and check CI output for "skipped"
• wait until the Rust test harness gets better at this
• use cargo nexttest as our test runner

[mpguerra]

@gustavovalverde was this resolved?