Skip to content

Conversation

@james-mcnulty
Copy link
Member

Description

Right now, every InflightActivation in the unit tests is constructed manually field by field. This is extremely verbose and largely unnecessary, as most fields are set to use default values (for example, 0 when the type is an integer or None when the type is an Option).

We can reduce this boilerplate by using the builder pattern to construct these structs instead. That way, you only need to set the fields you are interested in, and it's easier to tell what's different between two InflightActivations as you don't need to parse a million irrelevant fields that are all the same.

For a demonstration, consider the two activations A and B below. What's the difference between them?

Activation A

InflightActivation {
    id: "task-123".to_string(),
    namespace: "my-namespace".to_string(),
    taskname: "my-task".to_string(),
    activation: TaskActivation {
        id: "task-123".to_string(),
        namespace: "my-namespace".to_string(),
        taskname: "my-task".to_string(),
        parameters: "{}".to_string(),
        headers: HashMap::new(),
        processing_deadline_duration: 0,
        received_at: None,
        retry_state: None,
        expires: None,
        delay: None,
    }.encode_to_vec(),
    status: InflightActivationStatus::Pending,
    partition: 0,
    offset: 0,
    added_at: Utc::now(),
    received_at: Utc::now(),
    processing_attempts: 0,
    processing_deadline_duration: 0,
    expires_at: None,
    delay_until: None,
    processing_deadline: None,
    on_attempts_exceeded: OnAttemptsExceeded::Discard,
    at_most_once: false
}

Activation B

InflightActivation {
    id: "task-123".to_string(),
    namespace: "my-namespace".to_string(),
    taskname: "my-task".to_string(),
    activation: TaskActivation {
        id: "task-123".to_string(),
        namespace: "my-namespace".to_string(),
        taskname: "my-task".to_string(),
        parameters: "{}".to_string(),
        headers: HashMap::new(),
        processing_deadline_duration: 0,
        received_at: None,
        retry_state: None,
        expires: None,
        delay: None,
    }.encode_to_vec(),
    status: InflightActivationStatus::Pending,
    partition: 0,
    offset: 10,
    added_at: Utc::now(),
    received_at: Utc::now(),
    processing_attempts: 0,
    processing_deadline_duration: 0,
    expires_at: None,
    delay_until: None,
    processing_deadline: None,
    on_attempts_exceeded: OnAttemptsExceeded::Discard,
    at_most_once: false
}

Activation A has an offset of 0 while B has an offset of 10. Even if it didn't take very long to figure that out, you still had to read quite a few irrelevant fields that are the same between both activations.

Now consider the same two activations constructed using a builder. We don't need to read all the fields because only the offset setter is used! This tells us almost immediately that the offset is the only value that differs between the two tasks.

Activation A

InflightActivationBuilder::default()
    .id("task-123")
    .namespace("my-namespace")
    .taskname("my-task")
    .offset(0)
    .activation(
        TaskActivationBuilder::default()
            .id(id)
            .namespace("my-namespace")
            .taskname("my-task")
            .build()
    )
    .build()

Activation B

InflightActivationBuilder::default()
    .id("task-123")
    .namespace("my-namespace")
    .taskname("my-task")
    .offset(10)
    .activation(
        TaskActivationBuilder::default()
            .id(id)
            .namespace("my-namespace")
            .taskname("my-task")
            .build()
    )
    .build()

As a result, it should be easier to write, read, and understand tests.

Details

  • Used the derive_builder crate to generate a builder for InflightActivation with reasonable defaults
  • Manually implemented a builder for TaskActivation since it's generated by sentry-protos
  • Replaced every occurance of InflightActivation { ... } in the tests with the builder

@james-mcnulty
Copy link
Member Author

Since the new builder boilerplate takes around 150 lines, these changes remove nearly 300 lines from the tests overall. We could cut even more code by...

  • Generating the builder for TaskActivation in sentry-protos instead
  • Building the TaskActivation inside of the InflightActivationBuilder instead of requiring the user to provide an activation

Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Nice work. This should make writing tests simpler.

@james-mcnulty james-mcnulty marked this pull request as ready for review December 31, 2025 19:17
@james-mcnulty james-mcnulty requested a review from a team as a code owner December 31, 2025 19:17
Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Nice work 👏

@james-mcnulty james-mcnulty requested a review from a team as a code owner January 15, 2026 20:48
@james-mcnulty james-mcnulty changed the title Use Builder for InflightActivation [STREAM-689] Use Builder for InflightActivation Jan 15, 2026
@linear
Copy link

linear bot commented Jan 15, 2026

@james-mcnulty james-mcnulty merged commit 90d554a into main Jan 15, 2026
18 checks passed
@james-mcnulty james-mcnulty deleted the george/activation-builder branch January 15, 2026 21:18
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.

4 participants