remote-test-client tests mutate TEST_DEVICE_ADDR and break remote-test execution environments

[Hemanthkumarmd]

Rust version: 1.93.0

Commit: 444f1c5

#[test]
fn test_timeout() {
    let mut cmd = assert_cmd::cargo::cargo_bin_cmd!();
    cmd.env("TEST_DEVICE_CONNECT_TIMEOUT_SECONDS", "1");
    cmd.env("TEST_DEVICE_ADDR", "127.69.69.69:6969");
    ...
}

We run rustc tests under a remote execution environment where remote-test-client is also
used as infrastructure (not just a tool under test).A real remote-test-server is running,
and TEST_DEVICE_ADDR is set globally by the test harness.

Environment:

• Tests executed via src/bootstrap/bootstrap.py test
• remote-test-server running on a target (QEMU)
• remote-test-client used by the test harness itself
• TEST_DEVICE_ADDR is set globally for all tests

When this mentioned testcase ran I got the below failure :

thread 'main' panicked at src/tools/remote-test-client/src/main.rs:332:9:
client.read_exact(&mut header) failed with failed to fill whole buffer

The panic occurs because the test modifies TEST_DEVICE_ADDR to an unreachable
value, causing the harness-level remote-test-client to disconnect mid-protocol.
The test mutates global environment variables used by the remote-test protocol,
which can interfere with other tests when remote-test-client is also active.

Possible fixes:

• Skip this test when TEST_DEVICE_ADDR is already set
• Run test_timeout with a sanitized environment
• Gate the test on a cfg flag or env var

Can we make this testcase is non-interfering in environments where
remote-test-client is active as infrastructure.

Maybe we can fix this test like below ?

 if std::env::var_os("TEST_DEVICE_ADDR").is_some() {
    eprintln!("Skipping test_timeout when TEST_DEVICE_ADDR is pre-set");
    return;
}

[workingjubilee]

cc @pvdrz since you implemented 444f1c5 and the selection betwen the possible solutions here seems important for preserving the test's intent.

[workingjubilee]

@Hemanthkumarmd Though, uhm, also, your report is incorrect: TEST_DEVICE_ADDR is not globally mutated. It is only modified for a new command which is then spawned.

[Hemanthkumarmd]

@workingjubilee Thanks for the clarification. In a local test environment, you are correct: cmd.env() only sets the variable for the spawned test, and TEST_DEVICE_ADDR is not globally mutated.
However, in our setup, we run the entire Rust testsuite under a single bootstrap.py invocation in a QEMU image, where remote-test-client is active as part of the test harness. In this environment, the test_timeout test mutates TEST_DEVICE_ADDR in a spawned child process, which indirectly interferes with the already-running remote-test-client infrastructure. This causes the client to disconnect mid-protocol, resulting in the panic.
To prevent interference in environments like ours, we propose skipping this test when TEST_DEVICE_ADDR is already set. This change is non-intrusive for normal test runs and prevents failures in our integrated testing environment.

[workingjubilee]

In a local test environment, you are correct: cmd.env() only sets the variable for the spawned test, and TEST_DEVICE_ADDR is not globally mutated.

Mmm... mmm. "Globally" is confusing here because we're talking about multiple threads and processes. Let us not say it because it's unclear exactly which globe we're talking about.

A test, in libtest, is normally executed on its own thread, last I checked. It sets the variable, not for the spawned thread, but for the child process that the test spawns. Let us try to discuss this distinction carefully, because the actual test is not the spawned process, it is its own thread, and the bug here in your test suite is almost certainly due to code that muddles those two, which could be in our code somewhere.

However, in our setup, we run the entire Rust testsuite under a single bootstrap.py invocation in a QEMU image, where remote-test-client is active as part of the test harness. In this environment, the test_timeout test mutates TEST_DEVICE_ADDR in a spawned child process, which indirectly interferes with the already-running remote-test-client infrastructure

Can you be more specific about how it "indirectly interferes"?

Can you specify what target this is for?

The remote test client is listening to some pipes, but Command::output creates new pipes, doesn't it? The new child process should consequently not interfere with the pipes that remote-test-client is listening to by dumping output into its parent's pipes, right? Maybe it does, hm...

[workingjubilee]

...oh. Earth to Jubilee, networking still applies on the same machine???

@Hemanthkumarmd So uhm... there isn't any chance this all gets fixed by binding to a different IP address than 0.0.0.0 with the server, here, is there? https://github.com/openembedded/openembedded-core/blob/7109e5d85a8ea466762329f036806886f93f0faa/meta/lib/oeqa/selftest/cases/rust.py#L114

Because I just realized, that test is trying to set up a client that accesses a very specific address, there, but it is also a local loopback address, and my understanding of the 0.0.0.0 binding is that this is exposing you to all such things, and the remote-test infrastructure specifically advises to not use 0.0.0.0 despite providing it as an example case...