Summary
commands::client::tests::build_client_with_valid_token (and its two siblings build_client_with_empty_token, build_client_different_urls) fail intermittently under cargo test --workspace:
---- commands::client::tests::build_client_with_valid_token stdout ----
thread '...' panicked at src/commands/client.rs:153:9:
assertion failed: client.is_ok()
Seen on the Test job of CI run 34418230597 (PR #176). It is not caused by that PR — reproduced on unmodified main (393be2d).
Cause
build_client calls transport::apply_custom_ca, which reads the process-global AK_CA_CERT env var. The transport::tests set that variable — including to non-existent paths such as /some/ca.pem (ca_cert_path_from_env, apply_custom_ca_propagates_load_errors, load_missing_file_is_clear_error) — under test_utils::ENV_LOCK. The three client::tests do not take ENV_LOCK, so when libtest schedules them on another thread during that window, load_ca_certificates fails on the bogus path and build_client returns Err.
cargo test runs the whole binary's tests as threads in one process, so the race is live there. cargo nextest gives each test its own process, which is why it never fails there.
Evidence
Unmodified main @ 393be2d, running only the two modules in one process, 60 times:
$ for i in $(seq 1 60); do target/debug/deps/ak-<hash> transport::tests client::tests; done
failures: 6 / 60
Deterministic version of the same failure — the ambient env var alone is enough:
$ AK_CA_CERT=/some/ca.pem target/debug/deps/ak-<hash> build_client_with_valid_token
test result: FAILED. 0 passed; 1 failed
Suggested fix
Have the three client::tests take crate::test_utils::ENV_LOCK and clear AK_CA_CERT for their duration, the way transport::tests already do. (Longer term, threading the CA path through as a parameter rather than reading the env var inside build_client would remove the shared state entirely.)
Summary
commands::client::tests::build_client_with_valid_token(and its two siblingsbuild_client_with_empty_token,build_client_different_urls) fail intermittently undercargo test --workspace:Seen on the
Testjob of CI run 34418230597 (PR #176). It is not caused by that PR — reproduced on unmodifiedmain(393be2d).Cause
build_clientcallstransport::apply_custom_ca, which reads the process-globalAK_CA_CERTenv var. Thetransport::testsset that variable — including to non-existent paths such as/some/ca.pem(ca_cert_path_from_env,apply_custom_ca_propagates_load_errors,load_missing_file_is_clear_error) — undertest_utils::ENV_LOCK. The threeclient::testsdo not takeENV_LOCK, so when libtest schedules them on another thread during that window,load_ca_certificatesfails on the bogus path andbuild_clientreturnsErr.cargo testruns the whole binary's tests as threads in one process, so the race is live there.cargo nextestgives each test its own process, which is why it never fails there.Evidence
Unmodified
main@ 393be2d, running only the two modules in one process, 60 times:Deterministic version of the same failure — the ambient env var alone is enough:
Suggested fix
Have the three
client::teststakecrate::test_utils::ENV_LOCKand clearAK_CA_CERTfor their duration, the waytransport::testsalready do. (Longer term, threading the CA path through as a parameter rather than reading the env var insidebuild_clientwould remove the shared state entirely.)