Skip to content

test(bindings): cover MoqBroadcastConsumer.closed in Kotlin, Swift, and Dart - #7540

Merged
moq-bot[bot] merged 4 commits into
mainfrom
quest/m2/binding-broadcast-end
Nov 2, 2026
Merged

moq-bot[bot] merged 4 commits into
mainfrom
quest/m2/binding-broadcast-end

Conversation

@kixelated

@kixelated kixelated commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Finishes quest/m2/binding-broadcast-end.md and deletes it.

The quest had two parts. Both are now done or owned by another quest:

  • MoqBroadcastConsumer.closed() shipped in moq-dev/moq#7304, along with tests in Rust (rs/moq-ffi/src/test.rs), Go (TestBroadcastConsumerClosed), and Python (test_broadcast_consumer_closed_resolves_on_close). Kotlin, Swift, and Dart already expose it through their aliases but had no test. The Plan said "Cover it in each binding's tests", so this PR adds one test to each:
    • kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt: a broadcast consumer's closed returns once the broadcast ends
    • swift/Tests/MoqTests/SmokeTests.swift: testBroadcastConsumerClosedReturnsOnClose
    • dart/moq/test/moq_test.dart: a broadcast consumer's closed() resolves once it closes. This one also checks that closed() is still pending while the broadcast is live, as the Python and Rust tests do.
  • A moq-transport run end arriving as Internal is the whole Goal of quest/m1/ffi-ietf-run-end.md. That quest came out of the same planning of moq-dev/moq.pro#3169 a day later, on 2026-10-31, and its Plan already notes that closed() shipped. It is being worked in draft moq-dev/moq#7479, which waits on a maintainer decision about the wire mapping. This quest did not need a second copy of that work.

The downstream bullet, moq.pro's Go and Kotlin samples switching to closed(), lives in moq.pro and comes with its pin bump.

