Tests: Convert Run Command Tests to Swift Testing #8771
Conversation
bkhouri
force-pushed
the
t/convertBuildComment/convert_swift-run_test-commands-to-swift-testing
branch
from
ca043ed to
928132e
Compare
|
@swift-ci test |
bkhouri
force-pushed
the
t/convertBuildComment/convert_swift-run_test-commands-to-swift-testing
branch
3 times, most recently
from
cdf999c to
e928ec6
Compare
|
@swift-ci test |
|
@swift-ci test windows |
bkhouri
force-pushed
the
t/convertBuildComment/convert_swift-run_test-commands-to-swift-testing
branch
from
e928ec6 to
7572ef6
Compare
|
@swift-ci test |
|
@swift-ci test windows |
bkhouri
force-pushed
the
t/convertBuildComment/convert_swift-run_test-commands-to-swift-testing
branch
from
7572ef6 to
90ef1a8
Compare
|
@swift-ci test |
|
@swift-ci test windows |
Convert the Run Command Tests to Swift Testing. Make use of parameterized tests to reduce duplication and use withKnownIssue to get signal when tests have been fixed.
bkhouri
force-pushed
the
t/convertBuildComment/convert_swift-run_test-commands-to-swift-testing
branch
from
90ef1a8 to
e4d7187
Compare
|
@swift-ci test |
|
@swift-ci test windows |
1 similar comment
|
@swift-ci test windows |
bkhouri
requested review from
jakepetroules,
dschaefer2,
shawnhyam,
bripeticca,
plemarquand,
owenv,
cmcgee1024 and
daveyc123
as code owners
|
@swift-ci test windows |
| @@ -123,7 +123,7 @@ public let swiftTest: AbsolutePath = swiftpmBinaryDirectory.appending(component: | |||
|
|
|||
| public let swiftRun: AbsolutePath = swiftpmBinaryDirectory.appending(component: "swift-run") | |||
|
|
|||
| public let isSelfHosted: Bool = { | |||
| public let runningInSelfHostedPipeline: Bool = { | |||
There was a problem hiding this comment.
thought: By detecting the specific CI job that these tests are running, it could be undermining real understanding of the reasons why tests are failing, and having specific detection mechanisms for those cases. Otherwise, this seems to be no better than just marking isIntermittent on each withKnownIssue block. Also, it opens things up to intermittent failures if the job environment changes, such as an OS upgrade, or any other effect that makes these tests fail.
| @@ -63,6 +63,14 @@ public let isRealSigningIdentitTestDefined = { | |||
| #endif | |||
| }() | |||
|
|
|||
| public let duplicateSymbolRegex: Regex<AnyRegexOutput>? = { | |||
There was a problem hiding this comment.
suggestion (non-blocking): Make this regex non-optional, and in the computed property allow it to throw. Then if this regex ever fails to compile for any reason, the test will throw an error with the details of why the regex failed to compile.
| @Suite( | ||
| .serialized, // to limit the number of swift executable running. | ||
| .tags( | ||
| Tag.TestSize.large, |
There was a problem hiding this comment.
praise: This is useful metadata to help understand the nature of the test suite.
| .bug("https://github.com/swiftlang/swift-package-manager/issues/8602"), | ||
| arguments: SupportedBuildSystemOnPlatform, | ||
| ) | ||
| func productArgumentPassing( |
There was a problem hiding this comment.
question: Why drop the word "unknown" from the name of the test function? Shouldn't this be testUnknownProductAndArgumentPassing?
| XCTAssertMatch(error.stderr, .contains("error: no executable product named 'unknown'")) | ||
| // swift-build-tool output should go to stderr. | ||
| withKnownIssue { | ||
| #expect(stderr.contains("Compiling")) |
There was a problem hiding this comment.
question: There is no GitHub issue raised related to the stderr containing this specific output. Can you add one and add a .bug() reference to it?
| } | ||
| } when: { | ||
| ProcessInfo.hostOperatingSystem == .windows |
There was a problem hiding this comment.
question: where is the .bug() reference to the GitHub issue for this known issue with Windows?
| func swiftRunSIGINT( | ||
| buildSystem: BuildSystemProvider.Kind, | ||
| ) throws { | ||
| try withKnownIssue("Seems to be flaky in CI", isIntermittent: true) { |
There was a problem hiding this comment.
praise: This looks like a good way to mark a known intermittent issue. Is there a GitHub issue for this flaky test?
Convert the Run Command Tests to Swift Testing.
Make use of parameterized tests to reduce duplication and use
withKnownIssueto get signal when tests have been fixed.Depends on: #8714