feat: Introduce a new Testcontainers.Xunit package
What does this PR do?
This pull request introduces a new Testcontainers.Xunit NuGet package.
It provides two main classes to simplify working with Testcontainers within xUnit.net tests.
ContainerTestis a base class for tests needing one container per test method.ContainerFixtureis a fixture that can be injected into test classes that need one container per test class (a.k.a. singleton container instance).
Both support logging, respectively through ITestOutputHelper and IMessageSink which are the standard xUnit.net logging mechanisms.
DbContainerTest and DbContainerFixture are also provided to ease working with database containers. They provide methods to simplify the creation of DbConnection instances.
Why is it important?
This will greatly reduce boilerplate code needed when using Testcontainers with xUnit.net, both for Testcontainers consumers and for the Testcontainers tests themselves.
Related issues
- Closes #990
Follow-ups
I have another branch (feature/Testcontainers.Xunit+samples) where I have started using Testcontainers.Xunit in the Testcontainers tests themselves. I have currently updated MongoDbContainerTest, ClickHouseContainerTest, MariaDbContainerTest and PostgreSqlContainerTest. You can have a look at them to see how the tests are simplified by using the new base classes or fixtures.
I'm not sure yet whether updating all the Testcontainers tests should be part of this pull request or part of a subsequent pull request.
I've been using xUnit.net extensively so I'm pretty confident about the design of this new package. I'm not familiar enough with either NUnit or MSTest to propose similar packages, maybe someone else can step in.
Deploy Preview for testcontainers-dotnet ready!
| Name | Link |
|---|---|
| Latest commit | ea693d8f089cb9faccb329dd0abae9d52fbf7013 |
| Latest deploy log | https://app.netlify.com/sites/testcontainers-dotnet/deploys/66edb98be54d15000805df5b |
| Deploy Preview | https://deploy-preview-1165--testcontainers-dotnet.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
I think we can consolidate a few things. The underlying configuration for a single instance (ContainerTest) and a shared instance (ContainerFixture) do not differ that much. At least in the past, I reused the same implementation. I think we can do the same here. I will share the setup I typically use tomorrow.
Excellent point! It did not even come to my mind that the lifetime implementation could be shared. I have done it in c20188e0938ffc9e88b413c4ccf933a329ad71f0 with a new ContainerLifetime base class used by both ContainerTest and ContainerFixture.
Sorry for the late response today. Tuesdays are always busy for me.
Excellent point! It did not even come to my mind that the lifetime implementation could be shared. I have done it in c20188e with a new
ContainerLifetimebase class used by bothContainerTestandContainerFixture.
π We do not need to apply the suggestion, but I would like to share and discuss it quickly because I think the code (xUnit's shared context) has a lot in common, and some parts feel like copy and paste. The example I am sharing is not complete because I had to remove some critical parts, but the important parts are still there, and the concept should not be very difficult to understand. Please also take into account that the example contains much more than what is actually necessary for Testcontainers.
The ILoggerProvider implementation is similar to what @samtrion shared. The ILogger implementation is the same for both ITestOutputHelper and IMessageSink, so I do not think code duplication is really necessary.
internal sealed class XunitLoggerProvider : ILoggerProvider
{
private readonly Stopwatch _stopwatch = Stopwatch.StartNew();
private readonly ITestOutputHelper _testOutputHelper;
public XunitLoggerProvider(IMessageSink messageSink)
{
_testOutputHelper = new MessageSinkTestOutputHelper(messageSink);
}
public XunitLoggerProvider(ITestOutputHelper testOutputHelper)
{
_testOutputHelper = testOutputHelper;
}
public void Dispose()
{
}
public ILogger CreateLogger(string categoryName)
{
return new XunitLogger(_stopwatch, _testOutputHelper, categoryName);
}
private sealed class MessageSinkTestOutputHelper : ITestOutputHelper
{
private readonly IMessageSink _messageSink;
public MessageSinkTestOutputHelper(IMessageSink messageSink)
{
_messageSink = messageSink;
}
public void WriteLine(string message)
{
_messageSink.OnMessage(new DiagnosticMessage(message));
}
public void WriteLine(string format, params object[] args)
{
_messageSink.OnMessage(new DiagnosticMessage(format, args));
}
}
private sealed class XunitLogger : ILogger
{
private readonly Stopwatch _stopwatch;
private readonly ITestOutputHelper _testOutputHelper;
private readonly string _categoryName;
public XunitLogger(Stopwatch stopwatch, ITestOutputHelper testOutputHelper, string categoryName)
{
_stopwatch = stopwatch;
_testOutputHelper = testOutputHelper;
_categoryName = categoryName;
}
public IDisposable BeginScope<TState>(TState state)
{
return Disposable.Instance;
}
public bool IsEnabled(LogLevel logLevel)
{
return logLevel != LogLevel.None;
}
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
{
_testOutputHelper.WriteLine("[{0} {1:hh\\:mm\\:ss\\.ff}] {2}", _categoryName, _stopwatch.Elapsed, formatter.Invoke(state, exception));
}
private sealed class Disposable : IDisposable
{
private Disposable()
{
}
public static IDisposable Instance { get; } = new Disposable();
public void Dispose()
{
}
}
}
}
I had to remove some parts here that I cannot share. The example builds on top of the WebApplicationFactory<TEntryPoint>, but the concept will be the same for Testcontainers. The actual test class utilizes either the SharedInstance or the SingleInstance implementation.
public abstract class CustomWebApplicationFactory<TEntryPoint> : WebApplicationFactory<TEntryPoint>, IAsyncLifetime where TEntryPoint : class
{
private readonly List<IWebHostConfiguration> _configurations = new();
protected CustomWebApplicationFactory()
{
AddWebHostConfiguration(new ClearLoggingProviders());
}
public void AddWebHostConfiguration(IWebHostConfiguration configuration)
{
_configurations.Add(configuration);
}
protected override void ConfigureWebHost(IWebHostBuilder builder)
{
base.ConfigureWebHost(builder);
foreach (var configuration in _configurations)
{
configuration.ConfigureWebHost(builder);
}
}
// For Testcontainers, the AsyncLifetime property is not necessary; this is specific to the type clash (WebApplicationFactory<TEntryPoint>).
public IAsyncLifetime AsyncLifetime => this;
public ValueTask InitializeAsync()
{
// Due to a naming conflict where the interfaces `IAsyncLifetime` and
// `IAsyncDisposable` share the same name for the member `DisposeAsync`, but have
// different return types, we have implemented the members of the `IAsyncLifetime`
// interface explicitly. To prevent warnings, a non-explicit implementation has
// been added. This matter will be addressed in xUnit.net version 3, which also
// utilizes the `IAsyncDisposable` interface.
throw new InvalidOperationException("Use the delegate property AsyncLifetime instead.");
}
async Task IAsyncLifetime.InitializeAsync()
{
await Task.WhenAll(_configurations.Select(configuration => configuration.InitializeAsync()))
.ConfigureAwait(false);
}
async Task IAsyncLifetime.DisposeAsync()
{
await Task.WhenAll(_configurations.Select(configuration => configuration.DisposeAsync()))
.ConfigureAwait(false);
}
public class SharedInstance : CustomWebApplicationFactory<TEntryPoint>
{
public SharedInstance(IMessageSink messageSink)
{
AddWebHostConfiguration(new LoggingWebHostConfiguration(messageSink));
}
}
public class SingleInstance : CustomWebApplicationFactory<TEntryPoint>
{
private readonly MoqLoggerProvider _mockOfILogger = new();
public SingleInstance(ITestOutputHelper testOutputHelper)
{
AddWebHostConfiguration(new LoggingWebHostConfiguration(testOutputHelper));
AddWebHostConfiguration(new LoggingWebHostConfiguration(_mockOfILogger));
}
public Mock<ILogger> MockOfILogger => _mockOfILogger;
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
_mockOfILogger.Dispose();
}
}
private sealed class ClearLoggingProviders : WebHostConfiguration
{
public override void ConfigureWebHost(IWebHostBuilder builder)
{
builder.ConfigureTestServices(services => services.RemoveAll<ILoggerFactory>());
builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders());
}
}
}
The test class will use either one of the instances, similar to:
public class XunitShardContext1 : IClassFixture<CustomWebApplicationFactory<Program>.SharedInstance>
{
private readonly CustomWebApplicationFactory<Program> _app;
public XunitShardContext1(CustomWebApplicationFactory<Program>.SharedInstance app)
{
_app = app;
}
}
public class XunitShardContext2 : IAsyncLifetime
{
private readonly CustomWebApplicationFactory<Program> _app;
public XunitShardContext2(ITestOutputHelper testOutputHelper)
{
_app = new CustomWebApplicationFactory<Program>.SingleInstance(testOutputHelper);
}
}
@0xced, are there any tasks left? It would be great to finalize the PR and merge it. I think many developers can benefit from this package. Please, no hurry β I just want to double-check if there are any tasks left. I can take care of them too if you need help.
Iβm pretty happy with the current implementation. I have another branch (feature/Testcontainers.Xunit+samples) on top of this one where I started using the new package for Testcontainers own tests. Currently I have only updated a couple of modules and everything looks fine.
Do you plan to release a preview package first so that we could gather some feedback before committing to a stable API ?
Finally, except for the xmldoc, no documentation has been written. I can start writing documentation next week.
Do you plan to release a preview package first so that we could gather some feedback before committing to a stable API ?
I will try to review the PR sometime this week. Afterward, I can publish a snapshot version π.
The PR looks good. We need to address the failing build and update the "container per test class" documentation, but overall it looks good. I'd like to run some local tests before merging it. Do we need to add additional tests?
I addressed the failing build. The cause was that I used xunit.v3.extensibility.core version 0.3.0-pre.8 which is not yet published to NuGet for the Testcontainers.XunitV3 package. I reverted to version 0.2.0-pre.69 in order to fix the build but it means that Testcontainers.XunitV3 will not work properly since it requires version 0.3.0-pre.8 where https://github.com/xunit/xunit/issues/3001 is fixed. Anyway, I think we should hold releasing Testcontainers.XunitV3 until the final xUnit v3 is released. But it was a good thing that I experimented with xUnit v3 since I recently reported two bugs which are already fixed!
I also finalised the Testing with xUnit documentation. I'm not super happy with the result as I'm not a good technical writer unfortunately. If someone else want to improve it or even rewrite it entirely that's would be fine.
Do we need to add additional tests?
I encourage you to have a look at my (feature/Testcontainers.Xunit+samples) branch where I have started using Testcontainers.Xunit in the Testcontainers tests themselves. I think these are perfect tests for the Testcontainers.Xunit package but I finally think that updating all Testcontainers module tests should be done as a subsequent pull request once this one is merged. Other than that I don't think additional tests are warranted.
ππ Really looking forward to this PR. Like seriously looking forward to this.
@0xced Having a read of the testing_xunit.md doc, there's a few examples which are awesome. Specifically, with the "Here's a more more complete example setting up a PostgreSQL database shared across all tests of the same class." example I noticed this:
// Match the database name with the one from the initialization script
public override string ConnectionString
{
get
{
var builder = new NpgsqlConnectionStringBuilder(base.ConnectionString)
{
Database = "chinook_auto_increment",
};
return builder.ConnectionString;
}
}
I can see here that you're setting the Database to be chinook_auto_increment for all tests in this class. Instead, could we be able to define the Database per test method which in effect would be a single db per test method. Why? So each test would have a fresh, consistent db per test method. Previously I've "reset" the database back. Which also means each test method (per class) has to run synchronously. If we have a single db tenant per test method (per class) we then have each test running in isolation which could mean we could even have each test running concurrently (if this is possible in v3?). At least it means we don't need to reset the db after a test method is done, ready for the next text method.
The problem I'd love to solve is having ONE database instance for the enter test run, over all test methods, over all test classes. This is like xUnit's COLLECTION Fixture concept. and then each test class (or test method if we could get those to run concurrently) to have it's own database tenant on this single instance. Sure, creating the db tenant and seeding the schema and data is costly .. but less cost than ALSO creating a db container for each test class or test method.
I sincerely hope this ramble makes some sense π π
I think we should hold releasing
Testcontainers.XunitV3until the final xUnit v3 is released.
Should we set IsPublishable to false then?
I'm not super happy with the result as I'm not a good technical writer unfortunately. If someone else want to improve it or even rewrite it entirely that's would be fine.
I can create a minimal test project using the source code from the docs and try to improve certain parts, but overall, I think it's already really good. Referencing the actual source code has the advantage of ensuring it compiles and stays up-to-date even when we make changes to the project.
Should we set
IsPublishabletofalsethen?
Either this or release a preview version of the Testcontainers.XunitV3 package.
I can create a minimal test project using the source code from the docs and try to improve certain parts, but overall, I think it's already really good. Referencing the actual source code has the advantage of ensuring it compiles and stays up-to-date even when we make changes to the project.
Awesome work on the documentation! I'd just change the RedisBuilder.RedisImage constant to its "redis:7.0" string value in the sample code. This is supposed to be read in a browser where you don't have easy access to the definition of this constant. Also, the docker ps command a few lines later displays redis:7.0 so that's good for the reader's comprehension.
Additionally, I made a small change to how we instantiate the logger instances.
Overall this looks good except for this specific change (commit 630a07320806be99667122b9fd764b98c3c523f6). I think it's worse than the previous implementation in many ways.
- There's no need for an
ILoggerProvider. It just complicates things IMHO. I don't see how a complex 95 lines file is better than two simple 21 lines (plus 20 lines base class). MessageSinkTestOutputHelperdoesn't really makes sense and implementingITestOutputHelperforces to throwNotImplementedExceptionin theOutputimplementation. Again, previous implementation was much simpler.- Even though it's recommended to pass
ITestOutputHelperandIMessageSinkto the base class, supporting null is probably a good idea. If I read the new implementation correctly, passing null will throw aNullReferenceException. In the previous implementation it would just not log anything which is probably what's you're expecting if you pass a null test output helper or null message sink. - The override of
GetHashCodeis important in order not to log the same information multiple times. This seems to be lost but right now I can't get any diagnostic to print to the console when runningdotnet testwith axunit.runner.jsonconfigured with diagnostic messages. It works in other projects but notTestcontainers.MariaDb.Testswhere I'm currently tryingβ¦ I'll try again tomorrow after some sleep. π΄
Awesome work on the documentation! I'd just change the
RedisBuilder.RedisImageconstant to its"redis:7.0"string value in the sample code.
Yes, I was thinking about that too, but I would like to reuse existing pulled images and avoid pulling more versions, since we had issues in the past with running out of disk space (I know, the Redis image is tiny). Maybe we can use a delegate that contains the version number in its name to make it more comprehensive.
β https://github.com/testcontainers/testcontainers-dotnet/pull/1165/commits/2e79dd7eabb29128a3b2e47ba4d19800361195fe: I replaced the const with a plain string and assert we are using the same version.
- There's no need for an
ILoggerProvider. It just complicates things IMHO. I don't see how a complex 95 lines file is better than two simple 21 lines (plus 20 lines base class).
I don't think so. The previous one was inconsistent in its log messages (this was the main reason I changed it). This is probably a matter of taste, but simply using fewer lines is not a valid argument. If necessary, you can reduce the XunitLoggerProvider to fewer lines, too (if you count everything that is involvedΒ (including removing the statement body, there is barely a difference). It aligns with @samtrion suggestion as well, and is something I would like to keep.
MessageSinkTestOutputHelperdoesn't really makes sense and implementingITestOutputHelperforces to throwNotImplementedExceptionin theOutputimplementation. Again, previous implementation was much simpler.
This is not an issue, as the implementation is not public. We can change this behavior in the future when Xunit.net V3 is officially available. Until then, a lot will probably change anyway.
- Even though it's recommended to pass
ITestOutputHelperandIMessageSinkto the base class, supporting null is probably a good idea. If I read the new implementation correctly, passing null will throw aNullReferenceException.
Yes, this is something I wanted to ask you about. It's good that you brought it up. My idea was to support a default constructor that doesn't expect any arguments and then uses a NullLogger instance (empty singleton of the interface). WDYT?
- The override of
GetHashCodeis important in order not to log the same information multiple times.
Probably, we are expecting different behavior here. I intended to log it once per fixture, but it looks like you want to log it once for the entire IMessageSink instance. I'm fine with either approach. We can override GetHashCode() then π.
β
https://github.com/testcontainers/testcontainers-dotnet/pull/1165/commits/2e79dd7eabb29128a3b2e47ba4d19800361195fe: I overrode the GetHashCode() method.
Edit: Something I wanted to mention is that in the future, I would like to transition to using a simple annotation that we can add to an IContainer field or property. That annotation would handle creating and starting the test resources. I was thinking of utilizing Fody for this.
The previous one was inconsistent in its log messages (this was the main reason I changed it).
This inconsistency was deliberate because a timestamp is already included by xUnit for diagnostic messages.
Previous version (without additional timestamps in the [testcontainers.org] tag):
[xUnit.net 00:00:00.54] Testcontainers.MariaDb.Tests: [testcontainers.org] Docker container 9b38c1107ab6 created
[xUnit.net 00:00:00.61] Testcontainers.MariaDb.Tests: [testcontainers.org] Start Docker container 9b38c1107ab6
[xUnit.net 00:00:00.90] Testcontainers.MariaDb.Tests: [testcontainers.org] Wait for Docker container 9b38c1107ab6 to complete readiness checks
[xUnit.net 00:00:00.90] Testcontainers.MariaDb.Tests: [testcontainers.org] Docker container 9b38c1107ab6 ready
[xUnit.net 00:00:00.96] Testcontainers.MariaDb.Tests: [testcontainers.org] Docker container b23d15ee4e5b created
[xUnit.net 00:00:00.96] Testcontainers.MariaDb.Tests: [testcontainers.org] Docker container bcb93b97b3bb created
[xUnit.net 00:00:00.98] Testcontainers.MariaDb.Tests: [testcontainers.org] Start Docker container b23d15ee4e5b
[xUnit.net 00:00:00.98] Testcontainers.MariaDb.Tests: [testcontainers.org] Start Docker container bcb93b97b3bb
[xUnit.net 00:00:01.42] Testcontainers.MariaDb.Tests: [testcontainers.org] Add file to tar archive: Content length: 52 byte(s), Target file: "/etc/mysql/my.cnf"
After "fix" for consistency:
[xUnit.net 00:00:00.54] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:00.36] Docker container 7294d9e2dbcf created
[xUnit.net 00:00:00.61] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:00.43] Start Docker container 7294d9e2dbcf
[xUnit.net 00:00:00.91] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:00.74] Wait for Docker container 7294d9e2dbcf to complete readiness checks
[xUnit.net 00:00:00.91] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:00.74] Docker container 7294d9e2dbcf ready
[xUnit.net 00:00:00.97] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:00.80] Docker container 87ae49845e7c created
[xUnit.net 00:00:00.97] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:00.80] Docker container d4473f1f2d81 created
[xUnit.net 00:00:00.98] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:00.81] Start Docker container 87ae49845e7c
[xUnit.net 00:00:00.98] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:00.81] Start Docker container d4473f1f2d81
[xUnit.net 00:00:01.42] Testcontainers.MariaDb.Tests: [testcontainers.org 00:00:01.25] Add file to tar archive: Content length: 49 byte(s), Target file: "/etc/mysql/my.cnf"
I think we don't want this double timestamps with a few milliseconds of difference.
This is probably a matter of taste, but simply using fewer lines is not a valid argument. If necessary, you can reduce the XunitLoggerProvider to fewer lines, too (if you count everything that is involved (including removing the statement body, there is barely a difference).
It's not really the number of lines but more the cognitive overhead implementing the ILoggerProvider that nobody is actually using and wrapping a IMessageSink in a ITestOutputHelper implementation.
It aligns with @samtrion suggestion as well, and is something I would like to keep.
I still think that the initial implementation was much simpler to understand and follow. π€·ββοΈ But I don't want to hold this pull request over implementation details.
This is not an issue, as the implementation is not public. We can change this behavior in the future when Xunit.net V3 is officially available. Until then, a lot will probably change anyway.
It's not an issue until some test runner decides to access the Output property. π€ Anyway, I just asked the author of xUnit what's the purpose of this new property: https://github.com/xunit/xunit/discussions/3027
My idea was to support a default constructor that doesn't expect any arguments and then uses a NullLogger instance (empty singleton of the interface). WDYT?
Something like this (and similar for the ContainerTest class)? I think it would make sense (with proper xmldoc of course).
public class ContainerFixture<TBuilderEntity, TContainerEntity>
: ContainerLifetime<TBuilderEntity, TContainerEntity>
where TBuilderEntity : IContainerBuilder<TBuilderEntity, TContainerEntity>, new()
where TContainerEntity : IContainer
{
public ContainerFixture() : base(NullLogger.Instance)
{
}
public ContainerFixture(IMessageSink messageSink) : base(new XunitLoggerProvider(messageSink).CreateLogger("testcontainers.org"))
{
}
}
β 2e79dd7eabb29128a3b2e47ba4d19800361195fe: I overrode the GetHashCode() method.
Good, because ultimately the sink writes to the same file/console output so this way we only have the runtime info logged once.
Also, I saw that you removed the empty line in 313b01e816cc4d9245265632dd41f20a25f2c0cd but there's still one empty line if there's no label at all.
Something I wanted to mention is that in the future, I would like to transition to using a simple annotation that we can add to an IContainer field or property. That annotation would handle creating and starting the test resources. I was thinking of utilizing Fody for this.
I don't understand what you mean, sorry. π Maybe it would be easier for me to get it with an example?
I still think that the initial implementation was much simpler to understand and follow. π€·ββοΈ But I don't want to hold this pull request over implementation details.
Me neither; I found the previous version more difficult to comprehend. If youβd like, we can revert it. In general, I favor consistency. Regarding the Output, no one can access itβitβs private.
Something like this (and similar for the
ContainerTestclass)? I think it would make sense (with proper xmldoc of course).
Yes π.
Also, I saw that you removed the empty line in 313b01e but there's still one empty line if there's no label at all.
Oh, I didnβt notice that. The latest Docker Desktop update always contains the label, I will double-check. Thanks.
I don't understand what you mean, sorry. π Maybe it would be easier for me to get it with an example?
My intention is to support the same or similar behavior as Java does. All you need here is the @Container annotation:
The extension finds all fields that are annotated with
@Containerand calls their container lifecycle methods (methods on theStartableinterface). Containers declared as static fields will be shared between test methods. They will be started only once before any test method is executed and stopped after the last test method has executed. Containers declared as instance fields will be started and stopped for every test method.
It seems that the recent GitHub runners update is causing issues with the tests (https://github.com/actions/runner-images/issues/10649). The MSSQL container crashes on the latest (Ubuntu-22.04) runner version. I noticed similar issues with 24.04 before. Hopefully, simply updating the version will resolve it (https://github.com/testcontainers/testcontainers-dotnet/compare/develop...bugfix/debug-failing-containers-on-ci). We need to remove the Azure SQL Edge module too (it crashes too). I doubt there will be an update since it is already obsolete.