ElasticSearchInstrumentationEndToEnd: Increase timeout for RemoteExecutor#5366
Merged
radical merged 4 commits intodotnet:mainfrom Aug 21, 2024
Merged
ElasticSearchInstrumentationEndToEnd: Increase timeout for RemoteExecutor#5366radical merged 4 commits intodotnet:mainfrom
radical merged 4 commits intodotnet:mainfrom
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)
```
ghost
added
the
area-integrations
radical
added
area-engineering-systems
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
sebastienros
approved these changes
Aug 20, 2024
Member
sebastienros
left a comment
sebastienros
left a comment
There was a problem hiding this comment.
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.*" } }, |
Member
There was a problem hiding this comment.
Are we sure this assembly needs retry? Isn't it unit tests only?
Member
Author
There was a problem hiding this comment.
These two can be dropped. We had added retries for old issues which have since been fixed.
4976ee1
Member
There was a problem hiding this comment.
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" } }, |
Member
There was a problem hiding this comment.
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." } } |
Member
There was a problem hiding this comment.
Funny, after my two previous comments, I see a mix, "contains" on "testAssembly". Not wrong though.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.. 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