perf(functests): speedup — profrc pre-baking, pty-close shutdown, parallel port pools #103
Reference in New Issue
Block a user
No description provided.
Delete Branch "feat/functest-speedup"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Description:
Functional test speedup: ~7.6s → ~0.8s per test on local run (93 tests in 78s wall-clock across 4 parallel groups).
Changes
Pre-baked profrc — replace 16 interactive UI setup commands (
/inpblock,/notify,/wrap,/roster,/occupants,/time) with a pre-written profrc containing[ui]and[notifications]sections. Saves ~1800ms/test.pty-close shutdown — replace
sleep(1)+ blockingwaitpid()with:close(pty fd)→ SIGHUP → pollingwaitpid(WNOHANG)→ SIGTERM/SIGKILL fallback. Saves ~4900ms/test.Remove stabber sleeps —
usleep(100ms)afterstbbr_start()andstbbr_stop()are unnecessary:bind()+listen()complete synchronously before return;pthread_join()guarantees socket release. Saves ~200ms/test.Parallel port pools —
PORTS_PER_GROUP=50isolated port ranges (group 1: 5230–5279, group 2: 5280–5329, etc.) for safe parallel execution without port conflicts.Dependencies
Requires stabber with accept-loop hang fix (devs/stabber@5bb776c) — without it
stbbr_stop()can hang when sleeps are removed.ee306c18f9toce94d89250Generally good, but a few fixes later it's going to be much better
@@ -248,4 +249,1 @@/* Calculate port base: each build uses a different range of TEST_GROUPS ports.* Build 0 (local/default): 5230-5233, Full: 5230-5233, Minimal: 5234-5237, etc.* Build 0 and Full share the same range because build 0 is for local runs or sequential run (no parallel builds),It was a useful explanation, I do not see a good reason to remove it.
Restored
@@ -315,38 +317,42 @@ init_prof_test(void **state)_create_chatlogs_dir();_create_logs_dir();/* Pre-write profrc with all UI settings that were previously set viaAgain comment reflects the change rather then adding the context to the code. It would make more sense to write why we setup this specific value prior to firing the tests (e.g., to ensure consistent output)
Corrected
@@ -362,1 +369,4 @@fd = 0;/* Poll for exit — typically completes in <100ms */int exited = 0;for (int i = 0; i < 100; i++) { /* 100 × 50ms = 5s max */100 is a magic number; it should be defined as a constant where explanation could be provided
Corrected
Not fixed actually (
usleep(100000);)@@ -363,0 +374,4 @@exited = 1;break;}usleep(50000); /* 50ms */here and later magic numbers
Corrected
@@ -378,3 +401,1 @@* See: https://git.jabber.space/devs/stabber/issues/3*/usleep(100000); // 100ms/* No sleep needed: stbbr_stop() calls pthread_join() which blocks untilThe comment again reflects the change, though it also provides the context. However, I am not sure if it needs to stay. This context could be moved to the git commit message, which could later be checked via blame by anyone who plans to change this part.
Corrected
ce94d89250tod98f270cb6d98f270cb6toec586105b4ec586105b4to24f7141c98LGTM
24f7141c98tof84ed1bf6a@@ -493,3 +542,3 @@prof_input("password");expect_timeout = 60;expect_timeout = EXPECT_TIMEOUT_CONNECT;it feels like it could substituted with
prof_timeout_resetto deduplicate code; it's out of scope though.