Skip to content

Commit

Permalink
Add cwd arg and cwd behavior (#1305)
Browse files Browse the repository at this point in the history
## Summary:
This adds a new `--cwd` argument to override the current working directory that checksync uses.

More importantly, it now sets the current working directory to the location of the configuration file, if one is loaded. This ensures that paths/globs specified in a configuration file are interpreted relative to the configuration file, not the current working directory, which makes it a lot easier to reason about the configuration file definition.

Issue: fixes #1262

## Test plan:
`yarn test`

Author: somewhatabstract

Reviewers: jeremywiebe, kevinbarabash

Required Reviewers:

Approved By:

Checks: ✅ CodeQL, ✅ codecov/project, ✅ Test and build (macOS-latest, 18.x), ✅ Test and build (macOS-latest, 16.x), ✅ Test and build (ubuntu-latest, 18.x), ✅ Test and build (ubuntu-latest, 16.x), ✅ Test and build (windows-latest, 18.x), ✅ Test and build (windows-latest, 16.x), ✅ Integration tests (windows-latest, 18.x), ✅ Integration tests (windows-latest, 16.x), ✅ Integration tests (macOS-latest, 18.x), ✅ Integration tests (macOS-latest, 16.x), ✅ Integration tests (ubuntu-latest, 18.x), ✅ Integration tests (ubuntu-latest, 16.x), ✅ Update test coverage (ubuntu-latest, 16.x), ✅ Lint and static types check (ubuntu-latest, 16.x), ✅ Analyze (javascript), ⏭  dependabot

Pull Request URL: #1305
  • Loading branch information
somewhatabstract committed Jun 11, 2023
1 parent 95d1a29 commit 14bbc5f
Show file tree
Hide file tree
Showing 29 changed files with 868 additions and 528 deletions.
7 changes: 7 additions & 0 deletions .changeset/lovely-apples-cough.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"checksync": major
---

- The location of the configuration file is now used as the current working directory, if a configuration file is used. This means that globs are resolved relative to the configuration file, not the current working directory of the process launching checksync, which makes for a more deterministic behavior for
folks trying to define and use their config files.
- A `--cwd` argument has been added for specifying the working directory in cases where a configuration file is not used, or the configuration file discover needs to start in a place other than where checksync is invoked. If a configuration file is loaded, the location of that file takes precedence.
8 changes: 4 additions & 4 deletions .codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ coverage:

status:
project:
default: on
default: true
patch:
default: off
changes:
Expand All @@ -14,6 +14,6 @@ coverage:
comment:
layout: "header, reach, diff, flags, files, footer"
behavior: default
require_changes: no
require_base: no
require_head: yes
require_changes: false
require_base: false
require_head: true
15 changes: 15 additions & 0 deletions src/__tests__/__snapshots__/cli.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ Where:
.checksyncrc
.checksyncrc.json
Paths within the config file are resolved relative to
the location of the config file.
--cwd The current working directory to use when searching
for a configuration file, and resolving relative paths
and globs.
The --config path takes precedence over this
argument. If there is no --config argument, this
location is used to find a config file. If a config file
is found, the working directory will then change to
the location of that file, otherwise this argument's
value is used when resolve the remainder of the given
arguments.
--dry-run,-n Ignored unless supplied with --update-tags.
--help,-h Outputs this help text.
Expand Down
739 changes: 392 additions & 347 deletions src/__tests__/__snapshots__/integration.test.ts.snap

Large diffs are not rendered by default.

16 changes: 6 additions & 10 deletions src/__tests__/check-sync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as ProcessCache from "../process-cache";
import Logger from "../logger";

import checkSync from "../check-sync";
import ExitCodes from "../exit-codes";
import {ExitCode} from "../exit-codes";

import {Options} from "../types";

Expand Down Expand Up @@ -110,16 +110,14 @@ describe("#checkSync", () => {
const result = await checkSync(options, NullLogger);

// Assert
expect(result).toBe(ExitCodes.NO_FILES);
expect(result).toBe(ExitCode.NO_FILES);
});

it("should build a marker cache from the files", async () => {
// Arrange
const NullLogger = new Logger();
jest.spyOn(GetFiles, "default").mockResolvedValue(["filea", "fileb"]);
jest.spyOn(ProcessCache, "default").mockResolvedValue(
ExitCodes.SUCCESS,
);
jest.spyOn(ProcessCache, "default").mockResolvedValue(ExitCode.SUCCESS);
const getMarkersFromFilesSpy = jest
.spyOn(GetMarkersFromFiles, "default")
.mockResolvedValue({});
Expand Down Expand Up @@ -151,7 +149,7 @@ describe("#checkSync", () => {
jest.spyOn(GetMarkersFromFiles, "default").mockResolvedValue(fakeCache);
const ProcessCacheSpy = jest
.spyOn(ProcessCache, "default")
.mockResolvedValue(ExitCodes.SUCCESS);
.mockResolvedValue(ExitCode.SUCCESS);
const options: Options = {
includeGlobs: ["glob1", "glob2"],
excludeGlobs: [],
Expand Down Expand Up @@ -180,9 +178,7 @@ describe("#checkSync", () => {
const fakeCache: Record<string, any> = {};
jest.spyOn(GetFiles, "default").mockResolvedValue(["filea", "fileb"]);
jest.spyOn(GetMarkersFromFiles, "default").mockResolvedValue(fakeCache);
jest.spyOn(ProcessCache, "default").mockResolvedValue(
ExitCodes.SUCCESS,
);
jest.spyOn(ProcessCache, "default").mockResolvedValue(ExitCode.SUCCESS);

// Act
const result = await checkSync(
Expand All @@ -199,6 +195,6 @@ describe("#checkSync", () => {
);

// Assert
expect(result).toBe(ExitCodes.SUCCESS);
expect(result).toBe(ExitCode.SUCCESS);
});
});
91 changes: 64 additions & 27 deletions src/__tests__/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ import * as minimist from "minimist";
import fs from "fs";
import {run} from "../cli";
import * as CheckSync from "../check-sync";
import ExitCodes from "../exit-codes";
import {ExitCode} from "../exit-codes";
import Logger from "../logger";
import {version} from "../../package.json";
import * as DetermineOptions from "../determine-options";
import defaultOptions from "../default-options";
import * as Exit from "../exit";
import * as SetCwd from "../set-cwd";

jest.mock("../set-cwd");
jest.mock("minimist");
jest.mock("../logger", () => {
const realLogger = jest.requireActual("../logger").default;
Expand Down Expand Up @@ -45,7 +48,7 @@ describe("#run", () => {
);
jest.spyOn(CheckSync, "default").mockResolvedValue(0);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementationOnce(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});

// Act
await run(__filename);
Expand All @@ -66,8 +69,8 @@ describe("#run", () => {
} as const;
jest.spyOn(minimist, "default").mockReturnValue(fakeParsedArgs);
const exitSpy = jest
.spyOn(process, "exit")
.mockImplementationOnce(() => {
.spyOn(Exit, "default")
.mockImplementation(() => {
throw new Error("PRETEND PROCESS EXIT!");
});

Expand All @@ -76,7 +79,10 @@ describe("#run", () => {

// Assert
expect(underTest).toThrowError("PRETEND PROCESS EXIT!");
expect(exitSpy).toHaveBeenCalledWith(ExitCodes.SUCCESS);
expect(exitSpy).toHaveBeenCalledWith(
expect.anything(),
ExitCode.SUCCESS,
);
},
);

Expand All @@ -86,7 +92,7 @@ describe("#run", () => {
version: true,
} as const;
jest.spyOn(minimist, "default").mockReturnValue(fakeParsedArgs);
jest.spyOn(process, "exit").mockImplementationOnce(() => {
jest.spyOn(Exit, "default").mockImplementation(() => {
throw new Error("PRETEND PROCESS EXIT!");
});
const logSpy = jest.spyOn(new Logger(null), "log");
Expand All @@ -105,7 +111,7 @@ describe("#run", () => {
help: true,
} as const;
jest.spyOn(minimist, "default").mockReturnValue(fakeParsedArgs);
jest.spyOn(process, "exit").mockImplementationOnce(() => {
jest.spyOn(Exit, "default").mockImplementation(() => {
throw new Error("PRETEND PROCESS EXIT!");
});
const logSpy = jest.spyOn(new Logger(null), "log");
Expand All @@ -119,6 +125,28 @@ describe("#run", () => {
expect(logSpy.mock.calls[0][0]).toMatchSnapshot();
});

it("should change the working directory if cwd arg present", async () => {
// Arrange
const fakeParsedArgs: any = {
cwd: "/some/path",
verbose: true,
} as const;
jest.spyOn(minimist, "default").mockReturnValue(fakeParsedArgs);
jest.spyOn(DetermineOptions, "default").mockResolvedValue(
defaultOptions,
);
jest.spyOn(CheckSync, "default").mockResolvedValue(0);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(Exit, "default").mockImplementation(() => {});
const setCwdSpy = jest.spyOn(SetCwd, "default");

// Act
await run(__filename);

// Assert
expect(setCwdSpy).toHaveBeenCalledWith(expect.anything(), "/some/path");
});

it("should pass arguments to determineOptions", async () => {
// Arrange
const fakeParsedArgs: any = {
Expand All @@ -130,7 +158,7 @@ describe("#run", () => {
.mockResolvedValue(defaultOptions);
jest.spyOn(CheckSync, "default").mockResolvedValue(0);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementationOnce(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});

// Act
await run(__filename);
Expand Down Expand Up @@ -158,7 +186,7 @@ describe("#run", () => {
.spyOn(CheckSync, "default")
.mockResolvedValue(0);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementationOnce(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});

// Act
await run(__filename);
Expand All @@ -181,7 +209,7 @@ describe("#run", () => {
jest.spyOn(minimist, "default").mockReturnValue(fakeParsedArgs);
jest.spyOn(fs, "existsSync").mockReturnValueOnce(false);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementationOnce(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});
const setVerboseSpy = jest.spyOn(new Logger(null), "setVerbose");

// Act
Expand All @@ -198,15 +226,18 @@ describe("#run", () => {
);
jest.spyOn(minimist, "default").mockReturnValue({} as any);
const exitSpy = jest
.spyOn(process, "exit")
.spyOn(Exit, "default")
// @ts-expect-error this is typed to never, but we want to mock it
.mockImplementationOnce(() => {});
.mockImplementation(() => {});

// Act
await run(__filename);

// Assert
expect(exitSpy).toHaveBeenCalledWith(ExitCodes.BAD_CONFIG);
expect(exitSpy).toHaveBeenCalledWith(
expect.anything(),
ExitCode.BAD_CONFIG,
);
});

it("should exit process with the exit code from checkSync", async () => {
Expand All @@ -217,15 +248,15 @@ describe("#run", () => {
);
jest.spyOn(minimist, "default").mockReturnValue({} as any);
const exitSpy = jest
.spyOn(process, "exit")
.spyOn(Exit, "default")
// @ts-expect-error this is typed to never, but we want to mock it
.mockImplementationOnce(() => {});
.mockImplementation(() => {});

// Act
await run(__filename);

// Assert
expect(exitSpy).toHaveBeenCalledWith(42);
expect(exitSpy).toHaveBeenCalledWith(expect.anything(), 42);
});

it("should log error on rejection of checkSync method", async () => {
Expand All @@ -238,7 +269,7 @@ describe("#run", () => {
);
jest.spyOn(minimist, "default").mockReturnValue({} as any);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementation(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});
const logSpy = jest.spyOn(new Logger(null), "error");

// Act
Expand All @@ -260,15 +291,18 @@ describe("#run", () => {
);
jest.spyOn(minimist, "default").mockReturnValue({} as any);
const exitSpy = jest
.spyOn(process, "exit")
.spyOn(Exit, "default")
// @ts-expect-error this is typed to never, but we want to mock it
.mockImplementation(() => {});

// Act
await run(__filename);

// Assert
expect(exitSpy).toHaveBeenCalledWith(ExitCodes.CATASTROPHIC);
expect(exitSpy).toHaveBeenCalledWith(
expect.anything(),
ExitCode.CATASTROPHIC,
);
});

describe("unknown arg handling", () => {
Expand All @@ -279,7 +313,7 @@ describe("#run", () => {
defaultOptions,
);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementationOnce(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});
const minimistSpy = jest
.spyOn(minimist, "default")
.mockReturnValue({} as any);
Expand All @@ -300,7 +334,7 @@ describe("#run", () => {
defaultOptions,
);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementationOnce(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});
const minimistSpy = jest
.spyOn(minimist, "default")
.mockReturnValue({} as any);
Expand All @@ -321,7 +355,7 @@ describe("#run", () => {
defaultOptions,
);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementationOnce(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});
const minimistSpy = jest
.spyOn(minimist, "default")
.mockReturnValue({} as any);
Expand All @@ -342,7 +376,7 @@ describe("#run", () => {
defaultOptions,
);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementationOnce(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});
const minimistSpy = jest
.spyOn(minimist, "default")
.mockReturnValue({} as any);
Expand All @@ -366,7 +400,7 @@ describe("#run", () => {
defaultOptions,
);
const exitSpy = jest
.spyOn(process, "exit")
.spyOn(Exit, "default")
// @ts-expect-error this is typed to never, but we want to mock it
.mockImplementation(() => {});
const minimistSpy = jest
Expand All @@ -379,7 +413,10 @@ describe("#run", () => {
unknownHandler?.("--imadethisup");

// Assert
expect(exitSpy).toHaveBeenCalledWith(ExitCodes.UNKNOWN_ARGS);
expect(exitSpy).toHaveBeenCalledWith(
expect.anything(),
ExitCode.UNKNOWN_ARGS,
);
});

it("should report unknown arguments starting with -", async () => {
Expand All @@ -389,7 +426,7 @@ describe("#run", () => {
defaultOptions,
);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementation(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});
const minimistSpy = jest
.spyOn(minimist, "default")
.mockReturnValue({} as any);
Expand All @@ -413,7 +450,7 @@ describe("#run", () => {
defaultOptions,
);
// @ts-expect-error this is typed to never, but we want to mock it
jest.spyOn(process, "exit").mockImplementation(() => {});
jest.spyOn(Exit, "default").mockImplementation(() => {});
const minimistSpy = jest
.spyOn(minimist, "default")
.mockReturnValue({} as any);
Expand Down

0 comments on commit 14bbc5f

Please sign in to comment.