-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix: Generate tests for executable/tool with --enable-swift-testing
#9032
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
Fix: Generate tests for executable/tool with --enable-swift-testing
#9032
Conversation
Updated InitPackage to handle .executable and .tool types when generating test files with SwiftTesting. Added corresponding tests to verify correct test file content for these package types.
@swift-ci test |
This patch does the same thing: #8995 I was waiting for CI to get back to green before progressing on it, but if you'd like to take over working on this I'm happy to pass it along. |
Thanks, @plemarquand — I didn’t realize #8995 was already in progress. After looking closely, your PR already includes the same functional changes I made (allowing tests to be generated for .executable and .tool) and it adds the swift-testing modernization. Sorry for the duplicate effort. The only delta I see from my side is a small style tweak where I used an exhaustive switch (no I’m happy to help land #8995 and can open a PR against your branch for any cleanups you want, or port your test changes into #9032 if you’d prefer consolidating here. I’ll follow your lead, 👍🏻. |
@JeffESchmitz I say you push forward with this PR. I'll give it a review when you unmark it as draft, and once it's merged I'll extract my work to convert the suite to Swift testing in to its own PR. Probably makes sense to break up that work anyway. Thanks! |
Sources/Workspace/InitPackage.swift
Outdated
@@ -660,10 +660,16 @@ public final class InitPackage { | |||
return | |||
} | |||
|
|||
// Intentionally exhaustive switch without `default` case - forces compilation failure when new |
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.
I don't think the long explanatory comment is necessary - this pattern is used dozens of times throughout any decent-sized Swift codebase, and this would be best mentioned once in a style guide vs applied to every one of those locations.
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.
Thank you for pointing that out, @jakepetroules.
My verbose comment was meant to capture my thought process—I probably should have included it in the PR description or removed it altogether.
I’ll go ahead and remove that inline comment and rely on the style guide to document the exhaustive‑switch pattern. Thanks again for catching that.
Sources/Workspace/InitPackage.swift
Outdated
case .library: | ||
case .empty, .buildToolPlugin, .commandPlugin: | ||
break | ||
case .library, .executable, .tool: // Added .executable and .tool here |
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.
Similarly, this kind of comment isn't adding any useful information, and could become out of date over time.
The exhaustive-switch pattern is self-evident here and common across the codebase. This comment adds noise rather than clarity, so it's better removed and documented via a style guide if needed
@swift-ci please test |
The Windows Swift Test build errors are puzzling. What is
https://ci-external.swift.org/job/pr-swiftpm-windows-self-hosted/1880/console @plemarquand, any ideas? |
@swift-ci test windows |
@JeffESchmitz odd, that doesn't seem related. I've rekicked a build. |
@swift-ci test windows |
Thanks @plemarquand for kicking off the Windows build. This PR should be ready for another review. Does it need anyone else to review and approve before it can be merged to main? |
@JeffESchmitz Looks good to me, thanks for all your help! |
Updated
InitPackage
to handle.executable
and.tool
types when generating test files with SwiftTesting. Added corresponding tests to verify correct test file content for these package types.Motivation:
The
swift package init
command did not generate aTests
directory for executable or tool packages when the--enable-swift-testing
flag was used. This was caused by logic inSources/Workspace/InitPackage.swift
that incorrectly skipped test generation for these package types. This change fixes that behavior, ensuring that tests are created as expected.Modifications:
Sources/Workspace/InitPackage.swift
, thewriteTests()
function was modified to proceed with test generation for.executable
and.tool
package types.writeTestFileStubs()
function was updated to include.executable
and.tool
types, allowing the creation of standard library-style test files for them.Result:
After this change, running
swift package init --type executable --enable-swift-testing
orswift package init --type tool --enable-swift-testing
correctly generates aTests
directory with sample test files, aligning the tool's behavior with its intended functionality.Reproduction and Verification:
Reproducing the Issue (on
main
branch):mkdir -p /tmp/test-repro && cd /tmp/test-repro
swift package init --type executable --enable-swift-testing
.Tests
directory is created by runningls -F
.Verifying the Fix (on this branch):
swift build
find .build -name swift-package
mkdir -p /tmp/test-fix && cd /tmp/test-fix
../.build/arm64-apple-macosx/debug/swift-package init --type executable --enable-swift-testing
(adjust path as needed).Tests
directory is now created by runningls -F
.