Skip to content

refactor: split engine into a library target (src/lib.rs)#29

Merged
zoosky merged 2 commits into
mainfrom
feature/lib-binary-separation
Jun 2, 2026
Merged

refactor: split engine into a library target (src/lib.rs)#29
zoosky merged 2 commits into
mainfrom
feature/lib-binary-separation

Conversation

@zoosky

@zoosky zoosky commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

driller was a binary-only crate (no src/lib.rs), which blocked integration tests, benchmarks, and sibling tooling from using the engine directly -- they could only shell out to the compiled binary.

This PR introduces a clean engine-in-library / CLI-in-binary split:

  • src/lib.rs is the new crate root. It re-exports the engine modules (actions, benchmark, checker, config, expandable, reader, tags) and exposes a single typed entry point:
    pub fn run(options: &RunOptions) -> BenchmarkResult
    interpolator and writer stay crate-private (they appear in no public interface).
  • src/main.rs keeps everything CLI: clap structs, version strings, argv -> RunOptions mapping, the stats/compare presentation, and process::exit codes. It now drives the engine via driller::run.
  • Cargo.toml declares explicit [lib] and [[bin]] targets (both named driller).

This is a cleaner shape than making main() public: consumers get run(options) -> result, not "run the whole CLI," and clap/version/exit concerns never leak into the library.

Why

Unblocks library-level integration tests, in-process parse/throughput benchmarks (a benches/ harness can now use driller::reader), and sibling crates depending on driller via path.

Behaviour-preserving

CLI output and exit codes are unchanged; only module reachability changed (no member-level visibility was altered).

  • cargo build / cargo clippy --all-targets -- -D warnings -- clean
  • cargo fmt --check -- clean
  • cargo test -- 95 lib + 25 bin unit tests, cli_errors, serde_yaml_moat (32), and the new lib doctest all pass
  • cargo doc --no-deps -- builds
  • Smoke: driller --version and an ad-hoc run (connection error -> clean 520, exit 0) verified

Notes

Implements a deliberately cleaner design than upstream fcsonline/drill#180 (which makes main() public and leaves CLI concerns in the library). The die() -> Result cleanup (so the library never calls process::exit) is intentionally out of scope and left as a follow-up.

driller was a binary-only crate, which blocked integration tests,
benchmarks, and sibling tools from using the engine directly -- they
could only shell out to the binary.

Introduce a clean engine-in-library / CLI-in-binary boundary:

- src/lib.rs is the new crate root. It re-exports the engine modules
  (actions, benchmark, checker, config, expandable, reader, tags) and
  exposes a typed entry point, `run(&RunOptions) -> BenchmarkResult`.
  interpolator and writer stay crate-private (no public interface).
- src/main.rs keeps the CLI: clap structs, version strings, argv ->
  RunOptions mapping, the stats/compare presentation, and process exit
  codes. It now drives the engine via `driller::run`.
- Cargo.toml declares explicit [lib] and [[bin]] targets.

Behaviour-preserving: identical CLI output and exit codes; fmt, clippy,
and the full test suite (lib + bin unit tests, integration tests, the
new lib doctest) pass unchanged. Member-level visibility is untouched --
only module-level reachability changed.
@zoosky

zoosky commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

Code review

Found 4 issues:

  1. The new crate doc comment names the harness/ crate, which exists only in the private workbench (driller-workbench), not in this public repo -- so it will surface internal structure in cargo doc. CLAUDE.md's "Cross-project commit safety" section requires the public submodule carry "no internal context"; suggest "(tests, benchmarks, and sibling crates)".

driller/src/lib.rs

Lines 24 to 29 in d6475fd

//!
//! The engine modules are exposed at module level for in-repo consumers
//! (tests, benchmarks, the `harness/` crate); their members keep their original
//! visibility. `interpolator` and `writer` stay crate-private as they appear in
//! no public interface.

  1. BenchmarkResult becomes public API for the first time via this pub use (defined at src/benchmark.rs#L49), but has no struct-level doc comment and its reports/duration fields are undocumented. CLAUDE.md Rule 12 says "Every module, struct, enum, trait, and public function must have doc comments".

driller/src/lib.rs

Lines 40 to 42 in d6475fd

pub use benchmark::{BenchmarkResult, RunOptions};

  1. run() (and pub fn compare) can terminate the caller's process: benchmark::execute calls std::process::exit(1) on an empty plan (src/benchmark.rs#L141) and checker::compare does the same on a bad baseline (src/checker.rs#L30). The die() -> Result cleanup is deferred (noted in the PR), but Rule 12 requires public functions to "Document error conditions and edge cases" -- the run() doc should at least warn callers these paths exit.

driller/src/lib.rs

Lines 51 to 54 in d6475fd

/// so it is safe to call from a synchronous context.
pub fn run(options: &RunOptions) -> BenchmarkResult {
benchmark::execute(options)
}

  1. The crate doc calls run "the single typed entry point", but pub mod benchmark also exposes pub fn execute with an identical signature, so driller::benchmark::execute is a second public path. Either soften to "primary"/"recommended" or make execute pub(crate).

driller/src/lib.rs

Lines 11 to 14 in d6475fd

//!
//! [`run`] is the single typed entry point: build a [`RunOptions`] and call it.
//!
//! ```no_run

All four are documentation/visibility refinements; the structural split itself is behaviour-preserving (verified clean across history, comments, and a shallow bug scan).

Follow-up to the code review on the library-target split:

- Drop the reference to the private-workbench-only `harness/` crate from
  the crate-level doc comment (it does not exist in this repo and would
  surface in cargo doc); use a generic "sibling crates" phrasing.
- Document BenchmarkResult and its reports/duration fields, now that it
  is part of the public API.
- Narrow benchmark::execute to pub(crate) so driller::run is genuinely
  the single public entry point for executing a run.
- Document on run() and checker::compare() that some fatal-input paths
  terminate the process via process::exit rather than returning, so
  library callers can pre-validate. The die() -> Result migration
  remains a tracked follow-up.
@zoosky

zoosky commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

Addressed all four in e052ad3:

  1. Dropped the harness/ mention from the crate doc (generic "sibling crates").
  2. Documented BenchmarkResult + its reports/duration fields.
  3. Narrowed benchmark::execute to pub(crate), so driller::run is genuinely the single public entry point.
  4. Documented on run() and checker::compare() that fatal-input paths process::exit rather than return; the die() -> Result migration stays a tracked follow-up.

fmt + clippy clean; all tests (95 lib + 25 bin + cli_errors + 32 moat + doctest) pass; cargo doc builds.

@zoosky zoosky merged commit 351a863 into main Jun 2, 2026
14 checks passed
@zoosky zoosky deleted the feature/lib-binary-separation branch June 2, 2026 15:28
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