Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mock: improve ergonomics when an ExpectedSpan is needed #3097

Open
wants to merge 2 commits into
base: hds/mock-expected-span-check
Choose a base branch
from

Commits on Oct 3, 2024

  1. mock: unify expected span checking

    Previously there were two separate implementations which would check an
    `ExpectedSpan` against and actual span. One on the `ExpectedSpan` struct
    itself, which took a `SpanState` as used by the `MockCollector` and
    another on the `MockSubscriber` which took a `SpanRef` (from
    `tracing-subscriber`).
    
    In reality, both of these checks needed a `span::Id` and a `Metadata` to
    check, but the structure was also somewhat different, with the
    `MockSubscriber` span check giving better error output.
    
    This change combines the two checks into the one on `ExpectedSpan`,
    which is now generic over an `ActualSpan` implementation, which has been
    provided for `SpanState`, `SpanRef`, and also `span::Id` (for the case
    where no `Metadata` is available).
    
    The better error output from `MockSubscriber` has been integrated into
    that check.
    hds committed Oct 3, 2024
    Configuration menu
    Copy the full SHA
    935bbe0 View commit details
    Browse the repository at this point in the history
  2. mock: improve ergonomics when an ExpectedSpan is needed

    Many of the methods on `MockCollector` take an `ExpectedSpan`. This
    often requires significant boilerplate. For example, to expect that a
    span with a specific name enters and then exits, the following code is
    needed:
    
    ```rust
    let span = expect::span().named("span name");
    
    let (collector, handle) = collector::mock()
        .enter(span.clone())
        .exit(span)
        .run_with_handle();
    ```
    
    In order to make using `tracing-mock` more ergonomic and also more
    compact, the `MockCollector` and `MockSubscriber` methods that previous
    took an `ExpectedSpan`, are now generic over `Into<ExpectedSpan>`.
    
    There are currently 3 implementations of `From` for `ExpectedSpan` which
    allow the following shorthand uses:
    
    `T: Into<String>` - an `ExpectedSpan` will be created that expects to
    have a name specified by `T`.
    
    ```rust
    let (collector, handle) = collector::mock()
        .enter("span name")
        .exit("span name")
        .run_with_handle();
    ```
    
    `&ExpectedId` - an `ExpectedSpan` will be created that expects to have
    the expected Id. A reference is taken and cloned internally because the
    caller always needs to use an `ExpectedId` in at least 2 calls to the
    mock collector/subscriber.
    
    ```rust
    let id = expect::id();
    
    let (collector, handle) = collector::mock()
        .new_span(&id)
        .enter(&id)
        .run_with_handle();
    ```
    
    `&ExpectedSpan` - The expected span is taken by reference and cloned.
    
    ```rust
    let span = expect::span().named("span name");
    
    let (collector, handle) = collector::mock()
        .enter(&span)
        .exit(&span)
        .run_with_handle();
    ```
    
    In Rust, taking a reference to an object and immediately cloning it is
    an anti-pattern. It is considered better to force the user to clone
    outside the API to make the cloning explict.
    
    However, in the case of a testing framework, it seems reasonable to
    prefer a more concise API, rather than having it more explicit.
    
    To reduce the size of this PR and to avoid unnecessary churn in other
    crates, the tests within the tracing repo which use `tracing-mock` will
    not be updated to use the new `Into<ExpectedSpan>` capabilities. The new
    API is backwards compatible and those tests can remain as they are.
    hds committed Oct 3, 2024
    Configuration menu
    Copy the full SHA
    17a232d View commit details
    Browse the repository at this point in the history