Skip to content

Tests: Minor tweaks in Swift Build related tests #8473

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 3 commits into
base: main
Choose a base branch
from

Conversation

pmattos
Copy link
Contributor

@pmattos pmattos commented Apr 9, 2025

Motivation:

This includes some (minor) tweaks I did in Swift Build related tests while working on the much bigger #8454.

Modifications:

Minor improvements to all BuildSystemProviderTestCase subclasses. For instance, these tests:

Plus some changes to sibling test classes too.

Related to #8454.

}

override func testDuplicateProductNamesWithNonDefaultLibsThrowError() async throws {
try await super.testDuplicateProductNamesWithNonDefaultLibsThrowError()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed all overrides that just call back the corresponding super class method.

Copy link
Contributor

Choose a reason for hiding this comment

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

chore (blocking): Please re-add the override as it was needed to ensure the XCTests on linux instantiate all the tests in the subclass.

@cmcgee1024 can provide additional information as to why it was originally added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, wasn't aware of that Linux hack, sorry :-(

So to be clear: all Linux test suites need to override all test cases... and then call super? If so, are we sure we actually did this hack everywhere? For instance BuildPlanSwiftBuildTests has dozen of tests but only two are overridden like this.

CC @cmcgee1024

Copy link
Contributor

Choose a reason for hiding this comment

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

from my understanding, only 1 test needs to be overridden to have XCTest "populate" all the tests in the subclass.


override func testDuplicateProductNamesWithNonDefaultLibsThrowError() async throws {
try await super.testDuplicateProductNamesWithNonDefaultLibsThrowError()
final class BuildPlanNativeTests: BuildPlanTestCase {
Copy link
Contributor Author

@pmattos pmattos Apr 9, 2025

Choose a reason for hiding this comment

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

Marked all these subclasses final to make it clear we don't further extend such tests anywhere else.

@pmattos
Copy link
Contributor Author

pmattos commented Apr 9, 2025

@swift-ci test

@pmattos
Copy link
Contributor Author

pmattos commented Apr 12, 2025

@swift-ci test windows

@pmattos pmattos force-pushed the pmattos/swift-build-tests-tweaks branch from c6ec569 to 7424110 Compare April 12, 2025 02:08
@pmattos
Copy link
Contributor Author

pmattos commented Apr 12, 2025

@swift-ci test

@pmattos
Copy link
Contributor Author

pmattos commented Apr 12, 2025

@swift-ci test windows

@pmattos pmattos force-pushed the pmattos/swift-build-tests-tweaks branch from 7424110 to 32944d8 Compare April 12, 2025 07:55
@pmattos
Copy link
Contributor Author

pmattos commented Apr 12, 2025

@swift-ci test

@pmattos
Copy link
Contributor Author

pmattos commented Apr 12, 2025

@swift-ci test

@pmattos
Copy link
Contributor Author

pmattos commented Apr 13, 2025

@swift-ci test macOS

1 similar comment
@pmattos
Copy link
Contributor Author

pmattos commented Apr 13, 2025

@swift-ci test macOS

}

override func testDuplicateProductNamesWithNonDefaultLibsThrowError() async throws {
try await super.testDuplicateProductNamesWithNonDefaultLibsThrowError()
Copy link
Contributor

Choose a reason for hiding this comment

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

chore (blocking): Please re-add the override as it was needed to ensure the XCTests on linux instantiate all the tests in the subclass.

@cmcgee1024 can provide additional information as to why it was originally added.

return .native
}

override func skipIfApiDigesterUnsupportedOrUnset() throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

chore (blocking): please re-add this override.

return .swiftbuild
}

override func skipIfApiDigesterUnsupportedOrUnset() throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

chore (blocking): please re-add this override.

return .native
}

override func testUsage() 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.

chore (blocking): please re-add this override.

return .xcode
}

override func testUsage() 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.

chore (blocking): please re-add this override.

return .native
}

override func testUsage() 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.

chore (blocking): please re-add this override.

return .swiftbuild
}

override func testUsage() 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.

chore (blocking): please re-add this override.

return .native
}

override func testUsage() 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.

chore (blocking): please re-add this override.

return .swiftbuild
}

override func testUsage() 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.

chore (blocking): please re-add this override.

return .swiftbuild
}

override func testNoParameters() 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.

chore (blocking): please re-add this override to ensure the tests gets "instantiated" on linux when all other tests are "fixed".

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.

3 participants