Skip to content

Commit

Permalink
fix: remove cwd pre-check (#276)
Browse files Browse the repository at this point in the history
Currently, we check if cwd exists during `parseEnv`. This is not really necessary, because all operations which use cwd will also check if the path exist or handle missing files accordingly.

We can thus safely remove this check. This comes with two slight changes in behavior:

- The app fails later if cwd is not found. Instead of the app failing immediately during start-up, will will only fail once cwd is actually required.
- There was a error-log in `parseEnv` which notified of the missing path. This log is now removed.
  • Loading branch information
ComradeVanti committed Apr 24, 2024
1 parent 93ad7fc commit 52f988d
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 57 deletions.
24 changes: 6 additions & 18 deletions src/services/parse-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ import {
UpmConfigLoadError,
} from "../io/upm-config-io";
import path from "path";
import fs from "fs";
import { coerceRegistryUrl, makeRegistryUrl } from "../domain/registry-url";
import { tryGetAuthForRegistry, UPMConfig } from "../domain/upm-config";
import { CmdOptions } from "../cli/options";
import { FileParseError } from "../common-errors";
import { Err, Ok, Result } from "ts-results-es";
import { Ok, Result } from "ts-results-es";
import {
ProjectVersionLoadError,
tryLoadProjectVersion,
Expand Down Expand Up @@ -41,20 +40,14 @@ export type Env = Readonly<{
}>;

export type EnvParseError =
| NotFoundError
| GetUpmConfigDirError
| UpmConfigLoadError
| ProjectVersionLoadError;

function determineCwd(options: CmdOptions): Result<string, NotFoundError> {
const cwd =
options._global.chdir !== undefined
? path.resolve(options._global.chdir)
: process.cwd();

if (!fs.existsSync(cwd)) return Err(new NotFoundError(cwd));

return Ok(cwd);
function determineCwd(options: CmdOptions): string {
return options._global.chdir !== undefined
? path.resolve(options._global.chdir)
: process.cwd();
}

function determineWsl(options: CmdOptions): boolean {
Expand Down Expand Up @@ -147,12 +140,7 @@ export function makeParseEnvService(): ParseEnvService {
const upstreamRegistry = determineUpstreamRegistry(options);

// cwd
const cwdResult = determineCwd(options);
if (cwdResult.isErr()) {
log.error("env", `can not resolve path ${cwdResult.error.path}`);
return cwdResult;
}
const cwd = cwdResult.value;
const cwd = determineCwd(options);

// editor version
const projectVersionLoadResult = await tryLoadProjectVersion(cwd).promise;
Expand Down
39 changes: 0 additions & 39 deletions test/parse-env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,45 +453,6 @@ describe("env", () => {

expect(result).toBeOk((env: Env) => expect(env.cwd).toEqual(expected));
});

it("should fail if specified path is not found", async () => {
const { parseEnv } = makeDependencies();
const notExistentPath = "/some/other/path";
jest
.mocked(fs.existsSync)
.mockImplementation((path) => path !== notExistentPath);

const result = await parseEnv({
_global: {
chdir: notExistentPath,
},
});

expect(result).toBeError((error) =>
expect(error).toBeInstanceOf(NotFoundError)
);
});

it("should notify if specified path is not found", async () => {
const { parseEnv } = makeDependencies();

const notExistentPath = "/some/other/path";
jest
.mocked(fs.existsSync)
.mockImplementation((path) => path !== notExistentPath);
const logSpy = jest.spyOn(log, "error");

await parseEnv({
_global: {
chdir: notExistentPath,
},
});

expect(logSpy).toHaveBeenCalledWith(
"env",
expect.stringContaining("can not resolve")
);
});
});

describe("editor-version", () => {
Expand Down

0 comments on commit 52f988d

Please sign in to comment.