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

Refactor the test suite #437

Merged
merged 2 commits into from
Feb 13, 2025
Merged

Refactor the test suite #437

merged 2 commits into from
Feb 13, 2025

Conversation

fiseni
Copy link
Collaborator

@fiseni fiseni commented Feb 13, 2025

This is a huge PR. Before we start with all the planned changes for version 9, I refactored the whole test suite and rewrote the tests from the ground up.

  • Now that we have specification constructors as public, the specs are created within each test. This makes the tests self-contained and much easier to reason about. Removed all shared specifications that were used previously.
  • Rewrote all the tests they were not very consistent. Also, I added tons of new tests.
  • Added and utilized the Testcontainers.MsSql package. We no longer need the docker files, we can just run the dotnet test, simple as that. By default, we're also checking whether LocalDB is installed. If so, we're not even using containers at all. This can be overridden manually.

This is not done. I'll keep improving and adding tests while working on the features. The aim is to have 100% line and branch coverage.

@fiseni fiseni requested a review from ardalis February 13, 2025 10:28
@@ -0,0 +1,45 @@
//namespace Tests.Evaluators;
Copy link
Owner

Choose a reason for hiding this comment

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

Can we remove this or uncomment?

@@ -0,0 +1,21 @@
//using System.Linq.Expressions;
Copy link
Owner

Choose a reason for hiding this comment

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

Uncomment or remove?

@@ -0,0 +1,108 @@
//namespace Tests.Extensions;
Copy link
Owner

Choose a reason for hiding this comment

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

Delete or uncomment

@@ -0,0 +1,89 @@
//using System.Runtime.CompilerServices;
Copy link
Owner

Choose a reason for hiding this comment

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

Uncomment or remove

@ardalis
Copy link
Owner

ardalis commented Feb 13, 2025

I assume the commented full files are TODO...

@fiseni
Copy link
Collaborator Author

fiseni commented Feb 13, 2025

@ardalis In the next PR, I'll refactor the solution, and update the TFMs (add net9.0 and drop the unsupported ones). Also, I'll do a general cleanup (update the editorconfig and clean everything). There are some issues with "implicit usings" for .NET Framework consumers. I'll have to figure that part out. So, all those using statements will be consolidated.

As for tests, yes I've commented some of them. I'll be working on them as I work on the new features. At the end of this process, we'll have a consolidated and robust test suite.

This PR is already massive. So, let's merge it if possible, and I'll sort out everything along the way.

@ardalis ardalis merged commit 237f2e2 into main Feb 13, 2025
1 check passed
@ardalis
Copy link
Owner

ardalis commented Feb 13, 2025

If I approve you're welcome to merge - no need to wait for me. Sometimes I'll avoid merging myself in case you wanted to change anything before merging yourself.

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