-
Notifications
You must be signed in to change notification settings - Fork 543
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
[tests] Use existing container for tests executed with RemoteExecutor #3444
Conversation
TODO: try to use the existing container from the main process. thanks for @eerhardt for the suggestion. |
The tracing tests in `ConformanceTests` for some components are run with `RemoteExecutor.Invoke(() => RunWithFixtureAsync(obj => obj.ActivitySourceTest(key: null))).Dispose()` .. where `RunWithFixtureAsync` starts up a `testcontainer` in the remote process, and then runs the test. But unlike other instances of the tracing tests, here starting up the container can take ~1min causing the `RemoteExecutor` to fail after the default 1 min timeout. Instead pass the connection string from the fixture to the `RemoteExecutor`, so a new container doesn't need to be spun up. Example log from the remote execution: ``` [testcontainers.org 00:00:02.97] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [testcontainers.org 00:00:12.09] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [testcontainers.org 00:00:21.15] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [testcontainers.org 00:00:30.22] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [testcontainers.org 00:00:39.29] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [testcontainers.org 00:00:48.35] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [testcontainers.org 00:00:57.42] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [testcontainers.org 00:00:58.52] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [testcontainers.org 00:00:59.60] Execute "/opt/mssql-tools/bin/sqlcmd -Q SELECT 1;" at Docker container d69a9831c425 [xUnit.net 00:02:38.60] Aspire.Microsoft.EntityFrameworkCore.SqlServer.Tests.ConformanceTests_TypeSpecificConfig.TracingEnablesTheRightActivitySource [FAIL] [xUnit.net 00:02:38.60] Microsoft.DotNet.RemoteExecutor.RemoteExecutionException : Half-way through waiting for remote process. [xUnit.net 00:02:38.60] Timed out at 3/27/2024 5:35:56 PM after 60000ms waiting for remote process. [xUnit.net 00:02:38.60] Process ID: 36471 [xUnit.net 00:02:38.60] Handle: 5036 [xUnit.net 00:02:38.60] Name: dotnet [xUnit.net 00:02:38.60] MainModule: /datadisks/disk1/work/B5B5097E/p/dotnet-cli/dotnet [xUnit.net 00:02:38.60] StartTime: 3/27/2024 5:34:56 PM [xUnit.net 00:02:38.60] TotalProcessorTime: 00:00:02.1500000 [xUnit.net 00:02:38.60] [xUnit.net 00:02:38.60] Stack Trace: [xUnit.net 00:02:38.60] /_/src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeHandle.cs(225,0): at Microsoft.DotNet.RemoteExecutor.RemoteInvokeHandle.Dispose(Boolean disposing) [xUnit.net 00:02:38.60] /_/src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeHandle.cs(55,0): at Microsoft.DotNet.RemoteExecutor.RemoteInvokeHandle.Dispose() [xUnit.net 00:02:38.60] /_/tests/Aspire.Microsoft.EntityFrameworkCore.SqlServer.Tests/ConformanceTests.cs(116,0): at Aspire.Microsoft.EntityFrameworkCore.SqlServer.Tests.ConformanceTests.TracingEnabl> [xUnit.net 00:02:38.60] at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor) [xUnit.net 00:02:38.60] at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr) ``` Fixes dotnet#3221
4647257
to
531fe7c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable. This means that one test failure could cascade to another, presumably -- or one test could accidentally depend on another running first.
Merging this. @eerhardt if any changes are needed here then I can do them in a follow up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this fix @radical. The changes look good to me.
/backport to release/8.0 |
Started backporting to release/8.0: https://github.com/dotnet/aspire/actions/runs/8620885626 |
Fixed by dotnet#3444 .
The tracing tests in
ConformanceTests
for some components are run withRemoteExecutor.Invoke(() => RunWithFixtureAsync(obj => obj.ActivitySourceTest(key: null))).Dispose()
.. where
RunWithFixtureAsync
starts up atestcontainer
in the remoteprocess, and then runs the test. But unlike other instances of the
tracing tests, here starting up the container can take ~1min causing the
RemoteExecutor
to fail after the default 1 min timeout.
Instead pass the connection string from the fixture to the
RemoteExecutor
, so a new container doesn't need to be spun up.Example log from the remote execution:
Fixes #3221