Skip to content

Commit 9f8cf10

Browse files
authored
Use build description to check for unified testing binary (#1004)
We were checking the `mtime` of the `<ProductName>.swift-testing` binary to determine if it was produced by the latest build. The issue with this is that when you run tests over and over without making any code changes the binary is never rewritten because it is already up to date. Instead of checking the `mtime` of the .swift-testing binary in the build folder to determine if it was produced, use the build `description.json` and check the builtTestProducts list to see if the legacy `.swift-testing` binary was produced by the latest build.
1 parent 51d5be1 commit 9f8cf10

File tree

2 files changed

+18
-17
lines changed

2 files changed

+18
-17
lines changed

src/TestExplorer/TestRunner.ts

-4
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,6 @@ export class TestRunner {
430430
fifoPipePath,
431431
this.testKind,
432432
this.testArgs.swiftTestArgs,
433-
new Date(),
434433
true
435434
);
436435

@@ -700,8 +699,6 @@ export class TestRunner {
700699
});
701700
subscriptions.push(buildTask);
702701

703-
const buildStartTime = new Date();
704-
705702
// Perform a build all before the tests are run. We want to avoid the "Debug Anyway" dialog
706703
// since choosing that with no prior build, when debugging swift-testing tests, will cause
707704
// the extension to start listening to the fifo pipe when no results will be produced,
@@ -729,7 +726,6 @@ export class TestRunner {
729726
fifoPipePath,
730727
this.testKind,
731728
this.testArgs.swiftTestArgs,
732-
buildStartTime,
733729
true
734730
);
735731

src/debugger/buildConfig.ts

+18-13
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ export class TestingDebugConfigurationFactory {
3838
fifoPipePath: string,
3939
testKind: TestKind,
4040
testList: string[],
41-
buildStartTime: Date,
4241
expandEnvVariables = false
4342
): Promise<vscode.DebugConfiguration | null> {
4443
return new TestingDebugConfigurationFactory(
@@ -47,7 +46,6 @@ export class TestingDebugConfigurationFactory {
4746
testKind,
4847
TestLibrary.swiftTesting,
4948
testList,
50-
buildStartTime,
5149
expandEnvVariables
5250
).build();
5351
}
@@ -64,7 +62,6 @@ export class TestingDebugConfigurationFactory {
6462
testKind,
6563
TestLibrary.xctest,
6664
testList,
67-
null,
6865
expandEnvVariables
6966
).build();
7067
}
@@ -80,7 +77,6 @@ export class TestingDebugConfigurationFactory {
8077
testKind,
8178
testLibrary,
8279
[],
83-
null,
8480
true
8581
).testExecutableOutputPath();
8682
}
@@ -91,7 +87,6 @@ export class TestingDebugConfigurationFactory {
9187
private testKind: TestKind,
9288
private testLibrary: TestLibrary,
9389
private testList: string[],
94-
private buildStartTime: Date | null,
9590
private expandEnvVariables = false
9691
) {}
9792

@@ -454,23 +449,33 @@ export class TestingDebugConfigurationFactory {
454449
);
455450
}
456451

452+
private buildDescriptionPath(): string {
453+
return path.join(this.buildDirectory, this.artifactFolderForTestKind, "description.json");
454+
}
455+
457456
private async isUnifiedTestingBinary(): Promise<boolean> {
458457
// Toolchains that contain https://github.com/swiftlang/swift-package-manager/commit/844bd137070dcd18d0f46dd95885ef7907ea0697
459458
// no longer produce a .swift-testing binary, instead we want to use `unifiedTestingOutputPath`.
460459
// In order to determine if we're working with a unified binary we need to check if the .swift-testing
461-
// binary _doesn't_ exist, and if it does exist we need to check that it wasn't built by an old toolchain
462-
// and is still in the .build directory. We do this by checking its mtime and seeing if it is after
463-
// the `buildStartTime`.
460+
// binary was produced by the latest build. If it was then we are not using a unified binary.
464461

465462
// TODO: When Swift 6 is released and enough time has passed that we're sure no one is building the .swift-testing
466-
// binary anymore this fs.stat workaround can be removed and `swiftTestingPath` can be returned. The
467-
// `buildStartTime` argument can be removed and the build config generation can be made synchronous again.
463+
// binary anymore this workaround can be removed and `swiftTestingPath` can be returned, and the build config
464+
// generation can be made synchronous again.
468465

469466
try {
470-
const stat = await fs.stat(this.swiftTestingOutputPath());
471-
return !this.buildStartTime || stat.mtime.getTime() < this.buildStartTime.getTime();
467+
const buildDescriptionStr = await fs.readFile(this.buildDescriptionPath(), "utf-8");
468+
const buildDescription = JSON.parse(buildDescriptionStr);
469+
const testProducts = buildDescription.builtTestProducts as { binaryPath: string }[];
470+
if (!testProducts) {
471+
return false;
472+
}
473+
const testBinaryPaths = testProducts.map(({ binaryPath }) => binaryPath);
474+
const swiftTestingBinaryRealPath = await fs.realpath(this.swiftTestingOutputPath());
475+
return !testBinaryPaths.includes(swiftTestingBinaryRealPath);
472476
} catch {
473-
// If the .swift-testing binary doesn't exist then swift-testing tests are in the unified binary.
477+
// If the .swift-testing binary wasn't produced by the latest build then we assume the
478+
// swift-testing tests are in the unified binary.
474479
return true;
475480
}
476481
}

0 commit comments

Comments
 (0)