Skip to content

Fixing event pipeline provider type#127

Merged
MichaelGHSeg merged 3 commits into
mainfrom
mgh/event_pipeline_provider_type_fix
Mar 4, 2025
Merged

Fixing event pipeline provider type#127
MichaelGHSeg merged 3 commits into
mainfrom
mgh/event_pipeline_provider_type_fix

Conversation

@MichaelGHSeg

Copy link
Copy Markdown
Contributor

Custom pipelines can now be set, plus a test to validate they are used.

didiergarcia
didiergarcia previously approved these changes Feb 28, 2025

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Overview

This PR updates the event pipeline provider configuration to use a custom provider interface and validates its usage through new tests.

  • Updates Configuration.cs to accept IEventPipelineProvider instead of a concrete type.
  • Introduces new tests validating both standard and custom event pipeline provider implementations.

Reviewed Changes

File Description
Tests/Utilities/EventPipelineTest.cs Adds tests to validate that the event pipeline providers, both standard and custom, are used in the configuration.
Analytics-CSharp/Segment/Analytics/Configuration.cs Changes the parameter type for event pipeline provider to IEventPipelineProvider, supporting pluggable custom implementations.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

Tests/Utilities/EventPipelineTest.cs:225

  • [nitpick] Consider adding an assertion to verify that the event pipeline provider is actively processing or modifying events, to ensure complete test coverage of the provider integration.
analytics.Track("test");

Tests/Utilities/EventPipelineTest.cs:241

  • [nitpick] Consider asserting the exception message or adding further validation to ensure that the NotImplementedException is raised for the expected reason, if a specific message is available.
Assert.Throws<NotImplementedException>(() => {

wenxi-zeng
wenxi-zeng previously approved these changes Mar 4, 2025
Comment thread Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs Outdated
Comment thread Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs
Comment thread Analytics-CSharp/Segment/Analytics/Utilities/SyncEventPipeline.cs Outdated
@MichaelGHSeg MichaelGHSeg merged commit 6d875a0 into main Mar 4, 2025
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