Add client conformance tests #1102
Conversation
The OAuth HTTP listener was being stopped after each authorization callback, preventing it from handling multiple OAuth flows such as scope step-up. Remove the finally block that stops the listener so it remains active for the lifetime of the client, allowing it to handle multiple authorization flows including scope step-up and re-authentication scenarios.
mikekistler
force-pushed
the
mdk/conformance-client
branch
from
da7ca68 to
014be50
Compare
tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs
Outdated
Show resolved
Hide resolved
tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs
Outdated
Show resolved
Hide resolved
| var exeSuffix = OperatingSystem.IsWindows() ? ".exe" : ""; | ||
| var conformanceClientPath = Path.GetFullPath($"./ModelContextProtocol.ConformanceClient{exeSuffix}"); | ||
| // Replace AspNetCore.Tests with ConformanceClient in the path | ||
| conformanceClientPath = conformanceClientPath.Replace("AspNetCore.Tests", "ConformanceClient"); |
There was a problem hiding this comment.
It's too bad that we need to use an actual path for this since it's invoked from node rather than directly from C#, but I wonder if we can do more to make sure the client is at this path even if the entire solution hasn't been built. We could use, dotnet publish, but I understand not wanting to slow down the test if it's already built.
I think it would be a good idea to add a project reference in ModelContextProtocol.AspNetCore.Tests.csproj to ModelContextProtocol.ConformanceClient similar to what we do for ModelContextProtocol.ConformanceServer, so the ConformanceClient gets rebuilt automatically when the test project is rebuilt even if you don't rebuild the entire solution.
<ProjectReference Include="..\ModelContextProtocol.ConformanceClient\ModelContextProtocol.ConformanceClient.csproj" />There was a problem hiding this comment.
We had a project reference in ModelContextProtocol.AspNetCore.Tests.csproj previously but it was not helpful, because it trimmed dlls required by the client but not visible to MSBuild because the client was never invoked directly. I wasted a day debugging the obscure errors that resulted. I don't think we should go down that path again.
There was a problem hiding this comment.
I was about to argue against this, but I see you did include the ProjectReference in f047c47#diff-c331a50438b507f3674e24b92de8e2e4d6c4ccc9c4f8347e4f86907b8515ce04
Since we're not publishing, I believe the trimming only happens at the assembly level, so we just need to add a dummy reference to any class in the ConformanceClient if we want to completely avoid the possiblity of trimming, but I think this is fine as long as it works which it appears to.
Co-authored-by: Stephen Halter <halter73@gmail.com>
…nt conformance setup
Motivation and Context
This PR adds the initial set of client conformance tests which are required for Tier 1 SDKs. All tests are passing.
How Has This Been Tested?
All tests build and pass.
Breaking Changes
None.
Types of changes
Checklist
Additional context