Skip to content

Conversation

@TheConstructor
Copy link
Contributor

What does this PR do?

Add a new test-project "Testcontainers.XunitV3.Tests" to verify the "Testcontainers.XunitV3"-package. The test-project depends on the current xunit V3, to also proof, that it can be used alongside it.

Why is it important?

Currently there is no test for the "Testcontainers.XunitV3"-package, which means, that runtime issues can not be detected. This should also ease verifying further development around "Testcontainers.XunitV3" and showcase the usage of the package.

@netlify
Copy link

netlify bot commented Apr 18, 2025

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit f73b78d
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/68061730d92584000816c5c9
😎 Deploy Preview https://deploy-preview-1430--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@TheConstructor TheConstructor force-pushed the feature/xunit3-v2 branch 2 times, most recently from 38171ea to 3defd02 Compare April 18, 2025 13:06
@0xced
Copy link
Contributor

0xced commented Apr 20, 2025

I'd rather reuse the whole Testcontainers.Xunit.Tests source code and sprinkle a few #if XUNIT_V3 instead of duplicating all the code, like it was done for the Testcontainers.Xunit and Testcontainers.XunitV3 packages.

Actually, I already did exactly this here 0xced@609a08c but haven't opened a pull request yet in order not to overwhelm @HofmeisterAn with yet another pull request from me. 😆

@TheConstructor
Copy link
Contributor Author

TheConstructor commented Apr 20, 2025

@0xced why did you need the .editorconfig? I also would argue a bit for the copy to be better suited as usage-example. I locally also played a bit with enhancing the documentation with the Xunit.net V3-stuff, but I thought that that may be better in a separate PR.

Lastly I also wondered, if it makes sense to switch the main-testing-framework for Xunit.net V3, but again, that probably should be a separate PR

@0xced
Copy link
Contributor

0xced commented Apr 20, 2025

why did you need the .editorconfig?

It's there in all projects, but I'm not sure why. 🤷🏻‍♂️

I also would argue a bit for the copy to be better suited as usage-example.

I don't understand why. Currently, the Testcontainers.Xunit and Testcontainers.XunitV3 are equal from a features point of view. Usage examples should be identical for both packages.

I locally also played a bit with enhancing the documentation with the Xunit.net V3-stuff.

Do you mean enhancing the documentation with the Xunit.net V3-stuff only? Because enhancing the V3 stuff should also automatically enhance the xUnit.net v2 documentation, shouldn't it?

Lastly I also wondered, if it makes sense to switch the main-testing-framework for Xunit.net V3, but again, that probably should be a separate PR.

Yes, this should definitely be discussed separately.

@TheConstructor
Copy link
Contributor Author

TheConstructor commented Apr 20, 2025

why did you need the .editorconfig?

It's there in all projects, but I'm not sure why. 🤷🏻‍♂️

Ok, seems I missed that then. It seems we want to be very sure to use all the defaults

I also would argue a bit for the copy to be better suited as usage-example.

I don't understand why. Currently, the Testcontainers.Xunit and Testcontainers.XunitV3 are equal from a features point of view. Usage examples should be identical for both packages.

But the XUnit-framework did change - and I wonder how long both packages are just referencing things by the same name in different namespaces. Take for example ITestOutputHelper that is present in the *ContainerTest-constructors. There is now TestContext.Current.TestOutputHelper and we could get totally rid of passing around ITestOutputHelper, and instead have a bool with a default.

I locally also played a bit with enhancing the documentation with the Xunit.net V3-stuff.

Do you mean enhancing the documentation with the Xunit.net V3-stuff only? Because enhancing the V3 stuff should also automatically enhance the xUnit.net v2 documentation, shouldn't it?

As stated above, there are some slight differences, and while the documentation does not include the usings, it's probably nicer to not have #if-lines included in the examples.

Or take your workaround for the missing TestContext.Current.CancellationToken - while your workaround currently wouldn't show up in the documentation, referencing TestContext.Current.CancellationToken would be shown as valid Xunit.net V2-code. I actually thought about whether I should present both versions, but decided, that the analyzer would likely do its magic. What's your opinion on that?

Signed-off-by: Andre Hofmeister <[email protected]>
HofmeisterAn
HofmeisterAn previously approved these changes Apr 21, 2025
Copy link
Collaborator

@HofmeisterAn HofmeisterAn left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, LGTM! What do you think @0xced?

As stated above, there are some slight differences, and while the documentation does not include the usings, it's probably nicer to not have #if-lines included in the examples.

Yeah, this is a general issue we're dealing with right now. A lot of our tests aren't ideal to use them directly in the docs. Usually, the test setups are kind of weird and don't fit well in the docs. We should probably split it and use a dedicated project for the docs or something like that.

@HofmeisterAn HofmeisterAn added the chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups label Apr 22, 2025
@HofmeisterAn HofmeisterAn changed the title Add tests against XUnit V3 2.0.1 chore: Add xUnit.net v3 tests Apr 22, 2025
@HofmeisterAn HofmeisterAn merged commit d053837 into testcontainers:develop Apr 27, 2025
68 checks passed
@TheConstructor TheConstructor deleted the feature/xunit3-v2 branch April 27, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants