Skip to content

Unskip all BuildPlanSwiftBuildTests for Swift Build #8503

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

pmattos
Copy link
Contributor

@pmattos pmattos commented Apr 15, 2025

Motivation:

Increase test coverage for the new PIF builder (for Swift Build).

Modifications:

Enable/unskip all tests in BuildPlanSwiftBuildTests when using Swift Build.

testTargetsWithPackageAccess

The build error logging, when using --build-system swiftbuild, redirects all build tasks command lines to ObservabilityScope, including those checked by this test. These are then redirected to stderr together with all other logging. I feel this makes sense, providing a nice flexibility when filtering the swift build output, etc.

Conversely, the --build-system native sends the build tasks command lines to stdout instead.

As such, this updates the testTargetsWithPackageAccess to reflect that.

Result:

All BuildPlanSwiftBuildTests are now green on macOS; and just 1 test is skipped on Linux and Windows.

Copy link
Contributor

@bkhouri bkhouri left a comment

Choose a reason for hiding this comment

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

Oddly enough, there is code that should not compile, but the CI jobs all passed. I have a "blocking" comment, which is simply to remove the whitespace that may have been accidentally inserted in the middle of a variable name.

@@ -6996,27 +6998,19 @@ class BuildPlanSwiftBuildTests: BuildPlanTestCase {
try await super.testDuplicateProductNamesWithNonDefaultLibsThrowError()
}

override func testTargetsWithPackageAccess() async throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: I love seeing tests added/re-enabled. party🎉

@pmattos
Copy link
Contributor Author

pmattos commented Apr 15, 2025

Oddly enough, there is code that should not compile, but the CI jobs all passed. I have a "blocking" comment, which is simply to remove the whitespace that may have been accidentally inserted in the middle of a variable name.

Hmm, I don't think we ran CI in this one Sam. BTW, this is still stacked on this other PR #8454.

@bkhouri
Copy link
Contributor

bkhouri commented Apr 15, 2025

@swift-ci test

@pmattos pmattos force-pushed the pmattos/new-pif-builder-in-SwiftBuildSupport branch 5 times, most recently from 04e20e5 to db5124f Compare April 16, 2025 23:06
Base automatically changed from pmattos/new-pif-builder-in-SwiftBuildSupport to main April 17, 2025 08:03
@pmattos pmattos force-pushed the pmattos/unskip-BuildPlanSwiftBuildTests branch from 586a83f to cffbb4a Compare April 24, 2025 18:53
@pmattos
Copy link
Contributor Author

pmattos commented Apr 24, 2025

@swift-ci please test

@pmattos
Copy link
Contributor Author

pmattos commented Apr 24, 2025

@swift-ci test self hosted windows

@pmattos pmattos enabled auto-merge (squash) April 24, 2025 19:09
@pmattos
Copy link
Contributor Author

pmattos commented Apr 24, 2025

@swift-ci test windows

@pmattos
Copy link
Contributor Author

pmattos commented Apr 24, 2025

@swift-ci test linux

@pmattos
Copy link
Contributor Author

pmattos commented Apr 24, 2025

@swift-ci test macOS

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