Skip to content

Commit c5bc86b

Browse files
committed
test: make e2e tests hermetic and load-robust under parallel CTest
Three tests flaked or could collide under `ctest -j`: - toolset_e2e_test: (1) used a FIXED shared temp root ($TMPDIR/agentty_toolset_e2e) while setting a global HOME and spawning real subprocesses — every other test in the suite keys its sandbox on getpid(); made this one match. (2) The process_poll checks raced fixed wait_ms windows (500 ms for the first poll) against subprocess scheduling latency; on a loaded box the child hadn't been scheduled to emit its first line yet, cascading three failures. Replaced with a poll-until-marker loop on a generous deadline so the assertions test semantics (initial output, later output once, no replay), not timing. - mcp_bridge_test / acp_integration_test: same fixed-shared-path anti-pattern (agentty_mcp_e2e / agentty_acp_it) — made both PID-unique and remove_all the root up front. Full suite now 100% green across repeated -j12 runs (was intermittently 1-2 failures).
1 parent 35da7ee commit c5bc86b

3 files changed

Lines changed: 41 additions & 6 deletions

File tree

tests/acp_integration_test.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ int main() {
4848
namespace fs = std::filesystem;
4949
// The write tool's sandbox refuses paths outside the workspace root.
5050
// Point the root at a tmp dir we own so the scripted `write` succeeds.
51-
const fs::path tmp = fs::temp_directory_path() / "agentty_acp_it";
51+
// PID-unique so the suite stays hermetic under parallel CTest (-j).
52+
const fs::path tmp = fs::temp_directory_path() /
53+
("agentty_acp_it_" + std::to_string(::getpid()));
54+
fs::remove_all(tmp);
5255
fs::create_directories(tmp);
5356
// Sandbox persistence too: AgentServer::persist() writes every turn
5457
// to persistence::threads_dir() = $HOME/.agentty/threads. Without

tests/mcp_bridge_test.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include <cstdio>
1414
#include <cstdlib>
15+
#include <unistd.h> // getpid
1516
#include <filesystem>
1617
#include <fstream>
1718
#include <string>
@@ -56,8 +57,10 @@ int main() {
5657

5758
// Write a temp mcp.json pointing at the example server, and aim the bridge
5859
// at it via AGENTTY_MCP_CONFIG.
59-
auto tmp = fs::temp_directory_path() / "agentty_mcp_e2e";
60-
std::error_code ec; fs::create_directories(tmp, ec);
60+
// PID-unique so the suite stays hermetic under parallel CTest (-j).
61+
auto tmp = fs::temp_directory_path() /
62+
("agentty_mcp_e2e_" + std::to_string(::getpid()));
63+
std::error_code ec; fs::remove_all(tmp, ec); fs::create_directories(tmp, ec);
6164
auto cfg = tmp / "mcp.json";
6265
{
6366
std::ofstream f(cfg);

tests/toolset_e2e_test.cpp

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <chrono>
1919
#include <cstdio>
2020
#include <cstdlib>
21+
#include <unistd.h> // getpid
2122
#include <filesystem>
2223
#include <fstream>
2324
#include <string>
@@ -83,7 +84,13 @@ int main() {
8384
// ── Sandbox: everything under one temp root, BEFORE first registry()
8485
// touch (the registry is a process-lifetime static; workspace root and
8586
// HOME must be final before it is built).
86-
auto root = fs::temp_directory_path() / "agentty_toolset_e2e";
87+
// PID-unique sandbox: CTest runs the suite with -j, and this test spawns
88+
// real subprocesses (bash/process_start) under a global HOME. A FIXED
89+
// shared path let a sibling test's teardown race our tree — the source of
90+
// the intermittent toolset_e2e failure under -j12. Match the rest of the
91+
// suite and key the root on getpid().
92+
auto root = fs::temp_directory_path() /
93+
("agentty_toolset_e2e_" + std::to_string(::getpid()));
8794
std::error_code ec;
8895
fs::remove_all(root, ec);
8996
fs::create_directories(root / "src");
@@ -265,9 +272,31 @@ int main() {
265272
const auto marker = started->text.find("proc-");
266273
const auto end = started->text.find(' ', marker);
267274
const std::string id = started->text.substr(marker, end - marker);
268-
auto first = run("process_poll", {{"id", id}, {"wait_ms", 500}});
275+
276+
// Poll until a marker shows rather than racing one fixed window:
277+
// under a loaded CI box (this test co-schedules with the whole
278+
// suite) the child can take far longer than any single wait_ms to
279+
// be scheduled and have its pipe drained. Loop with a generous
280+
// overall deadline so the assertion tests SEMANTICS (initial
281+
// output arrives, later output arrives once, no replay) instead
282+
// of subprocess scheduling latency.
283+
auto poll_until = [&](const char* needle, int budget_ms) {
284+
auto last = run("process_poll", {{"id", id}, {"wait_ms", 500}});
285+
int waited = 500;
286+
while (!has(last, needle) && waited < budget_ms) {
287+
auto r = run("process_poll", {{"id", id}, {"wait_ms", 500}});
288+
waited += 500;
289+
// A poll only returns output produced since the previous
290+
// poll; keep the newest non-empty result so the hit isn't
291+
// lost when a later poll comes back empty.
292+
if (has(r, needle) || r) last = std::move(r);
293+
}
294+
return last;
295+
};
296+
297+
auto first = poll_until("process-first", 10000);
269298
check(has(first, "process-first"), "process_poll: returns initial output");
270-
auto second = run("process_poll", {{"id", id}, {"wait_ms", 2000}});
299+
auto second = poll_until("process-second", 10000);
271300
check(has(second, "process-second"), "process_poll: waits for new output");
272301
check(second.has_value() && !has(second, "process-first"),
273302
"process_poll: does not repeat previously delivered output");

0 commit comments

Comments
 (0)