Repository navigation
Split the grab-bag test files and add test_group.cpp #45
Copy link
Copy link
Closed
Labels
Description
Problem
test_server_roles.cpp holds seven suites, test_transport.cpp mixes pure URL parsing with write-invariant tests, test_server_players.cpp opens with a 670-line harness, several suites sit in files for other modules, and group.cpp has no dedicated test file.
Evidence
Checked against the code at e69f3b46a801:
- tests/test_server_roles.cpp (1485 lines) holds exactly 7 suites: ServerApproval, ServerController, ServerMetadata, ServerDisconnect, ServerGroups, ServerRolesInProcess and ServerTrace. The 9 ServerGroups tests span about 604-1140.
- tests/test_transport.cpp has TransportUrl.ParsesWebSocketUrls at line 454, a pure-function test, inside a file of loopback transport tests. The write-invariant tests (WritesOnlyWhenRoomWasReported... at 1323 through ~1717) are in the same file.
- tests/test_server_players.cpp has its first TEST at line 683, so lines 1-670 are harness (Signal, feed(), anchor checks).
- SchedRunView at test_sched_admission.cpp:280 tests src/sched/run_view.h, and there is no separate run-view test file. ColorSchedule at test_colors.cpp:187 tests scheduled_updates.h in a colors file. SourceFeed at test_source_decoder.cpp:57 tests source_feed.h in the decoder file. GroupStalls at test_audio_workers.cpp:536 tests Group in the audio-workers file.
- There is no tests/test_group.cpp, and src/group.cpp is 1153 lines.
- std::uniform_int/real_distribution is used at test_colors.cpp:90, 91, 136 and 137, test_inbound_ring.cpp:194-195 and test_source_clock.cpp:65 and 157. test_sched_property.cpp:98 and test_group_audio.cpp:~983 do avoid it, with comments explaining the mapping is not fixed by the standard.
Checked and not part of this issue
- The portability point is hygiene, not a demonstrated defect. The affected tests are robust to the mapping: test_colors.cpp's seeded tests assert guarantees plus loose controls (adjusted and kept both over 1000 out of 20000 draws) and test_inbound_ring.cpp compares against a model FIFO. The test_source_clock.cpp lock bound comes from the model clock ('not from the loop'), per the comment at its expect_lock. None of these would fail under libc++'s mapping.
- docs/conventions.md has no rule requiring one test file per source file, a dedicated test_group.cpp, or portable distributions. Line 80 only says 'seeded where randomized, as test_sched_property.cpp'. The cleanup is therefore a preference, not a convention violation.
- group.cpp is not entirely untested: Group is reached through test_audio_workers.cpp (Group::PipelineJob, GroupStalls) and test_server_core.cpp, and GroupAudio is covered by test_group_audio.cpp. There is just no file named for Group.
Direction
Split along suite lines, move misplaced suites, and start test_group.cpp.
Done when
- Each module's tests live in its file; the test count is unchanged.
Reactions are currently unavailable
Activity
kahrendt commentedon Oct 15, 2026
Claimed c25b729e; branch issue-45-split-tests
kahrendt commentedon Oct 15, 2026
Draft PR: #93