Skip to content

Conversation

@AnnaSasDev
Copy link
Contributor

@AnnaSasDev AnnaSasDev commented Apr 30, 2025

Please check the following before creating a Pull Request

  • If this is a new feature or piece of functionality, have you started a discussion and gotten agreement on it?

    I havent started a discussion about it, but I think I follow the naming convention with DoesNot... like DoesNotEndWith()

  • Did you write tests to ensure you code works properly?

    I didnt write a test for this, as I didnt find a link to an existing test for the original Matches assertion, but the method is a simple inversion of the original Matches assertion method.
    I would be more than happy to write some tests for it, but I dont know exactly where I would need to put them.

@thomhurst
Copy link
Owner

Thanks! Can you add some tests for positive and negative paths?

@AnnaSasDev
Copy link
Contributor Author

Thanks! Can you add some tests for positive and negative paths?

yes absolutely! I was just unsure where to exactly place them, as to my knowledge the original .Matches() method didnt have any tests either. Do I place these in the TUnit.Assertions.UnitTests? I'll make both tests for .Matches() and .DoesNotMatch() then.

@AnnaSasDev
Copy link
Contributor Author

Tests are running successfully locally, but had to do some #if NET logic to ensure net framework compatibility.
I tried to follow the standerds from the other tests I could find in the same project.
If I did anything wrong, please do tell me, so I can do better in the future

@AnnaSasDev
Copy link
Contributor Author

Ah, I see Codacy has an issue on partial class, probably on the net framework flow as far as I can imagine, as the partial keyword isnt needed then.
Do I add an extra #if to resolve this, which seems a bit overly verbose for me?

@thomhurst
Copy link
Owner

Don't worry about codacy - I can sort it later.

You need to run the public API tests, which will fail, and then accept the new snapshots (instructions are here if you've not used verify: https://github.com/VerifyTests/Verify?tab=readme-ov-file#initial-verification) and then commit them

@AnnaSasDev
Copy link
Contributor Author

Sorry, I had missed the net framework verify

@thomhurst
Copy link
Owner

Thanks!

@thomhurst thomhurst merged commit 8a44603 into thomhurst:main May 1, 2025
7 of 9 checks passed
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.

2 participants