Improve unit test infrastructure and coverage#1292
Merged
Conversation
Each module now ships defaults for the config sections its classes read: jicofo-common (codec, metrics, octo, sctp, xmpp, local-region, visitors.vnode-join-latency-interval), jicofo-selector (bridge, transcription, translation), jicofo (authentication, conference, health, jibri, jibri-sip, jigasi, rest, visitors). Typesafe config merges all reference.conf resources on the classpath, so runtime behavior is unchanged, but each module is now self-contained (tests in jicofo-common no longer depend on defaults shipped in jicofo-selector).
Tests for jicofo-common classes (sources, codec, xmpp config, IQ handlers) lived in the jicofo module via the shared package, so per-module coverage reports under-counted and jicofo-common appeared untested. Move them to jicofo-common, and the colibri session tests (with TestColibri2Server) to jicofo-selector. Publish the reusable test utilities via test-jars: mocks (MockXmppConnection, MockXmppProvider, MockChatRoom), the in-place executors (previously defined inside ConferenceTest), config helpers and a shared shouldBeValidJson (previously duplicated three times) from jicofo-common; TestColibri2Server from jicofo-selector.
Move ColibriAndJingleXmppConnection and RemoteParticipant out of ConferenceTest into their own file, and extract the JitsiMeetConferenceImpl wiring (mock JicofoServices, XMPP provider, chat room, jingle session tracking) into a ConferenceHarness that new conference-level tests can reuse instead of rebuilding the mock tower by hand.
With -Pcoverage the jicofo module now also produces target/site/jacoco-aggregate/ covering all three modules with merged execution data. Tests in one module exercise classes from the others, so the per-module reports under-count coverage (jicofo-common showed 5% standalone vs 58% aggregated).
The only reader was JibriIqHandler, which used it to log a warning when no jibri detectors are configured. Pass that as a lazily-evaluated lambda instead (the detectors are created after XmppServices), and drop the singleton and the synchronization dance in Main.
The elvis branch created a lambda without invoking it, so the warning for a relay response with no matching session was silently dropped.
…anager. Handle colibri2 relay create/update/expire (with remote endpoint tracking and an actpass DTLS fingerprint) instead of throwing, and add ColibriV2SessionManagerTest covering single-bridge allocation and expiry, and multi-bridge (Octo) conferences: relay creation on both bridges, transport exchange with active/passive DTLS roles, remote endpoints on relays, and relay expiry when a bridge is removed or its last participant leaves. Add PendingExecutor, which queues "async" tasks until drained. Unlike inPlaceExecutor it does not run tasks re-entrantly, which matters here: relay responses are processed under syncRoot on a separate thread in production, and processing them in-place (inside Cascade.addNodeToMesh, before the new session is added to the sessions map) makes the transport exchange silently fail.
Use real occupant JIDs derived from a room JID, allow specifying the initial member role, add settable audio/video sender counts and a setRole helper that fires memberPresenceChanged.
Take the specific collaborators (XmppServices, BridgeSelector, the jibri detectors, AuthenticationAuthority, FocusManager) instead of the whole JicofoServices, so conference tests can inject them directly without building a nested JicofoServices mock.
valueUpdated() used Instant.now() directly, ignoring the clock passed to the constructor, which made the rate-limiting logic untestable with a fake clock.
Covers the moveEndpoint/moveEndpoints/moveFraction API: parameter validation, greedy selection from the largest conferences, per-conference filtering and fraction rounding.
Cover message parsing and application (enable/disable, whitelists, the mute-all trigger, metadata capture), plus rejection of messages from the wrong component address, invalid JSON, and unknown rooms.
Covers field validation, unknown conferences, remote mute (with the notification to the muted participant), self mute, not-allowed and error results, and the video variant. Also avoid calling toXML() when logging a rejected malformed mute IQ, since MuteIq.toXML() throws when the mute field is missing.
Covers AutoOwnerRoleManager owner election (first participant, skipping jibri/jigasi/visitors, re-election when the owner leaves) and AuthenticationRoleManager (granting ownership on join, on authentication events, and via grantOwnership). Extend MockChatRoom.addMember with jibri/jigasi flags so they are set before listeners fire.
Covers presence processing: mute state via SourceInfo and the legacy extensions, invalid SourceInfo JSON, region and stats ID, video codec list handling (vp8 fallback, legacy codecType), jigasi detection with trusted/untrusted domains, and role changes (including the unsupported visitor transition).
Covers member tracking driven by MUC presence (join, repeated presence, leave, kick via status 307), visitor counting, audio sender counts updated from SourceInfo, AV moderation state and whitelists, and room metadata application. Uses a mocked Smack MultiUserChat so occupant roles can be controlled.
Set diarize=false in ColibriV2SessionManagerTest's allocate() call (the field was added to ParticipantAllocationParameters by the per-endpoint diarize signaling merged into master), and reformat TestColibri2Server's imports per ktlint after the merge.
jitsi-ci Bot
pushed a commit
to jitsi/jitsi-pr-tests-pages
that referenced
this pull request
Jul 14, 2026
jitsi-ci Bot
pushed a commit
to jitsi/jitsi-pr-tests-pages
that referenced
this pull request
Jul 14, 2026
…ommon. The CI job runs mvn verify (not just test), which enforces SpotBugs with includeTests=true. Tests moved into jicofo-common in an earlier commit hit checks that were already suppressed in jicofo's exclude filter but missing from jicofo-common's: - BC_IMPOSSIBLE_INSTANCEOF (MemberRoleTest, a known kotlin lazy-init false positive) - DM_GC (WeakValueMapTest legitimately forces GC to test weak-reference eviction) - RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT on shouldBeValidJson's toString() call, now suppressed directly since the helper isn't a *Test.kt file the blanket test exclusion matches Also fixed a genuine gap: ChatRoomImplTest's 'left' flag was set by the leave callback but never asserted.
jitsi-ci Bot
pushed a commit
to jitsi/jitsi-pr-tests-pages
that referenced
this pull request
Jul 15, 2026
JonathanLennox
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restructures the jicofo/jicofo-selector/jicofo-common test setup for easier testing and adds coverage:
reference.confso each module ships defaults for the config it owns, and moved misplaced tests to their owning modules (per-module coverage was badly undercounted before this). Test utilities (mocks, executors, config helpers) are now published via test-jars instead of duplicated/cross-imported.mvn test -Pcoverage) since per-module reports miss cross-module test coverage.JicofoServicessingleton and narrowed theJitsiMeetConferenceImplconstructor to its actual collaborators, extendedTestColibri2Server/MockChatRoom— these were the main blockers to testing conference/colibri code in isolation.ColibriV2SessionManager(including multi-bridge/Octo relays),ChatRoomImpl,ChatRoomMemberImpl,ChatRoomRoleManager,MuteIqHandler,AvModerationHandler,RoomMetadataHandler,LoadRedistributor,RateLimitedStat,WeakValueMap.ColibriV2SessionManager,RateLimitedStatignoring its injected clock, andMuteIqHandlercrashing ontoXML()for the malformed IQs it is supposed to reject.Aggregate coverage: 50.5%/35.6% (line/branch) → 57.8%/44.5%.