Skip to content

Skip Interop.FunctionalTests on arm #19648

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

Merged
merged 1 commit into from
Mar 6, 2020

Conversation

analogrelay
Copy link
Contributor

There's no h2spec build for it, so it fails.

We don't run arm64 builds in our PRs which is a) why the original PR to enable these tests didn't catch this and b) why this PR's checks are mostly moot.

If I get a successful build leg (to make sure I didn't mess that up), I may just force-merge this to unblock builds. Does that sound reasonable @dotnet/aspnet-build ?

@analogrelay analogrelay requested a review from a team March 6, 2020 17:46
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.

Doesn’t seem great to not run these tests on PRs.

@dougbu
Copy link
Contributor

dougbu commented Mar 6, 2020

@anurse yes, force pushing seems reasonable if you're comfortable w/ it

@jkotalik we don't have enough ARM64 agents to support running PR validation in that queue. Like macOS agents, these are physical boxes. Unlike macOS agents, there aren't enough of them.

@analogrelay
Copy link
Contributor Author

Doesn’t seem great to not run these tests on PRs.

It's not great, I agree, but it's a constraint we have. The ARM machines are physical and can't just be scaled up and down. We've been asked to keep our ARM usage fairly minimal. I probably should have been more proactive and checked that these tests had a binary dependency, but things like this will get missed. It's a trade-off.

@analogrelay analogrelay changed the base branch from master to release/5.0-preview2 March 6, 2020 18:46
@analogrelay
Copy link
Contributor Author

Stand by. Rebased the PR, now rebasing the branch.

there's no h2spec build for it
@analogrelay
Copy link
Contributor Author

Rebased.

@analogrelay
Copy link
Contributor Author

Override merging now as per justification above.

@analogrelay analogrelay merged commit 8502408 into release/5.0-preview2 Mar 6, 2020
@analogrelay analogrelay deleted the anurse/h2spec-arm branch March 6, 2020 19:19
@Pilchie
Copy link
Member

Pilchie commented Mar 9, 2020

Can we get this change merged into master? Looks like all our open rolling builds are failing in master because of this.

@analogrelay
Copy link
Contributor Author

Sure, I'll open a merge PR to try and pull this format. Should we just merge preview2 and merge to master now?

@analogrelay
Copy link
Contributor Author

We have #19655 (comment) going. I'll take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants