Skip to content

refactor(csharp): rewrite integration tests to remove DependsOn#2710

Open
atharvalade wants to merge 7 commits intoapache:masterfrom
atharvalade:refactor/csharp-rewrite-integration-tests
Open

refactor(csharp): rewrite integration tests to remove DependsOn#2710
atharvalade wants to merge 7 commits intoapache:masterfrom
atharvalade:refactor/csharp-rewrite-integration-tests

Conversation

@atharvalade
Copy link
Contributor

Remove all DependsOn attributes from 11 test files. Each test now creates its own resources with unique names via Guid.NewGuid(), ensuring full isolation and eliminating ordering dependencies.

Delete 16 unused fixture, helper, and model files that were only needed by the old DependsOn-based test structure.

Which issue does this PR close?

Closes #2654

Rationale

The DependsOn attribute creates ordering dependencies between tests, which leads to flaky failures and makes it hard to run tests in isolation. Independent tests are more reliable and easier to debug.

What changed?

Tests relied on DependsOn to run in a fixed sequence, sharing resources created by earlier tests. If one test failed, all downstream tests would skip or fail regardless of their own correctness.

Each test now creates its own stream, topic, and other resources with unique Guid-based names, then cleans up or simply leaves them isolated. The 16 fixture, helper, and model files that only existed to support the old shared-state pattern were removed as dead code.

Local Execution

  • Passed
  • Pre-commit hooks not ran (non-Rust changes only, prek targets Rust code)

AI Usage

  1. Opus 4.6
  2. Used for repetitive boilerplate and verifying no dangling references after deletions
  3. Verified by building locally (0 warnings, 0 errors) and running dotnet format
  4. Yes

@atharvalade atharvalade force-pushed the refactor/csharp-rewrite-integration-tests branch from 9fe5903 to b2e7673 Compare February 10, 2026 03:19
Copy link
Contributor

@lukaszzborek lukaszzborek left a comment

Choose a reason for hiding this comment

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

I added few comments, since now tests are no longer run in order, it should check more then just not throwing error

…he#2654)

Remove all DependsOn attributes from 11 test files. Each test now
creates its own resources with unique names via Guid.NewGuid(),
ensuring full isolation and eliminating ordering dependencies.

Delete 16 unused fixture, helper, and model files that were only
needed by the old DependsOn-based test structure.
@atharvalade atharvalade force-pushed the refactor/csharp-rewrite-integration-tests branch from b2e7673 to 2ff8ead Compare February 10, 2026 22:59
@atharvalade
Copy link
Contributor Author

@lukaszzborek @hubcio @spetz whenever you guys get a chance, please resolve the conversations and merge if appropriate!

Copy link
Contributor

@lukaszzborek lukaszzborek left a comment

Choose a reason for hiding this comment

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

LGTM now,
but you will have to do some tests change after merge #2701

@atharvalade
Copy link
Contributor Author

LGTM now, but you will have to do some tests change after merge #2701

cool! I'll wait for merge.

Remove DependsOn and SegmentsFixture, make tests self-contained with Guid-based names
@atharvalade
Copy link
Contributor Author

LGTM now, but you will have to do some tests change after merge #2701

cool! I'll wait for merge.

Merged master with #2701 and updated the new SegmentsTests.cs to follow the self-contained pattern deleted SegmentsFixture.cs and removed the DependsOn attributes.

spetz
spetz previously approved these changes Feb 15, 2026
@lukaszzborek
Copy link
Contributor

@atharvalade
can you check at this failing build?

@atharvalade
Copy link
Contributor Author

@atharvalade can you check at this failing build?

@lukaszzborek The CI was failing because merging master brought in PR #2734 (cooperative partition reassignment), which modified ConsumerGroupTests to use Fixture.PartitionsCount. On master, Fixture is a ConsumerGroupFixture that has that property, but on this branch it's an IggyServerFixture which doesn't. Fixed by referencing the local PartitionsCount constant already defined in the class.

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.

Rewrite .net integration tests

3 participants