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

[tests] Use existing container for tests executed with RemoteExecutor #3444

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

radical
Copy link
Member

@radical radical commented Apr 5, 2024

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 #3221

@radical radical requested review from eerhardt and joperezr April 5, 2024 22:52
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-integrations Issues pertaining to Aspire Integrations packages label Apr 5, 2024
@radical
Copy link
Member Author

radical commented Apr 5, 2024

TODO: try to use the existing container from the main process. thanks for @eerhardt for the suggestion.

@radical radical added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 5, 2024
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
@radical radical force-pushed the sqlserver-tests-timeout branch from 4647257 to 531fe7c Compare April 6, 2024 00:26
@radical radical changed the title [tests] Use 2min timeout for tests run RemoteExecutor+testcontainers [tests] Use existing container for tests executed with RemoteExecutor Apr 6, 2024
@radical radical removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 6, 2024
Copy link
Member

@danmoseley danmoseley left a 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.

@radical
Copy link
Member Author

radical commented Apr 9, 2024

Merging this. @eerhardt if any changes are needed here then I can do them in a follow up.

@radical radical merged commit bdc5ea4 into dotnet:main Apr 9, 2024
8 checks passed
@radical radical deleted the sqlserver-tests-timeout branch April 9, 2024 03:49
Copy link
Member

@eerhardt eerhardt 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 this fix @radical. The changes look good to me.

@radical
Copy link
Member Author

radical commented Apr 9, 2024

/backport to release/8.0

Copy link
Contributor

github-actions bot commented Apr 9, 2024

Started backporting to release/8.0: https://github.com/dotnet/aspire/actions/runs/8620885626

radical added a commit to radical/aspire that referenced this pull request Apr 11, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-integrations Issues pertaining to Aspire Integrations packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tests] TracingEnablesTheRightActivitySource test failing for EntityFramework.SqlServer and SqlClient
3 participants