Skip to content

test: fix the flaky RtpListenerTest and JieliTest (#257) - #259

Merged
compot-dev merged 2 commits into
v2from
fix/flaky-tests-257
Oct 31, 2026
Merged

compot-dev merged 2 commits into
v2from
fix/flaky-tests-257

Conversation

@compot-dev

Copy link
Copy Markdown
Collaborator

Closes #257. Tests only: no app change, no version bump.

RtpListenerTest.silenceIsReportedWithoutRebinding

Recorder.statuses / sdps / pts were Collections.synchronizedList, but the test reads them with Collections.frequency() and for-each while the session thread adds to them; iteration is not guarded by a synchronized list, hence the ConcurrentModificationException at line 288 on CI. They are CopyOnWriteArrayList now. The other tests that use synchronized lists only call contains/get/copy (guarded), so they are left as they are.

JieliTest.noVideoListensElsewhereAndAsksAgain

The test set diagnosticMs = 500. The client closes a diagnostic port at the first receive timeout (1 s) after that, so the port lived about a second. The test waits for the no-video state (20 ms polls), sleeps 100 ms and only then sends its packet; a pause longer than that second on a loaded runner lost the packet and failed assertNotNull(d) at line 825.

Reproduced locally by putting a 1.2 s pause before the send: the same failure at line 825 (0.7 s did not fail). With diagnosticMs = 3000 and up to 6 s to wait for the port to be freed, the 1.2 s pause passes. The probe is not part of the change.

Checked

  • Docker build as in CI: testDebugUnitTest, lintDebug.
  • Both tests 20 times in a row on 2 CPUs with 2 busy loops (a loaded CI-like runner): 20/20 passed (before the fix 35/35 also passed there, so the probe above is what shows the Jieli race).

RtpListenerTest read the status and payload-type lists with
Collections.frequency() and for-each while the session thread added to
them; a synchronizedList does not guard iteration, hence the
ConcurrentModificationException. The recorder now uses CopyOnWriteArrayList.

JieliTest kept the diagnostic port for diagnosticMs = 500, so the port
lived about a second (until the next receive timeout); a pause of more
than that on a loaded runner before the test sent its packet lost it and
failed assertNotNull(d) at line 825 (reproduced with a 1.2 s pause). The
port now lives 3 s and the test waits up to 6 s for it to be freed.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Stabilize RTP listener and Jieli diagnostic tests

🧪 Tests 🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

AI Description

• Prevent concurrent iteration failures in RTP listener test recordings.
• Extend the Jieli diagnostic test window so delayed packets can arrive and port release can be
 verified.
• Change tests only; application behavior is unchanged.
Diagram

graph TD
  Suite["Unit tests"] --> RTP["RTP listener test"] --> Recorder["Copy-on-write recorder"] --> Iteration["Safe iteration"]
  Suite --> Jieli["Jieli test"] --> Port["Diagnostic port"] --> Release["Release check"]
Loading
High-Level Assessment

Keep the targeted test-only changes. Explicitly locking every recorder iteration would be more intrusive, while coordinating the Jieli packet send with socket readiness would require additional test instrumentation. Neither is warranted for these focused fixes.

Files changed (2) +10 / -5

Tests (2) +10 / -5
JieliTest.javaAllow more time for diagnostic packet delivery and port release +4/-2

Allow more time for diagnostic packet delivery and port release

• Extends the diagnostic port lifetime from 500 ms to 3 seconds so a delayed test packet can arrive. Increases the port-release wait from 3 to 6 seconds to accommodate that lifetime.

android/app/src/test/java/com/openipc/decoder/JieliTest.java

RtpListenerTest.javaMake callback recordings safe to iterate during writes +6/-3

Make callback recordings safe to iterate during writes

• Replaces synchronized lists with copy-on-write lists for recorded statuses, SDP values, and payload types. This prevents concurrent modification failures when assertions iterate while the session thread appends.

android/app/src/test/java/com/openipc/decoder/RtpListenerTest.java

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. The test can miss late diagnostic data ✓ Resolved
Description
The wait in noVideoListensElsewhereAndAsksAgain still uses a three-second deadline and calls
stats() in the loop condition, while the assertions take a later snapshot. On a loaded runner,
diagnostic_ports can become available after the loop exits, and the separate snapshot can disagree
with the condition before packet assertions run.
Code

android/app/src/test/java/com/openipc/decoder/JieliTest.java[804]

+        Jieli.diagnosticMs = 3000;
Evidence
The PR increases diagnosticMs to 3000, but the subsequent polling loop still ends after three
seconds, checks stats() inside the condition, and obtains a separate stats snapshot afterward.
This fails the rule's requirement that the wait cover diagnostic data with additional headroom and
that the loop and assertions use a consistent snapshot.

Make JieliTest wait for the asserted diagnostic data
android/app/src/test/java/com/openipc/decoder/JieliTest.java[802-804]
android/app/src/test/java/com/openipc/decoder/JieliTest.java[816-826]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
The test extends the diagnostic port lifetime but still waits only three seconds and evaluates `diagnostic_ports` using a different stats snapshot than the assertions. Delayed diagnostic data can therefore cause the test to fail on a loaded runner or produce inconsistent assertions.
Fix Focus Areas
- android/app/src/test/java/com/openipc/decoder/JieliTest.java[816-826]
Recommended Fix
Extend the wait deadline beyond three seconds so it covers both the second `OPEN_RT_STREAM` request and `diagnostic_ports` data. Capture one stats snapshot per loop iteration for the condition, retain the final snapshot, and use that same snapshot for the corresponding diagnostic assertions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@compot-dev

Copy link
Copy Markdown
Collaborator Author

/review

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit fd1e71b

@compot-dev
compot-dev merged commit 96bde7b into v2 Oct 31, 2026
7 checks passed
@compot-dev
compot-dev deleted the fix/flaky-tests-257 branch October 31, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky unit tests on CI: RtpListenerTest and JieliTest fail now and then

1 participant