-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Add gRPC interop tests #17040
Conversation
@Pilchie Some of the files in this PR are from grpc-dotnet. They all exist under What third-party-code actions are required? |
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 🙏 |
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 |
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.
Ooo, spicy. I'll let @davidfowl get mad at you.
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.
Don't see any reason to block this.
@JunTaoLuo Some outstanding errors. How do I resolve these? Build error:
Test error (one sample, this was thrown for for every test):
The test error happened when calling |
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. |
@JunTaoLuo Can you take this over - just need to get them passing in CI environment. |
fc28db1
to
94a9520
Compare
50b4253
to
e25ee07
Compare
@HaoK The tests are passing on Helix! I'm doing one more pass to do a clean up to remove the AzDO modifications. |
Yup that should do it
… On Mar 20, 2020, at 2:12 PM, John Luo ***@***.***> wrote:
@JunTaoLuo commented on this pull request.
In src/Grpc/test/InteropTests/Helpers/WebsiteProcess.cs:
> @@ -78,7 +78,17 @@ private void Process_OutputDataReceived(object sender, DataReceivedEventArgs e)
public void Dispose()
{
- File.WriteAllText(_serverLogPath ?? Path.Combine(Directory.GetCurrentDirectory(), "InteropServer.log"), _consoleOut.ToString());
+ if (!string.IsNullOrEmpty(_serverLogPath))
so CurrentDirectory/artifacts/logs?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
I would approve but it is my own PR 😄 |
Addresses #11064
PR adds gRPC interop tests.
Is this a good approach?
What changes need to happen elseware to run test project in ProjectTemplates job?