Skip to content

Conversation

@thomhurst
Copy link
Owner

Summary

  • Add [Platform] attribute to recognized NUnit attributes (removed during migration as no TUnit equivalent exists)
  • Add [Description] attribute handling (removed as no direct TUnit equivalent)
  • Add [Theory] -> [Test] mapping (NUnit Theory is the same as Test)
  • Add [Ignore] -> [Skip] mapping for proper attribute conversion
  • Update ShouldRemoveAttribute to include Platform and Description
  • Add tests for Platform and Description attribute removal

Test plan

  • Run NUnit migration tests (67 passed)
  • Run xUnit migration tests (39 passed, 1 skipped)
  • Run MSTest migration tests (23 passed)

Fixes #4337, #4339, #4340

🤖 Generated with Claude Code

…butes in migration

- Add [Platform] attribute to recognized NUnit attributes (removed during migration as no TUnit equivalent)
- Add [Description] attribute handling (removed as no direct TUnit equivalent)
- Add [Theory] -> [Test] mapping (NUnit Theory is same as Test)
- Add [Ignore] -> [Skip] mapping for proper attribute conversion
- Update ShouldRemoveAttribute to include Platform and Description
- Add tests for Platform and Description attribute removal

Fixes #4337, #4339, #4340

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@thomhurst
Copy link
Owner Author

Summary

Adds migration support for NUnit's [Platform], [Description], [Theory], and [Ignore] attributes during NUnit to TUnit conversion.

Critical Issues

None found ✅

Review Notes

The implementation correctly handles the newly added attributes according to TUnit migration patterns:

Additions Reviewed:

  1. [Platform] attribute - Correctly removed (no TUnit equivalent, line 68 in CodeFixProvider, line 32 in MigrationHelpers)

    • Properly added to IsFrameworkAttribute check
    • Included in ShouldRemoveAttribute logic
  2. [Description] attribute - Correctly removed with helpful comment (line 68, 33 in MigrationHelpers)

    • Comment notes to use [Property] if needed - good guidance
  3. [Theory] attribute - Correctly mapped to [Test] (line 25 in MigrationHelpers)

    • Makes sense as NUnit Theory is functionally identical to Test
  4. [Ignore] attribute - Correctly mapped to [Skip] (line 34 in MigrationHelpers)

    • Proper semantic conversion
  5. Tests - Two new tests verify Platform and Description removal behavior

    • Tests follow existing patterns
    • Verify complete attribute removal (not just conversion)

Code Quality:

  • Consistent with existing codebase patterns
  • No performance concerns (compile-time code generation)
  • No security issues
  • No reflection or AOT concerns (analyzer/code fixer context)

The changes are minimal, focused, and correctly implement the missing attribute handling reported in the linked issues.

Verdict

APPROVE - No critical issues. Changes correctly implement NUnit attribute migration following TUnit patterns.

…ssues-batch2

Resolved conflict in NUnitMigrationAnalyzerTests.cs by keeping both
test sets: Platform/Description attribute tests from PR and
InterfaceImplementation test from main.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@thomhurst
Copy link
Owner Author

Summary

Adds migration support for NUnit's [Platform], [Description], [Theory], and [Ignore] attributes during NUnit to TUnit conversion.

Critical Issues

None found ✅

Review Notes

The implementation correctly handles the newly added attributes according to TUnit migration patterns:

Additions Reviewed:

  1. [Platform] attribute (TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs:68, TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs:33,123)

    • Correctly added to IsFrameworkAttribute check
    • Properly included in ShouldRemoveAttribute logic to be removed (no TUnit equivalent)
    • Test coverage added in NUnitMigrationAnalyzerTests.cs:2477-2509
  2. [Description] attribute (TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs:33,123)

    • Correctly removed with helpful comment noting to use [Property] if needed
    • Properly included in ShouldRemoveAttribute logic
    • Test coverage added in NUnitMigrationAnalyzerTests.cs:2511-2545
  3. [Theory] attribute (TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs:25)

    • Correctly mapped to [Test] with clear comment explaining NUnit Theory is same as Test
    • Makes semantic sense as NUnit Theory behaves identically to Test
    • Properly added to IsFrameworkAttribute check (TUnit.Analyzers.CodeFixers/NUnitMigrationCodeFixProvider.cs:68)
  4. [Ignore] attribute (TUnit.Analyzers/Migrators/Base/MigrationHelpers.cs:33)

    • Correctly mapped to [Skip] - proper semantic conversion
    • Already present in IsFrameworkAttribute check (line 68 of CodeFixProvider)
  5. Tests - Two new comprehensive tests verify Platform and Description removal

    • Tests follow existing codebase patterns
    • Verify complete attribute removal (not just conversion)
    • Proper setup with ConfigureNUnitTest helper

Code Quality:

  • ✅ No dual-mode concerns (analyzer/code fixer, not engine code)
  • ✅ No snapshot test updates needed (no source generator output changed)
  • ✅ No public API changes
  • ✅ No VSTest usage
  • ✅ No performance concerns (compile-time code generation)
  • ✅ No security issues
  • ✅ No AOT concerns (analyzer context)
  • ✅ Consistent with existing migration patterns
  • ✅ Minimal, focused changes

The changes correctly implement the missing attribute handling reported in issues #4337, #4339, and #4340.

Previous Review Status

The PR author previously reviewed this PR and approved it. This independent review confirms those findings.

Verdict

APPROVE - No critical issues. Changes correctly implement NUnit attribute migration following TUnit patterns and critical rules.

@thomhurst thomhurst merged commit 17cd7db into main Jan 13, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the fix/nunit-migration-issues-batch2 branch January 13, 2026 00:22
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.

NUnit Migration: using NUnit.Framework; statements not removed

2 participants