Choices made without asking

  • I deleted this quest instead of rewriting it down to the error-kind bullet. The newer m1 quest (moq-dev/moq#7453) took over that outcome and sits in a higher milestone, so the newer recorded plan wins. If you would rather keep an m2 placeholder, restore the file with Required: /quest/m1/ffi-ietf-run-end.md.
  • The Kotlin and Swift tests check only that closed() returns after the broadcast ends, like the Go test. Neither checks that it stays pending while the broadcast is live, because there is no reliable way to do that under runTest or with an XCTest Task. The Rust FFI test covers that behavior.

Validation

Head ee0502a495 merges origin/main (d8288e6934) into the PR. That merge resolves a conflict in dart/moq/test/moq_test.dart: base replaced the old microsecond timestamp test with timestamps round-trip exactly at the track scale and timestamps convert to and from Durations. The closed test now sits before those two tests, so against main the Dart diff adds only that test.

All runs used quest gate nix develop -c ... on the merged head:

  • just dart check: passed. All 36 tests passed, including "a broadcast consumer's closed() resolves once it closes". An earlier run alongside just kt check passed every test but then tripped the script's build-hook rebuild guard: the two checks share target/. Run by itself, it passes.
  • just kt check: passed. a broadcast consumer's closed returns once the broadcast ends[jvm] is in TEST-dev.moq.SmokeTest.xml with 0 failures.
  • just swift check: skipped, because this Linux host has no Swift toolchain. CI covers it on macOS through platform.yml.
  • quest check --base origin/main --branch quest/m2/binding-broadcast-end: passed.

Follow-ups

None.

Review

Codex (quest review), gpt-6.1-sol, medium effort. Round 1 of a new session reviewed head ee0502a495c4d11e13546b9a1c310fd214494fb0 against origin/main (a546cac082) and reported 0 findings. It ran from a local branch, because another worktree has quest/m2/binding-broadcast-end checked out. The earlier round 1 on 4e74e9a61a also reported 0 findings. Online reviewers (OpenAI review, Pullfrog) found no issues on 4e74e9a61a. CodeRabbit hit its rate limit.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 2 commits November 1, 2026 01:50
…nd Dart

Go, Python, and the Rust FFI already test that a broadcast consumer's
closed() returns once the broadcast ends; the remaining wrappers expose
it through aliases but had no test. The moq-transport run-end error kind
is owned by quest/m1/ffi-ietf-run-end.md, so this quest is complete.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Bindings see a broadcast end test(bindings): cover MoqBroadcastConsumer.closed in Kotlin, Swift, and Dart Nov 1, 2026
@kixelated
kixelated marked this pull request as ready for review November 1, 2026 09:59
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7d8bc8ae-a9c1-405c-999c-dabab79ef581
Commits

Reviewing files that changed from the base of the PR and between d8288e6 and ee0502a.

Files selected for processing: 5
  • dart/moq/test/moq_test.dart
  • kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt
  • quest/m2/README.md
  • quest/m2/binding-broadcast-end.md
  • swift/Tests/MoqTests/SmokeTests.swift
Files with no reviewable changes: 2
  • quest/m2/README.md
  • quest/m2/binding-broadcast-end.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

Walkthrough

Tests were added in Dart, Kotlin, and Swift to check that a broadcast consumer’s closed() operation completes after the broadcast closes. The Dart test also checks that the operation remains unresolved while the broadcast is live. The m2 README entry and planning document for “Bindings see a broadcast end” were removed.

Merge Risk: Minimal · up to ee050

The changes add valid closure coverage without establishing a current merge-blocking risk.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: adding coverage for MoqBroadcastConsumer.closed in the Kotlin, Swift, and Dart bindings.
Description check Passed The description directly explains the added binding tests, the Dart pending-state check, and the related quest deletion. It is clearly related to the changeset.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches
Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 4e74e9a61a2e21623cf72087534436e68e04082e.

No actionable bugs found. The small binding-level smoke tests exercise the existing close signal without adding API or wire complexity. Removing the duplicate quest is reasonable: the remaining moq-transport error mapping is still tracked by quest/m1/ffi-ietf-run-end.md.

Verification: static review of the changes and relevant wrappers/FFI behavior only; no tests run. Kotlin and Swift cover completion after close, not deterministic pending-wait wakeup coverage; the existing Rust test covers that distinction.

(Written by OpenAI)

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Dart closed() test (dart/moq/test/moq_test.dart:390) — asserts closed() rests pending while the broadcast is live, then resolves after broadcast.close(). The inline consumer handle is safe to discard: the generated closed() calls uniffiClonePointer() and retains the Rust Arc for the async call's lifetime.
  • Kotlin closed() test (kt/.../SmokeTest.kt:254) — uses broadcast.end(), which is the Kotlin name for the Rust close, not the AutoCloseable close() (that only releases the producer handle and, with a consumer held open, would not end the broadcast). Correct choice.
  • Swift closed() test (swift/.../SmokeTests.swift:329) — awaits broadcast.close() and the Task, ARC releases the handles.
  • Quest cleanup — quest/m2/binding-broadcast-end.md deleted and its quest/m2/README.md bullet removed; the remaining goal lives in quest/m1/ffi-ietf-run-end.md. No dangling references remain.

The tests exercise MoqBroadcastConsumer.closed() (unchanged FFI, rs/moq-ffi/src/consumer.rs:318) and fail loudly if it never resolves. The Dart 10 ms wall-clock delay is deterministic here (the broadcast is live, so resolved cannot flip early) and Dart exposes no injectable clock, so it is not a flake risk.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@moq-bot

moq-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

codex / Codex land fixer note:

The landing log reports at line 1:

Conflicts with the base: dart/moq/test/moq_test.dart

The PR inserts its broadcast-consumer closed test beside the old timestamp test, while base replaces that timestamp test. Preserve the PR's new test followed by both replacement timestamp tests from base. A trial merge confirmed this is the only conflicted file.

Validation is blocked here: running swift test from swift/ exits 127 with /bin/bash: line 1: swift: command not found. The task requires successful builds and tests for every touched package before reporting a fix. The trial merge was aborted; the PR branch remains at its pinned head.

(written by codex / Codex)

@moq-bot
moq-bot Bot disabled auto-merge November 2, 2026 01:54
@moq-bot moq-bot Bot removed the quest:merge label Nov 2, 2026
@moq-bot

moq-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

codex / Codex land fixer: Dart test conflict with base; validation blocked: &#96;cd swift && swift test&#96; exits 127: &#96;/bin/bash: line 1: swift: command not found&#96;.. Recommendation: Merge base, preserve the broadcast-consumer cl·osed test alongside base's replacement timestamp tests, then build and test Dart, Kotlin, and Swift on hosts with the required toolchains.

Conflicts with the base: dart/moq/test/moq_test.dart

(written by codex / Codex)

@pullfrog

pullfrog Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

OpenRouter returned an error, so this run stopped. If it names your key, plan or balance, fix that with the provider or pick another model.

Internal Server Error: {"error":{"message":"Internal Server Error","code":500}}

Pick another model →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using deepseek-v4.1-flash (free via Pullfrog for OSS)

@kixelated
kixelated enabled auto-merge (squash) November 2, 2026 05:44
@moq-bot
moq-bot Bot merged commit 4372e25 into main Nov 2, 2026
2 checks passed
@moq-bot
moq-bot Bot deleted the quest/m2/binding-broadcast-end branch November 2, 2026 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant