Add tests for command-line parser #102
Conversation
Previously, we didn't have any tests for how we parsed command-line flags. This was problematic because the tests didn't catch breaking changes in seemingly innocent pull requests like #100 that rename a variable. Now, we have tests for the command-line parser so that we hopefully catch more unintentional regressions in the future.
| @@ -19,7 +19,7 @@ export const noJsConfig = '{}' | |||
| * If the directory contains at least one `*.{ts,tsx}` file then the config will be empty (`{}`). | |||
| * If the directory doesn't contains one `*.{ts,tsx}` file then the config will | |||
| */ | |||
| export function inferTsConfig(projectPath: string): string { | |||
| export function inferTsconfig(projectPath: string): string { | |||
There was a problem hiding this comment.
I apologize for mixing this unrelated diff into the PR. It's quite painful to correct the capitalization of files on macOS and I didn't feel like doing it again to have a cleaner PR
|
|
||
| function checkIndexParser( | ||
| args: string[], | ||
| expectedOptions: Partial<MultiProjectOptions>, |
There was a problem hiding this comment.
Oh wow, I didn't know this was a thing you could do so easily in TypeScript. (The Partial<> type)
There was a problem hiding this comment.
It's very handy for cases like this! TypeScript includes several utility types https://www.typescriptlang.org/docs/handbook/utility-types.html#partialtype but you can go a bit overboard with them so use with care.
src/CommandLineOptions.ts
Outdated
| .option('--cwd', 'the working directory', process.cwd()) | ||
| .option('--yarn-workspaces', 'whether to index all yarn workspaces', false) | ||
| .option( | ||
| '--infer-tsconfig', | ||
| "whether to infer the tsconfig.json file, if it's missing", | ||
| false | ||
| ) | ||
| .option('--output', 'path to the output file', 'dump.lsif-typed') | ||
| .option('--no-progress-bar', 'whether to disable the progress bar') | ||
| .argument('[projects...]') |
There was a problem hiding this comment.
Now I'm wondering... is there a difference between:
lsif-typescript index --cwd /some/path
vs
lsif-typescript index /some/path
?
There was a problem hiding this comment.
It's good you asked because it surfaced a bug in how we parse the --cwd flag. I fixed the bug and added a cli test for --cwd. There is a difference between those two:
index --cwd /pathgenerates/path/dump.lsif-typedindex /pathgenerates$PWD/dump.lsif-typed
Previously, we didn't have any tests for how we parsed command-line
flags. This was problematic because the tests didn't catch breaking
changes in seemingly innocent pull requests like #100
that rename a variable. Now, we have tests for the command-line parser
so that we hopefully catch more unintentional regressions in the future.
Test plan
See new
CommandLineOptions.test.tstests.