Skip to content

Add patch skipping TestMachoIssue32233#2202

Merged
dagood merged 1 commit intomicrosoft/mainfrom
dev/dagood/skip-TestMachoIssue32233
Mar 20, 2026
Merged

Add patch skipping TestMachoIssue32233#2202
dagood merged 1 commit intomicrosoft/mainfrom
dev/dagood/skip-TestMachoIssue32233

Conversation

@dagood
Copy link
Member

@dagood dagood commented Mar 20, 2026

@dagood dagood requested a review from a team as a code owner March 20, 2026 15:57
Copilot AI review requested due to automatic review settings March 20, 2026 15:57
@gdams
Copy link
Member

gdams commented Mar 20, 2026

One concern I have with this is that the patches build in order CI job is getting longer and longer here. I wonder if we should add a [test]- prefix to patches that only affect tests and update the CI job to simply apply the patch and skip over the build?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a downstream patch to skip a flaky Go linker DWARF test (TestMachoIssue32233) that is currently blocking PR validation, tracked in #2200.

Changes:

  • Introduces a new patch file that injects an unconditional t.Skip(...) into TestMachoIssue32233.
  • Documents the rationale and tracking issue in the patch header and skip message.
Comments suppressed due to low confidence (1)

patches/0015-Skip-consistently-failing-TestMachoIssue32233.patch:1

  • This unconditionally skips the test on all builders, which can permanently hide real regressions even on platforms where it’s stable. Prefer making the skip conditional to the known failing conditions (e.g., specific GOOS/GOARCH/builder signal), and/or add a clear TODO with criteria for removal so it doesn’t become a long-lived blind spot.
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

@dagood
Copy link
Member Author

dagood commented Mar 20, 2026

One concern I have with this is that the patches build in order CI job is getting longer and longer here. I wonder if we should add a [test]- prefix to patches that only affect tests and update the CI job to simply apply the patch and skip over the build?

Didn't notice the time--still shorter than the AzDO-side builds so far in my experience.

I think we should update the workflow to build them in parallel, so we don't have to worry about number of patch files, and so we also don't lose the sequence-buildable guarantee for some subset of patches.

@dagood dagood merged commit 3cafd1d into microsoft/main Mar 20, 2026
46 checks passed
@dagood dagood deleted the dev/dagood/skip-TestMachoIssue32233 branch March 20, 2026 18:26
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