Skip to content

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Dec 21, 2022

Addresses #41716

Grpc.Tools (aka protoc.exe) doesn't support Windows ARM 64. Don't run gRPC templates on windows.11.arm64.open.

Also ensures gRPC template tests run on macOS.

@JamesNK JamesNK added the area-grpc Includes: GRPC wire-up, templates label Dec 21, 2022
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Dec 21, 2022
@JamesNK JamesNK force-pushed the jamesnk/grpctemplatetests-arm64 branch from 8dcad96 to c231d26 Compare December 23, 2022 01:35
@JamesNK JamesNK force-pushed the jamesnk/grpctemplatetests-arm64 branch from c231d26 to 8a80a1b Compare December 28, 2022 07:54
Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

This worked 😀

I can sign off from an infrastructure perspective.

I suggest

  1. Update the final commit description to make it clear OSX testing is reenabled in this PR.
  2. Perhaps getting a review from someone more familiar with gRPC and related tests.

@@ -35,7 +35,7 @@ public ITestOutputHelper Output
}

[ConditionalTheory]
[SkipOnHelix("Not supported queues", Queues = "All.OSX;" + HelixConstants.Windows10Arm64 + HelixConstants.DebianArm64)]
[SkipOnHelix("Not supported queues", Queues = "windows.11.arm64.open;" + HelixConstants.Windows10Arm64 + HelixConstants.DebianArm64)]
Copy link
Member

Choose a reason for hiding this comment

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

Can we add this value to HelixConstants?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably. But first, I'd like to ensure this change fixes the problem. The problem is that the Helix queue only runs on full CI (i.e. not PR CI). Need to merge to check.

Copy link
Member

Choose a reason for hiding this comment

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

There are ways to run it without merging PRs, oh well

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok!

#45885 😄

@JamesNK JamesNK merged commit 0d8af90 into main Jan 4, 2023
@JamesNK JamesNK deleted the jamesnk/grpctemplatetests-arm64 branch January 4, 2023 23:28
@ghost ghost added this to the 8.0-preview1 milestone Jan 4, 2023
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-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants