Skip to content

Narrow testSubprocessPlatformOptionsSupplementaryGroups() on Darwin#345

Open
broken-circle wants to merge 1 commit into
swiftlang:mainfrom
broken-circle:macOS-supplementary-groups-test
Open

Narrow testSubprocessPlatformOptionsSupplementaryGroups() on Darwin#345
broken-circle wants to merge 1 commit into
swiftlang:mainfrom
broken-circle:macOS-supplementary-groups-test

Conversation

@broken-circle

Copy link
Copy Markdown
Member

testSubprocessPlatformOptionsSupplementaryGroups() fails on Darwin:

% sudo xcrun swift test --scratch-path .build-root --filter testSubprocessPlatformOptionsSupplementaryGroups
...
Test testSubprocessPlatformOptionsSupplementaryGroups() recorded an issue at UnixTests.swift:115:9:
Expectation failed: (actualGroups → []) == (expectedGroups → [1079, 1526, 1744, 1210, 1055, 1878, 1538, 1130, 1836])

id -G on Darwin doesn't report the groups, but they are applied: the prefork child aborts the spawn if setgroups(2) fails, so a successful spawn confirms the call returned 0 and the credential was set; only Darwin's id -G read-back omits it. This PR keeps the membership assertion where id -G is faithful and asserts only spawn success on Darwin.

@broken-circle broken-circle requested a review from iCharlesHu as a code owner July 4, 2026 16:55
@broken-circle

broken-circle commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

The Unix PlatformOptions tests that require root run on Linux and FreeBSD, but are skipped on macOS. I verified this fix locally, but this change is currently a no-op on CI, since macOS skips the test and the other platforms take the unchanged assertion.

These tests were originally introduced in iCharlesHu/Subprocess/main@0600d01 with the same skip-without-sudo condition. I think these were meant to run in CI, and I can see a few ways to make that happen:

  1. A bespoke macOS sudo job in this repository's pull_request.yml, filtered to a new root suite (SubprocessUnixRootTests).
  2. A macOS "run as root" option added to the shared swift_package_test.yml in swiftlang/github-workflows, keeping the runner-specific details centralized there rather than here.
  3. Leaving them skipped on macOS (current behavior).

I was leaning toward option 1. I'd open a draft PR initially to validate that the macOS runners allow passwordless sudo. There's also runner cleanup to handle (root-owned build artifacts on a persistent runner).

Which approach would you most prefer? I don't think we should leave them skipped. I have the option 1 draft PR mostly ready, or I can get started with option 2, if it's likely such an addition would be accepted there.

@jakepetroules

Copy link
Copy Markdown
Contributor

1 seems like a good idea if it's possible

@broken-circle broken-circle force-pushed the macOS-supplementary-groups-test branch 2 times, most recently from 7dc99d9 to 3bf4f9e Compare July 9, 2026 00:51
@broken-circle

broken-circle commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

I tried in #351, but passwordless sudo isn't available on the self-hosted macOS runners. I verified on my fork that it's available if we use macos-26. Are you okay with one hosted-macOS exception to gain macOS coverage on these tests?

`id -G` on Darwin doesn't report the groups, but they are applied: the
prefork child aborts the spawn if `setgroups(2)` fails, so a successful
spawn confirms the call returned `0` and the credential was set; only
Darwin's `id -G` read-back omits it. Keep the membership assertion where
`id -G` is faithful and asserts only spawn success on Darwin.
@broken-circle broken-circle force-pushed the macOS-supplementary-groups-test branch from 3bf4f9e to b3bbfaa Compare July 10, 2026 01:05
@jakepetroules

Copy link
Copy Markdown
Contributor

Hmm, I was under the impression that only self-hosted macOS runners were available for the swiftlang org, but @shahmishal could confirm the details there for sure.

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.

2 participants