-
Notifications
You must be signed in to change notification settings - Fork 715
ElasticSearchInstrumentationEndToEnd: Increase timeout for RemoteExecutor #5366
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
Conversation
…tuor
.. from 60s to 120s. This has been failing frequently on CI with:
```
Aspire.Elastic.Clients.Elasticsearch.Tests.AspireElasticClientExtensionsTest.ElasticsearchInstrumentationEndToEnd [FAIL]
Microsoft.DotNet.RemoteExecutor.RemoteExecutionException : Half-way through waiting for remote process.
Timed out at 8/20/2024 6:22:40 PM after 60000ms waiting for remote process.
Process ID: 29680
Handle: 11984
Name: dotnet
MainModule: /datadisks/disk1/work/B7250A1D/p/dotnet-cli/dotnet
StartTime: 8/20/2024 6:21:39 PM
TotalProcessorTime: 00:00:00.7300000
Stack Trace:
/_/src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeHandle.cs(225,0): at Microsoft.DotNet.RemoteExecutor.RemoteInvokeHandle.Dispose(Boolean disposing)
/_/src/Microsoft.DotNet.RemoteExecutor/src/RemoteInvokeHandle.cs(55,0): at Microsoft.DotNet.RemoteExecutor.RemoteInvokeHandle.Dispose()
/_/tests/Aspire.Elastic.Clients.Elasticsearch.Tests/AspireElasticClientExtensionsTest.cs(145,0): at Aspire.Elastic.Clients.Elasticsearch.Tests.AspireElasticClientExtensionsTest.ElasticsearchInstrumentationEndToEnd()
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
```
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Just made comments unrelated to the goal of this PR.
eng/test-configuration.json
Outdated
| { "testAssembly": { "regex": "Aspire.EndToEnd.*" }, "failureMessage": { "regex": "App run failed" } }, | ||
| { "testAssembly": { "regex": "Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.*" } }, | ||
| { "testAssembly": { "regex": "Aspire.Microsoft.EntityFrameworkCore.SqlServer.*" } } | ||
| { "testAssembly": { "regex": "Aspire.Microsoft.EntityFrameworkCore.SqlServer.*" } }, |
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.
Are we sure this assembly needs retry? Isn't it unit tests only?
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.
These two can be dropped. We had added retries for old issues which have since been fixed.
4976ee1
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.
My point is more about:
- When to use
testAssemblyvstestName - That
"Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.*"is not a regex, or at least it's doing what we assume it tries to do by chance (the dot can be anything and the*appears right after a dot, double points for luck here).
| { "testAssembly": { "regex": "Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.*" } }, | ||
| { "testAssembly": { "regex": "Aspire.Microsoft.EntityFrameworkCore.SqlServer.*" } } | ||
| { "testAssembly": { "regex": "Aspire.Microsoft.EntityFrameworkCore.SqlServer.*" } }, | ||
| { "testName": { "contains": "Aspire.Elastic.Clients.Elasticsearch.Tests.ConformanceTests.HealthCheckReportsExpectedStatus" } }, |
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.
What I see is that the "testName/contains" rule might be more appropriate than
"testAssembly/regex" in the preceeding cases, since the regex is not really a regex, and contains could be used with the assembly name as the prefix instead.
eng/test-configuration.json
Outdated
| { "testAssembly": { "regex": "Aspire.Microsoft.EntityFrameworkCore.SqlServer.*" } } | ||
| { "testAssembly": { "regex": "Aspire.Microsoft.EntityFrameworkCore.SqlServer.*" } }, | ||
| { "testName": { "contains": "Aspire.Elastic.Clients.Elasticsearch.Tests.ConformanceTests.HealthCheckReportsExpectedStatus" } }, | ||
| { "testAssembly": { "contains": "Aspire.Elastic.Clients.Elasticsearch.Test" }, "failureMessage": { "contains": "RemoteExecutionException : Half-way through waiting for remote process." } } |
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.
Funny, after my two previous comments, I see a mix, "contains" on "testAssembly". Not wrong though.
.. from 60s to 120s. This has been failing frequently on CI with:
EndToEndtests failing to start uptestprojectwithdotnet run#2850, but the issue has since been fixed.Microsoft Reviewers: Open in CodeFlow