Skip to content
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

Avoid blocking folder addition on package loading #1422

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/FolderContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,12 @@ export class FolderContext implements vscode.Disposable {
): Promise<FolderContext> {
const statusItemText = `Loading Package (${FolderContext.uriName(folder)})`;
workspaceContext.statusItem.start(statusItemText);

const { linuxMain, swiftPackage } =
await workspaceContext.statusItem.showStatusWhileRunning(statusItemText, async () => {
const linuxMain = await LinuxMain.create(folder);
const swiftPackage = await SwiftPackage.create(folder, workspaceContext.toolchain);
return { linuxMain, swiftPackage };
});

workspaceContext.statusItem.end(statusItemText);

const folderContext = new FolderContext(
Expand All @@ -87,7 +85,7 @@ export class FolderContext implements vscode.Disposable {
workspaceContext
);

const error = swiftPackage.error;
const error = await swiftPackage.error;
if (error) {
vscode.window.showErrorMessage(
`Failed to load ${folderContext.name}/Package.swift: ${error.message}`
Expand All @@ -97,7 +95,6 @@ export class FolderContext implements vscode.Disposable {
folderContext.name
);
}

return folderContext;
}

Expand Down Expand Up @@ -186,11 +183,11 @@ export class FolderContext implements vscode.Disposable {
* @param uri URI to find target for
* @returns Target
*/
getTestTarget(uri: vscode.Uri, type?: TargetType): Target | undefined {
async getTestTarget(uri: vscode.Uri, type?: TargetType): Promise<Target | undefined> {
if (!isPathInsidePath(uri.fsPath, this.folder.fsPath)) {
return undefined;
}
const testTargets = this.swiftPackage.getTargets(type);
const testTargets = await this.swiftPackage.getTargets(type);
const target = testTargets.find(element => {
const relativeUri = path.relative(
path.join(this.folder.fsPath, element.path),
Expand Down
116 changes: 77 additions & 39 deletions src/SwiftPackage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { SwiftToolchain } from "./toolchain/toolchain";
import { BuildFlags } from "./toolchain/BuildFlags";

/** Swift Package Manager contents */
export interface PackageContents {
interface PackageContents {
name: string;
products: Product[];
dependencies: Dependency[];
Expand Down Expand Up @@ -184,8 +184,10 @@ function isError(state: SwiftPackageState): state is Error {
/**
* Class holding Swift Package Manager Package
*/
export class SwiftPackage implements PackageContents {
export class SwiftPackage {
public plugins: PackagePlugin[] = [];
private _contents: SwiftPackageState | undefined;

/**
* SwiftPackage Constructor
* @param folder folder package is in
Expand All @@ -194,7 +196,7 @@ export class SwiftPackage implements PackageContents {
*/
private constructor(
readonly folder: vscode.Uri,
private contents: SwiftPackageState,
private contentsPromise: Promise<SwiftPackageState>,
public resolved: PackageResolved | undefined,
private workspaceState: WorkspaceState | undefined
) {}
Expand All @@ -208,10 +210,34 @@ export class SwiftPackage implements PackageContents {
folder: vscode.Uri,
toolchain: SwiftToolchain
): Promise<SwiftPackage> {
const contents = await SwiftPackage.loadPackage(folder, toolchain);
const resolved = await SwiftPackage.loadPackageResolved(folder);
const workspaceState = await SwiftPackage.loadWorkspaceState(folder);
return new SwiftPackage(folder, contents, resolved, workspaceState);
const [resolved, workspaceState] = await Promise.all([
SwiftPackage.loadPackageResolved(folder),
SwiftPackage.loadWorkspaceState(folder),
]);
return new SwiftPackage(
folder,
SwiftPackage.loadPackage(folder, toolchain),
resolved,
workspaceState
);
}

/**
* Returns the package state once it has loaded.
* A snapshot of the state is stored in `_contents` after initial resolution.
*/
private get contents(): Promise<SwiftPackageState> {
return this.contentsPromise.then(contents => {
// If `reload` is called immediately its possible for it to resolve
// before the initial contentsPromise resolution. In that case return
// the newer loaded `_contents`.
if (this._contents === undefined) {
this._contents = contents;
return contents;
} else {
return this._contents;
}
});
}

/**
Expand Down Expand Up @@ -326,7 +352,9 @@ export class SwiftPackage implements PackageContents {

/** Reload swift package */
public async reload(toolchain: SwiftToolchain) {
this.contents = await SwiftPackage.loadPackage(this.folder, toolchain);
const loadedContents = await SwiftPackage.loadPackage(this.folder, toolchain);
this._contents = loadedContents;
this.contentsPromise = Promise.resolve(loadedContents);
}

/** Reload Package.resolved file */
Expand All @@ -343,31 +371,26 @@ export class SwiftPackage implements PackageContents {
}

/** Return if has valid contents */
public get isValid(): boolean {
return isPackage(this.contents);
public get isValid(): Promise<boolean> {
return this.contents.then(contents => isPackage(contents));
}

/** Load error */
public get error(): Error | undefined {
if (isError(this.contents)) {
return this.contents;
} else {
return undefined;
}
public get error(): Promise<Error | undefined> {
return this.contents.then(contents => (isError(contents) ? contents : undefined));
}

/** Did we find a Package.swift */
public get foundPackage(): boolean {
return this.contents !== undefined;
public get foundPackage(): Promise<boolean> {
return this.contents.then(contents => contents !== undefined);
}

public rootDependencies(): ResolvedDependency[] {
public get rootDependencies(): Promise<ResolvedDependency[]> {
// Correlate the root dependencies found in the Package.swift with their
// checked out versions in the workspace-state.json.
const result = this.dependencies.map(dependency =>
this.resolveDependencyAgainstWorkspaceState(dependency)
return this.dependencies.then(dependencies =>
dependencies.map(dependency => this.resolveDependencyAgainstWorkspaceState(dependency))
);
return result;
}

private resolveDependencyAgainstWorkspaceState(dependency: Dependency): ResolvedDependency {
Expand Down Expand Up @@ -443,55 +466,70 @@ export class SwiftPackage implements PackageContents {
}
}

/** name of Swift Package */
get name(): string {
return (this.contents as PackageContents)?.name ?? "";
/** getName of Swift Package */
get name(): Promise<string> {
return this.contents.then(contents => (contents as PackageContents)?.name ?? "");
}

/** array of products in Swift Package */
get products(): Product[] {
return (this.contents as PackageContents)?.products ?? [];
private get products(): Promise<Product[]> {
return this.contents.then(contents => (contents as PackageContents)?.products ?? []);
}

/** array of dependencies in Swift Package */
get dependencies(): Dependency[] {
return (this.contents as PackageContents)?.dependencies ?? [];
get dependencies(): Promise<Dependency[]> {
return this.contents.then(contents => (contents as PackageContents)?.dependencies ?? []);
}

/** array of targets in Swift Package */
get targets(): Target[] {
return (this.contents as PackageContents)?.targets ?? [];
get targets(): Promise<Target[]> {
return this.contents.then(contents => (contents as PackageContents)?.targets ?? []);
}

/** array of executable products in Swift Package */
get executableProducts(): Product[] {
return this.products.filter(product => product.type.executable !== undefined);
get executableProducts(): Promise<Product[]> {
return this.products.then(products =>
products.filter(product => product.type.executable !== undefined)
);
}

/** array of library products in Swift Package */
get libraryProducts(): Product[] {
return this.products.filter(product => product.type.library !== undefined);
get libraryProducts(): Promise<Product[]> {
return this.products.then(products =>
products.filter(product => product.type.library !== undefined)
);
}

/**
* Array of targets in Swift Package. The targets may not be loaded yet.
* It is preferable to use the `targets` property that returns a promise that
* returns the targets when they're guarenteed to be resolved.
**/
get currentTargets(): Target[] {
return (this._contents as unknown as { targets: Target[] })?.targets ?? [];
}

/**
* Return array of targets of a certain type
* @param type Type of target
* @returns Array of targets
*/
getTargets(type?: TargetType): Target[] {
async getTargets(type?: TargetType): Promise<Target[]> {
if (type === undefined) {
return this.targets;
} else {
return this.targets.filter(target => target.type === type);
return this.targets.then(targets => targets.filter(target => target.type === type));
}
}

/**
* Get target for file
*/
getTarget(file: string): Target | undefined {
async getTarget(file: string): Promise<Target | undefined> {
const filePath = path.relative(this.folder.fsPath, file);
return this.targets.find(target => isPathInsidePath(filePath, target.path));
return this.targets.then(targets =>
targets.find(target => isPathInsidePath(filePath, target.path))
);
}

private static trimStdout(stdout: string): string {
Expand Down
37 changes: 20 additions & 17 deletions src/TestExplorer/LSPTestDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
TextDocumentTestsRequest,
WorkspaceTestsRequest,
} from "../sourcekit-lsp/extensions";
import { SwiftPackage, TargetType } from "../SwiftPackage";
import { SwiftPackage } from "../SwiftPackage";
import { LanguageClientManager } from "../sourcekit-lsp/LanguageClientManager";
import { LanguageClient } from "vscode-languageclient/node";

Expand Down Expand Up @@ -68,7 +68,7 @@ export class LSPTestDiscovery {
// workspace/tests method, and is at least version 2.
if (this.checkExperimentalCapability(client, WorkspaceTestsRequest.method, 2)) {
const tests = await client.sendRequest(WorkspaceTestsRequest.type, token);
return this.transformToTestClass(client, swiftPackage, tests);
return await this.transformToTestClass(client, swiftPackage, tests);
} else {
throw new Error(`${WorkspaceTestsRequest.method} requests not supported`);
}
Expand Down Expand Up @@ -96,38 +96,41 @@ export class LSPTestDiscovery {
* Convert from `LSPTestItem[]` to `TestDiscovery.TestClass[]`,
* updating the format of the location.
*/
private transformToTestClass(
private async transformToTestClass(
client: LanguageClient,
swiftPackage: SwiftPackage,
input: LSPTestItem[]
): TestDiscovery.TestClass[] {
return input.map(item => {
): Promise<TestDiscovery.TestClass[]> {
let result: TestDiscovery.TestClass[] = [];
for (const item of input) {
const location = client.protocol2CodeConverter.asLocation(item.location);
return {
...item,
id: this.transformId(item, location, swiftPackage),
children: this.transformToTestClass(client, swiftPackage, item.children),
location,
};
});
result = [
...result,
{
...item,
id: await this.transformId(item, location, swiftPackage),
children: await this.transformToTestClass(client, swiftPackage, item.children),
location,
},
];
}
return result;
}

/**
* If the test is an XCTest, transform the ID provided by the LSP from a
* swift-testing style ID to one that XCTest can use. This allows the ID to
* be used to tell to the test runner (xctest or swift-testing) which tests to run.
*/
private transformId(
private async transformId(
item: LSPTestItem,
location: vscode.Location,
swiftPackage: SwiftPackage
): string {
): Promise<string> {
// XCTest: Target.TestClass/testCase
// swift-testing: TestClass/testCase()
// TestClassOrStruct/NestedTestSuite/testCase()
const target = swiftPackage
.getTargets(TargetType.test)
.find(target => swiftPackage.getTarget(location.uri.fsPath) === target);
const target = await swiftPackage.getTarget(location.uri.fsPath);

// If we're using an older sourcekit-lsp it doesn't prepend the target name
// to the test item id.
Expand Down
27 changes: 17 additions & 10 deletions src/TestExplorer/TestDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,34 @@ const defaultTags = [runnableTag.id, "test-target", "XCTest", "swift-testing"];
* @param swiftPackage A swift package containing test targets
* @param testClasses Array of test classes
*/
export function updateTestsFromClasses(
export async function updateTestsFromClasses(
testController: vscode.TestController,
swiftPackage: SwiftPackage,
testItems: TestClass[]
) {
const targets = swiftPackage.getTargets(TargetType.test).map(target => {
const filteredItems = testItems.filter(
testItem =>
testItem.location && swiftPackage.getTarget(testItem.location.uri.fsPath) === target
);
return {
const targets = await swiftPackage.getTargets(TargetType.test);
const results: TestClass[] = [];
for (const target of targets) {
const filteredItems: TestClass[] = [];
for (const testItem of testItems) {
if (
testItem.location &&
(await swiftPackage.getTarget(testItem.location.uri.fsPath)) === target
) {
filteredItems.push(testItem);
}
}
results.push({
id: target.c99name,
label: target.name,
children: filteredItems,
location: undefined,
disabled: false,
style: "test-target",
tags: [],
} as TestClass;
});
updateTests(testController, targets);
});
}
updateTests(testController, results);
}

export function updateTestsForTarget(
Expand Down
Loading
Loading