Reduce flakiness in long-running stateless HTTP tool timeout test#1300
Reduce flakiness in long-running stateless HTTP tool timeout test#1300
Conversation
Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
There was a problem hiding this comment.
This is intended to reduce test failures like the following:
Failed ModelContextProtocol.AspNetCore.Tests.MapMcpStatelessTests.LongRunningToolCall_DoesNotTimeout_WhenNoEventStreamStore [1 s]
Error Message:
System.Threading.Tasks.TaskCanceledException : The request was canceled due to the configured HttpClient.Timeout of 1 seconds elapsing.
https://github.com/modelcontextprotocol/csharp-sdk/actions/runs/22092487205/job/63841819265?pr=1295
We could consider using a timeout of larger than 1 second to reduce the flakiness, but then we'd have to make the LongRunningToolCall longer too. I think this retry logic should reduce the flakiness enough while still keeping the test shortish, but I'm open to ideas for alternative ways to test this that are faster or more reliable.
| } | ||
| catch (TaskCanceledException ex) when ( | ||
| attempt < 2 && | ||
| ex.ToString().Contains("HttpClient.Timeout", StringComparison.Ordinal)) |
There was a problem hiding this comment.
Is this part necessary?
There was a problem hiding this comment.
@copilot Update this to retry for any OperationCanceledException.
There was a problem hiding this comment.
Updated in 4443f79: the retry now catches any OperationCanceledException (bounded by attempt < 2), and I removed the timeout-string check.
| Assert.Equal("Operation completed after 2000ms", content.Text); | ||
| return; | ||
| } | ||
| catch (TaskCanceledException ex) when ( |
There was a problem hiding this comment.
This should be OperationCanceledException
There was a problem hiding this comment.
Updated in 4443f79: switched this to OperationCanceledException.
Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.