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

Improve tests to make the migration to NSubstitute easier #501

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

304NotModified
Copy link
Contributor

@304NotModified 304NotModified commented Feb 26, 2025

🤔 What's changed?

Refactored the tests, but don't changed the behavior of the tests.

  • Remove LegacyStepArgumentTypeConverterExtensions - it's a strange pattern and makes tests hard to understand
    • e.g. StepArgumentTypeConverterStub.Setup(LegacyStepArgumentTypeConverterExtensions.GetCanConvertMethodFilter is unclear/vague
    • It was sometimes unclear which method we were mocking/assertion (because of the extension)
  • Add ArgumentHelpers to avoid to much duplication in the tests and make the tests more readable.
  • Removed dead test code (commented out in the touched files)
  • Remove some unused vars in the touched files
  • Fixed some spelling in comments

Note, I used Resharper for most actions, e.g. inline method, inline variable etc.

⚡️ What's your motivation?

The POC with the migration to NSubstitute
#497 showed use that some parts where difficult to migration.

🏷️ What kind of change is this?

  • 🏦 Refactoring in tests

♻️ Anything particular you want feedback on?

Do we add these changes to the changelog?

📋 Checklist:

  • I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

Remove LegacyStepArgumentTypeConverterExtensions

Add ArgumentHelpers

Removed dead test code

Remove some unused vars in the tests

Fixed some spelling in comments
@@ -84,6 +85,7 @@ public class StepArgumentTransformationTests
private readonly Mock<IContextManager> contextManagerStub = new Mock<IContextManager>();
private readonly Mock<IAsyncBindingInvoker> methodBindingInvokerStub = new Mock<IAsyncBindingInvoker>();
private readonly List<IStepArgumentTransformationBinding> stepTransformations = new List<IStepArgumentTransformationBinding>();
private readonly CultureInfo _enUSCulture = new CultureInfo("en-US", false);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI same pattern as in StepArgumentTypeConverterTest.cs

@304NotModified 304NotModified marked this pull request as ready for review February 26, 2025 09:54
@304NotModified 304NotModified changed the title Improve tests to improve migration to NSubstitute Improve tests to make the migration to NSubstitute easier Feb 26, 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.

1 participant