Skip to content

Commit

Permalink
refactor: make project-manifest io async (#131)
Browse files Browse the repository at this point in the history
Refactor logic related to project-manifest io to be async. This way it can be used without blocking.
  • Loading branch information
ComradeVanti authored Jan 18, 2024
1 parent 4babac0 commit e45ed92
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 46 deletions.
5 changes: 3 additions & 2 deletions src/cmd-add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const add = async function (
let version = split[1];

// load manifest
const manifest = loadProjectManifest(env.cwd);
const manifest = await loadProjectManifest(env.cwd);
if (manifest === null) return { code: 1, dirty };
// packages that added to scope registry
const pkgsInScope = Array.of<DomainName>();
Expand Down Expand Up @@ -232,7 +232,8 @@ export const add = async function (
if (options.test) addTestable(manifest, name);
// save manifest
if (dirty) {
if (!saveProjectManifest(env.cwd, manifest)) return { code: 1, dirty };
if (!(await saveProjectManifest(env.cwd, manifest)))
return { code: 1, dirty };
}
return { code: 0, dirty };
};
Expand Down
6 changes: 5 additions & 1 deletion src/cmd-deps.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import log from "./logger";
import { parseEnv } from "./utils/env";
import { Dependency, fetchPackageDependencies, getNpmClient } from "./registry-client";
import {
Dependency,
fetchPackageDependencies,
getNpmClient,
} from "./registry-client";
import { isPackageUrl } from "./types/package-url";
import {
packageReference,
Expand Down
5 changes: 3 additions & 2 deletions src/cmd-remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const remove = async function (
return { code: 1, dirty };
}
// load manifest
const manifest = loadProjectManifest(env.cwd);
const manifest = await loadProjectManifest(env.cwd);
if (manifest === null) return { code: 1, dirty };
// not found array
const pkgsNotFound = Array.of<PackageReference>();
Expand All @@ -69,7 +69,8 @@ export const remove = async function (
}
// save manifest
if (dirty) {
if (!saveProjectManifest(env.cwd, manifest)) return { code: 1, dirty };
if (!(await saveProjectManifest(env.cwd, manifest)))
return { code: 1, dirty };
}
if (pkgsNotFound.length) {
log.error("404", `package not found: ${pkgsNotFound.join(", ")}`);
Expand Down
2 changes: 1 addition & 1 deletion src/types/package-reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function splitPackageReference(
/**
* Constructs a package-reference.
* @param name The package-name. Will be validated to be a {@link DomainName}.
* @param version Optional version-reference. Will be validated to be a
* @param version Optional version-reference. Will be validated to be a
* {@link VersionReference}.
*/
export function packageReference(
Expand Down
2 changes: 1 addition & 1 deletion src/types/project-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export function addTestable(manifest: UnityProjectManifest, name: DomainName) {
}

/**
* Determines the path to the package manifest based on the project
* Determines the path to the package manifest based on the project
* directory.
* @param projectPath The root path of the Unity project.
*/
Expand Down
14 changes: 7 additions & 7 deletions src/utils/project-manifest-io.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import fs from "fs";
import fs from "fs/promises";
import { assertIsError } from "./error-type-guards";
import log from "../logger";
import {
Expand All @@ -12,12 +12,12 @@ import path from "path";
* Attempts to load the manifest for a Unity project.
* @param projectPath The path to the root of the project.
*/
export const loadProjectManifest = function (
export const loadProjectManifest = async function (
projectPath: string
): UnityProjectManifest | null {
): Promise<UnityProjectManifest | null> {
const manifestPath = manifestPathFor(projectPath);
try {
const text = fs.readFileSync(manifestPath, { encoding: "utf8" });
const text = await fs.readFile(manifestPath, { encoding: "utf8" });
return JSON.parse(text);
} catch (err) {
assertIsError(err);
Expand All @@ -36,15 +36,15 @@ export const loadProjectManifest = function (
* @param projectPath The path to the projects root directory.
* @param data The manifest to save.
*/
export const saveProjectManifest = function (
export const saveProjectManifest = async function (
projectPath: string,
data: UnityProjectManifest
) {
const manifestPath = manifestPathFor(projectPath);
const json = JSON.stringify(data, null, 2);
try {
fse.ensureDirSync(path.dirname(manifestPath));
fs.writeFileSync(manifestPath, json);
await fse.ensureDir(path.dirname(manifestPath));
await fs.writeFile(manifestPath, json);
return true;
} catch (err) {
assertIsError(err);
Expand Down
16 changes: 9 additions & 7 deletions test/setup/unity-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import assert from "assert";
import { mockEnv, MockEnvSession } from "../mock-env";
import { UPMConfig } from "../../src/types/upm-config";
import { saveUpmConfig } from "../../src/utils/upm-config-io";
import {createProjectVersionTxt} from "../../src/utils/project-version-io";
import { createProjectVersionTxt } from "../../src/utils/project-version-io";

/**
* A mock Unity project for testing.
Expand All @@ -28,14 +28,16 @@ export type MockUnityProject = {
* Attempts to load the project manifest for the project.
* Null if not found.
*/
tryGetManifest(): UnityProjectManifest | null;
tryGetManifest(): Promise<UnityProjectManifest | null>;

/**
* Runs an assertion function on the project manifest.
* @param assertFn An assertion function.
* @throws AssertionError if no manifest was found.
*/
tryAssertManifest(assertFn: (manifest: UnityProjectManifest) => void): void;
tryAssertManifest(
assertFn: (manifest: UnityProjectManifest) => void
): Promise<void>;

/**
* Resets the mock-project to its original state.
Expand Down Expand Up @@ -113,7 +115,7 @@ export async function setupUnityProject(
// Project manifest
if (config.manifest !== false) {
const manifest = config.manifest ?? defaultManifest;
saveProjectManifest(projectPath, manifest);
await saveProjectManifest(projectPath, manifest);
}
}

Expand All @@ -125,14 +127,14 @@ export async function setupUnityProject(
envSession?.unhook();
}

function tryGetManifest(): UnityProjectManifest | null {
function tryGetManifest(): Promise<UnityProjectManifest | null> {
return loadProjectManifest(projectPath);
}

function tryAssertManifest(
async function tryAssertManifest(
assertFn: (manifest: UnityProjectManifest) => void
) {
const manifest = tryGetManifest();
const manifest = await tryGetManifest();
assert(manifest !== null);
assertFn(manifest);
}
Expand Down
30 changes: 15 additions & 15 deletions test/test-cmd-add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe("cmd-add.ts", function () {
it("add pkg", async function () {
const retCode = await add(packageA, options);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestA)
);
mockConsole.hasLineIncluding("out", "added").should.be.ok();
Expand All @@ -172,7 +172,7 @@ describe("cmd-add.ts", function () {
options
);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestA)
);
mockConsole.hasLineIncluding("out", "added").should.be.ok();
Expand All @@ -181,7 +181,7 @@ describe("cmd-add.ts", function () {
it("add pkg@latest", async function () {
const retCode = await add(packageReference(packageA, "latest"), options);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestA)
);
mockConsole.hasLineIncluding("out", "added").should.be.ok();
Expand All @@ -198,7 +198,7 @@ describe("cmd-add.ts", function () {
options
);
retCode2.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestA)
);
mockConsole.hasLineIncluding("out", "modified ").should.be.ok();
Expand All @@ -215,7 +215,7 @@ describe("cmd-add.ts", function () {
options
);
retCode2.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestA)
);
mockConsole.hasLineIncluding("out", "existed ").should.be.ok();
Expand All @@ -227,7 +227,7 @@ describe("cmd-add.ts", function () {
options
);
retCode.should.equal(1);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(defaultManifest)
);
mockConsole
Expand All @@ -239,7 +239,7 @@ describe("cmd-add.ts", function () {
const gitUrl = "https://github.com/yo/com.base.package-a" as PackageUrl;
const retCode = await add(packageReference(packageA, gitUrl), options);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
shouldHaveDependency(manifest, packageA, gitUrl)
);
mockConsole.hasLineIncluding("out", "added").should.be.ok();
Expand All @@ -249,7 +249,7 @@ describe("cmd-add.ts", function () {
const gitUrl = "git@github.com:yo/com.base.package-a" as PackageUrl;
const retCode = await add(packageReference(packageA, gitUrl), options);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
shouldHaveDependency(manifest, packageA, gitUrl)
);
mockConsole.hasLineIncluding("out", "added").should.be.ok();
Expand All @@ -259,7 +259,7 @@ describe("cmd-add.ts", function () {
const fileUrl = "file../yo/com.base.package-a" as PackageUrl;
const retCode = await add(packageReference(packageA, fileUrl), options);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
shouldHaveDependency(manifest, packageA, fileUrl)
);
mockConsole.hasLineIncluding("out", "added").should.be.ok();
Expand All @@ -268,15 +268,15 @@ describe("cmd-add.ts", function () {
it("add pkg-not-exist", async function () {
const retCode = await add(packageMissing, options);
retCode.should.equal(1);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(defaultManifest)
);
mockConsole.hasLineIncluding("out", "package not found").should.be.ok();
});
it("add more than one pkgs", async function () {
const retCode = await add([packageA, packageB], options);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestAB)
);
mockConsole
Expand All @@ -290,7 +290,7 @@ describe("cmd-add.ts", function () {
it("add pkg from upstream", async function () {
const retCode = await add(packageUp, upstreamOptions);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestUpstream)
);
mockConsole
Expand All @@ -301,7 +301,7 @@ describe("cmd-add.ts", function () {
it("add pkg-not-exist from upstream", async function () {
const retCode = await add(packageMissing, upstreamOptions);
retCode.should.equal(1);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(defaultManifest)
);
mockConsole.hasLineIncluding("out", "package not found").should.be.ok();
Expand All @@ -312,7 +312,7 @@ describe("cmd-add.ts", function () {
upstreamOptions
);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestC)
);
mockConsole.hasLineIncluding("out", "added").should.be.ok();
Expand All @@ -321,7 +321,7 @@ describe("cmd-add.ts", function () {
it("add pkg with tests", async function () {
const retCode = await add(packageA, testableOptions);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) =>
await mockProject.tryAssertManifest((manifest) =>
manifest.should.deepEqual(expectedManifestTestable)
);
mockConsole.hasLineIncluding("out", "added").should.be.ok();
Expand Down
8 changes: 4 additions & 4 deletions test/test-cmd-remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe("cmd-remove.ts", function () {
};
const retCode = await remove(packageA, options);
retCode.should.equal(0);
mockProject.tryAssertManifest((manifest) => {
await mockProject.tryAssertManifest((manifest) => {
shouldNotHaveDependency(manifest, packageA);
shouldHaveRegistryWithScopes(manifest, [packageB]);
});
Expand All @@ -71,7 +71,7 @@ describe("cmd-remove.ts", function () {
options
);
retCode.should.equal(1);
mockProject.tryAssertManifest((manifest) => {
await mockProject.tryAssertManifest((manifest) => {
manifest.should.deepEqual(defaultManifest);
});
mockConsole.hasLineIncluding("out", "please replace").should.be.ok();
Expand All @@ -84,7 +84,7 @@ describe("cmd-remove.ts", function () {
};
const retCode = await remove(missingPackage, options);
retCode.should.equal(1);
mockProject.tryAssertManifest((manifest) => {
await mockProject.tryAssertManifest((manifest) => {
manifest.should.deepEqual(defaultManifest);
});
mockConsole.hasLineIncluding("out", "package not found").should.be.ok();
Expand All @@ -98,7 +98,7 @@ describe("cmd-remove.ts", function () {
const retCode = await remove([packageA, packageB], options);
retCode.should.equal(0);

mockProject.tryAssertManifest((manifest) => {
await mockProject.tryAssertManifest((manifest) => {
shouldNotHaveDependency(manifest, packageA);
shouldNotHaveDependency(manifest, packageB);
});
Expand Down
14 changes: 8 additions & 6 deletions test/test-project-manifest-io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,30 @@ describe("project-manifest io", function () {
});

it("loadManifest", async function () {
const manifest = loadProjectManifest(mockProject.projectPath);
const manifest = await loadProjectManifest(mockProject.projectPath);
should(manifest).be.deepEqual({ dependencies: {} });
});
it("no manifest file", async function () {
const manifest = loadProjectManifest("/invalid-path");
const manifest = await loadProjectManifest("/invalid-path");
should(manifest).be.null();
mockConsole.hasLineIncluding("out", "does not exist").should.be.ok();
});
it("wrong json content", async function () {
const manifestPath = manifestPathFor(mockProject.projectPath);
fse.writeFileSync(manifestPath, "invalid data");

const manifest = loadProjectManifest(mockProject.projectPath);
const manifest = await loadProjectManifest(mockProject.projectPath);
should(manifest).be.null();
mockConsole.hasLineIncluding("out", "failed to parse").should.be.ok();
});
it("saveManifest", async function () {
const manifest = loadProjectManifest(mockProject.projectPath)!;
const manifest = (await loadProjectManifest(mockProject.projectPath))!;
shouldNotHaveAnyDependencies(manifest);
addDependency(manifest, domainName("some-pack"), semanticVersion("1.0.0"));
saveProjectManifest(mockProject.projectPath, manifest).should.be.ok();
const manifest2 = loadProjectManifest(mockProject.projectPath);
(
await saveProjectManifest(mockProject.projectPath, manifest)
).should.be.ok();
const manifest2 = await loadProjectManifest(mockProject.projectPath);
should(manifest2).be.deepEqual(manifest);
});
it("manifest-path is correct", function () {
Expand Down

0 comments on commit e45ed92

Please sign in to comment.