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

Add gRPC interop tests #17040

Merged
merged 30 commits into from
Mar 21, 2020
Merged

Add gRPC interop tests #17040

merged 30 commits into from
Mar 21, 2020

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Nov 12, 2019

Addresses #11064

PR adds gRPC interop tests.

  • Code is in src/Grpc directory
  • Code is "disconnected" from the rest of build infrastructure through blank directory props/targets. If required the test project could be made to integrate with the rest of the build by moving the blank directory props/targets files to testassets.
  • The interop client and server are completely independent projects, invoked by tests with dotnet run
  • Tests run in a minute on my machine. Could speed them up by publishing client and running exe rather than calling dotnet run 18 times. I have kept things simple for now.
  • Server runs with Kestrel. In the future we'll want to pass args to start server with HttpSys

Is this a good approach?
What changes need to happen elseware to run test project in ProjectTemplates job?

src/Grpc/Directory.Build.props Outdated Show resolved Hide resolved
src/Shared/Process/ProcessEx.cs Outdated Show resolved Hide resolved
src/Grpc/test/InteropTests/Infrastructure/ClientProcess.cs Outdated Show resolved Hide resolved
src/Grpc/test/InteropTests/InteropTestsFixture.cs Outdated Show resolved Hide resolved
@github-actions github-actions bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Nov 12, 2019
@JunTaoLuo JunTaoLuo added area-grpc Includes: GRPC wire-up, templates and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Nov 12, 2019
@github-actions github-actions bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Nov 13, 2019
@JamesNK JamesNK requested a review from Pilchie November 13, 2019 00:06
@JamesNK
Copy link
Member Author

JamesNK commented Nov 13, 2019

@Pilchie Some of the files in this PR are from grpc-dotnet. They all exist under /src/Grpc/test/testassets. None of this code runs outside of testing, and it doesn't ship.

What third-party-code actions are required?

@JamesNK JamesNK removed the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Nov 13, 2019
@JamesNK JamesNK changed the title [WIP] Add gRPC interop tests Add gRPC interop tests Nov 13, 2019
@github-actions github-actions bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Nov 13, 2019
@JamesNK
Copy link
Member Author

JamesNK commented Nov 13, 2019

The build passes. What are the next steps here? Is it a matter of merging the PR then updating project template job tasks to add a task to run test project? Or are there changes required to the other parts of the build to handle these new projects under /src/grpc?

I don't know what changes are required from here on. I need build SMEs to guide me 🙏

@Pilchie
Copy link
Member

Pilchie commented Nov 13, 2019

Ideally, you place the external code in a top level folder that makes it clear that it is external, and a notice added to https://github.com/aspnet/AspNetCore/blob/master/THIRD-PARTY-NOTICES.txt.

}
}

#region TestData
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooo, spicy. I'll let @davidfowl get mad at you.

Copy link
Contributor

@jkotalik jkotalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see any reason to block this.

@JamesNK JamesNK requested a review from a team as a code owner November 15, 2019 19:20
@github-actions github-actions bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Nov 15, 2019
@JamesNK
Copy link
Member Author

JamesNK commented Nov 16, 2019

@JunTaoLuo Some outstanding errors. How do I resolve these?

Build error:

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current\Bin\Roslyn\Microsoft.Managed.Core.targets(102,5): error : SourceRoot items must include at least one top-level (not nested) item when DeterministicSourcePaths is true [F:\workspace.9\_work\1\s\src\Grpc\test\testassets\InteropClient\InteropClient.csproj]
##[error]C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current\Bin\Roslyn\Microsoft.Managed.Core.targets(102,5): error : SourceRoot items must include at least one top-level (not nested) item when DeterministicSourcePaths is true

Test error (one sample, this was thrown for for every test):

System.TimeoutException : The operation at /_/src/Grpc/test/InteropTests/InteropTests.cs:43 timed out after reaching the limit of 30000ms.
   at Microsoft.AspNetCore.Testing.TaskExtensions.TimeoutAfter(Task task, TimeSpan timeout, String filePath, Int32 lineNumber)
   at InteropTests.InteropTests.InteropTestCase(String name) in /_/src/Grpc/test/InteropTests/InteropTests.cs:line 45
--- End of stack trace from previous location ---
Output:
[ERROR] It was not possible to find any compatible framework version
[ERROR] The framework 'Microsoft.NETCore.App', version '5.0.0-alpha1.19521.2' was not found.
[ERROR]   - The following frameworks were found:
[ERROR]       5.0.0-alpha.1.19551.1 at [F:\workspace.9\_work\1\s\.dotnet\shared\Microsoft.NETCore.App]
[ERROR]       5.0.0-alpha.1.19562.8 at [F:\workspace.9\_work\1\s\.dotnet\shared\Microsoft.NETCore.App]
[ERROR] 
[ERROR] You can resolve the problem by installing the specified framework and/or SDK.
[ERROR] 
[ERROR] The specified framework can be found at:
[ERROR]   - https://aka.ms/dotnet-core-applaunch?framework=Microsoft.NETCore.App&framework_version=5.0.0-alpha1.19521.2&arch=x64&rid=win10-x64

The test error happened when calling dotnet run on the interop client/server.

@JunTaoLuo
Copy link
Contributor

I think we might need to add something like this to the interop tests to pick up the latest shared framework: https://github.com/aspnet/AspNetCore/blob/master/src/ProjectTemplates/test/Infrastructure/TemplateTests.props.in.

Essentially we need the workaround here: https://github.com/aspnet/AspNetCore/blob/master/Directory.Build.targets#L115-L122 to be able to run on the latest shared framework. However, I believe the repo has been "isolated" so that fix isn't picked up and the latest shared framework cannot be found.

@JamesNK
Copy link
Member Author

JamesNK commented Nov 25, 2019

@JunTaoLuo Can you take this over - just need to get them passing in CI environment.

@JunTaoLuo JunTaoLuo force-pushed the jamesnk/interoptests branch from 50b4253 to e25ee07 Compare January 30, 2020 22:26
@JunTaoLuo
Copy link
Contributor

@HaoK The tests are passing on Helix! I'm doing one more pass to do a clean up to remove the AzDO modifications.

@HaoK
Copy link
Member

HaoK commented Mar 20, 2020 via email

@JunTaoLuo
Copy link
Contributor

@JamesNK @dotnet/aspnet-build @HaoK I'm planning on merging this once tests pass. Anything else I should address before I do that?

@JamesNK
Copy link
Member Author

JamesNK commented Mar 20, 2020

:shipit:

I would approve but it is my own PR 😄

@JunTaoLuo JunTaoLuo merged commit 8afb78f into master Mar 21, 2020
@JunTaoLuo JunTaoLuo deleted the jamesnk/interoptests branch March 21, 2020 02:32
Pilchie added a commit that referenced this pull request Mar 21, 2020
ghost pushed a commit that referenced this pull request Mar 21, 2020
JunTaoLuo pushed a commit that referenced this pull request Mar 23, 2020
JunTaoLuo pushed a commit that referenced this pull request Mar 23, 2020
* Revert "Revert "Add gRPC interop tests (#17040)" (#20047)"

This reverts commit 236dcd9.

* Fix daily and quarantine Helix runs

* Cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-grpc Includes: GRPC wire-up, templates area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants