Skip to content

Wait for a sent log batch to be handed over before asserting it - #324

Closed
arifBurakDemiray wants to merge 1 commit into
stagingfrom
log-batch-test-race
Closed

arifBurakDemiray wants to merge 1 commit into
stagingfrom
log-batch-test-race

Conversation

@arifBurakDemiray

Copy link
Copy Markdown
Member

What

SdkLogGatheringTests.trackingOffHold_keepsTheNewest500Lines_andDAccountsForEveryDroppedLine failed on #323's ubuntu run at line 201: lines were still held after the test waited for every batch to be sent.

The send callback in DefaultNetworking removes a sent log batch, then calls Log.onLogBatchSent(), which lets the logger queue the next one. awaitSentBatches waited for the stored batches to reach zero, so it could end between those two steps and see no batch on disk while the logger still held lines.

awaitSentBatches now also waits for the networking task that sent the batch (its callback runs inside it), then for the log delivery. DefaultNetworking.tasks is package-private for that; nothing else changes in the SDK.

Verified

  • With a 200 ms pause added locally between the removal and onLogBatchSent(), the old wait fails this test (expected:<0> but was:<475>) and armedDirective_uploadsTheInitLinesOfItsLevels_withTheGatherIdInBatchesOfAtMostB; the new wait passes all 13 tests of the class.
  • Same class of race as Wait for the lines logged after a request's removal before asserting them #322: something the SDK does after it removes a request.

🤖 Generated with Claude Code

The networking removes a sent log batch and only then lets the logger
queue the next one, so the tests' wait for sent batches could end in
between and find lines still held ("expected:<0> but was:<475>" on CI).
It now also waits for the networking task that sent the batch, for which
DefaultNetworking.tasks is package-private.
@github-actions

Copy link
Copy Markdown

Dependency security scan

Scanned 25 resolved dependencies, failing on HIGH and above.

No known vulnerabilities in 25 resolved dependencies.

Result: passed

Full run log

@github-actions

Copy link
Copy Markdown

Unit Test Results 🚀

739 tests  ±0   739 ✅ +1   2m 50s ⏱️ ±0s
 64 suites ±0     0 💤 ±0 
 64 files   ±0     0 ❌  - 1 

Results for commit 3873bf2. ± Comparison against base commit c0b090d.

@github-actions

Copy link
Copy Markdown

Code coverage

module lines branches
sdk-java 89.8% (7233/8054) 80.0% (3201/4000)
sdk-java-ui 70.5% (968/1374) 63.2% (499/790)

Files changed by this pull request

file lines covered
sdk-java/src/main/java/ly/count/sdk/java/internal/DefaultNetworking.java 83.8% (150/179)
Least covered classes (10 shown)
class module lines covered uncovered
ly.count.sdk.java.ui.JavaFxContentDisplay sdk-java-ui 10.6% (23/216) 193
ly.count.sdk.java.ui.FxSurfaces sdk-java-ui 74.8% (172/230) 58
ly.count.sdk.java.ui.ExternalBrowser sdk-java-ui 55.0% (66/120) 54
ly.count.sdk.java.internal.SDKCore sdk-java 89.0% (363/408) 45
ly.count.sdk.java.ui.JavaFxWidgetHost sdk-java-ui 81.2% (190/234) 44
ly.count.sdk.java.internal.ModuleRequests sdk-java 75.9% (132/174) 42
ly.count.sdk.java.internal.ModuleContent sdk-java 89.7% (280/312) 32
ly.count.sdk.java.internal.TimedEvents sdk-java 60.8% (48/79) 31
ly.count.sdk.java.internal.SessionImpl sdk-java 90.1% (274/304) 30
ly.count.sdk.java.internal.SDKStorage sdk-java 84.5% (158/187) 29

Full run log

@arifBurakDemiray

Copy link
Copy Markdown
Member Author

Moved into #323 (commit 0c9f587).

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.

1 participant