Skip to content

Refactor smoketest publish and subscribe helpers to builder pattern#5283

Open
clockwork-labs-bot wants to merge 14 commits into
masterfrom
bot/smoketest-builder-options
Open

Refactor smoketest publish and subscribe helpers to builder pattern#5283
clockwork-labs-bot wants to merge 14 commits into
masterfrom
bot/smoketest-builder-options

Conversation

@clockwork-labs-bot

@clockwork-labs-bot clockwork-labs-bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Description of Changes

Refactors the Rust smoketest helper surface so publish and subscribe variants use builders instead of parallel helper methods.

  • Replaces publish_module* helper variants with Smoketest::publish() and fluent options for name, clear, current database, break clients, stdin, replicas, organization, and source modules.
  • Replaces subscribe_* / subscribe_background* variants with Smoketest::subscribe(...).expect_rows(...).confirmed(...).background().
  • Updates smoketest call sites to the builder APIs.

API and ABI breaking changes

Internal smoketest helper API change only. No product API or ABI changes.

Expected complexity level and risk

2

This touches many smoketest call sites, but the underlying CLI command behavior remains centralized in the same helper internals.

Testing

  • Existing CI passes

Comment thread crates/smoketests/src/lib.rs Outdated
Comment thread crates/smoketests/src/lib.rs Outdated
bfops
bfops previously approved these changes Jun 11, 2026

@bfops bfops left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM if it's passing CI. still some inline questions though.

@bfops bfops dismissed their stale review June 11, 2026 18:41

wrong PR….

Comment thread crates/smoketests/src/lib.rs
Comment thread crates/smoketests/src/lib.rs
@clockwork-labs-bot clockwork-labs-bot changed the title WIP refactor smoketest publish helpers Refactor smoketest publish and subscribe helpers Jun 11, 2026
@clockwork-labs-bot clockwork-labs-bot marked this pull request as ready for review June 11, 2026 19:18
@bfops bfops changed the title Refactor smoketest publish and subscribe helpers Refactor smoketest publish and subscribe helpers to builder pattern Jun 11, 2026
Comment thread crates/smoketests/src/lib.rs Outdated
Comment thread crates/smoketests/src/lib.rs
Comment thread crates/smoketests/src/lib.rs
Comment thread crates/smoketests/src/lib.rs Outdated
Comment thread crates/smoketests/src/lib.rs
@clockwork-labs-bot

Copy link
Copy Markdown
Contributor Author

Pushed b83b803ec for the latest review:

  • restored publish_module_internal and have PublishBuilder call it
  • pass publish options as direct params, without the extra PublishCommand struct
  • inlined the one-call source publish dispatcher

I left PublishBuilder / SubscribeBuilder hand-written rather than adding derive_builder: using derive_builder here would either generate a second builder type (PublishBuilderBuilder) or require reintroducing a separate options struct, and these builders have a little stateful behavior (current_database, stdin disabling --yes, source dispatch) that is clearer as explicit methods.

Comment thread crates/smoketests/src/lib.rs Outdated
Comment thread crates/smoketests/src/lib.rs Outdated
Comment thread crates/smoketests/src/lib.rs
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.

2 participants