Skip to content

Split the grab-bag test files and add test_group.cpp #45

Closed

Description

@kahrendt

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.

Activity

  1. self-assigned this
    on Oct 15, 2026
  2. kahrendt commented on Oct 15, 2026

    @kahrendt
    ContributorAuthor

    Claimed c25b729e; branch issue-45-split-tests

  3. kahrendt commented on Oct 15, 2026

    @kahrendt
    ContributorAuthor

    Draft PR: #93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions