Git only tweaks (#39)

* Adding documentation for --only-git

* Fixing --root documentation

* adding int tests for --only-git

* Shuffling deck chairs and preffering exec over execFile

* giving git an identity in ci

Co-authored-by: github-codeowners <github-codeowners@example.com>

Author: jjmschofield

README.md

@@ -85,8 +85,9 @@ Options:
   -c, --codeowners <filePath>  path to codeowners file (default: "<dir>/.github/CODEOWNERS")
   -o, --output <outputFormat>  how to output format eg: simple, jsonl, csv (default: "simple")
   -u, --unloved                unowned files only (default: false)
+  -g, --only-git               consider only files tracked by git (default: false)
   -s, --stats                  output stats (default: true)
-  -i, --include <partialPath>  paths begening with partial path only (default: '')
+  -r, --root <rootPath>        the root path to filter files by (default: "")
   -h, --help                   output usage information
 ```
 

src/commands/__snapshots__/audit.test.int.ts.snap

@@ -45,6 +45,22 @@ src/ext-wildcard-owner.js,@js-owner
 "
 `;
 
+exports[`audit csv should only consider files tracked in git root when asked: stderr 1`] = `""`;
+
+exports[`audit csv should only consider files tracked in git root when asked: stdout 1`] = `
+".github/CODEOWNERS,@global-owner1,@global-owner2
+.gitignore,@global-owner1,@global-owner2
+build/logs/deep/recursive-root-dir-owner.log,@doctocat
+build/logs/recursive-root-dir-owner.log,@doctocat
+deep/apps/recursive-deep-dir-owner.ts,@octocat
+deep/nested-ignore/.gitignore,@global-owner1,@global-owner2
+deep/nested-ignore/overridden-ignore.js,@js-owner
+default-wildcard-owners.md,@global-owner1,@global-owner2
+docs/non-recursive-dir-owner.md,@doctocat
+src/ext-wildcard-owner.js,@js-owner
+"
+`;
+
 exports[`audit csv should show only unloved files when asked: stderr 1`] = `""`;
 
 exports[`audit csv should show only unloved files when asked: stdout 1`] = `""`;
@@ -90,6 +106,22 @@ exports[`audit jsonl should list ownership for all files: stdout 1`] = `
 "
 `;
 
+exports[`audit jsonl should only consider files tracked in git root when asked: stderr 1`] = `""`;
+
+exports[`audit jsonl should only consider files tracked in git root when asked: stdout 1`] = `
+"{\\"path\\":\\".github/CODEOWNERS\\",\\"owners\\":[\\"@global-owner1\\",\\"@global-owner2\\"],\\"lines\\":29}
+{\\"path\\":\\".gitignore\\",\\"owners\\":[\\"@global-owner1\\",\\"@global-owner2\\"],\\"lines\\":4}
+{\\"path\\":\\"build/logs/deep/recursive-root-dir-owner.log\\",\\"owners\\":[\\"@doctocat\\"],\\"lines\\":0}
+{\\"path\\":\\"build/logs/recursive-root-dir-owner.log\\",\\"owners\\":[\\"@doctocat\\"],\\"lines\\":0}
+{\\"path\\":\\"deep/apps/recursive-deep-dir-owner.ts\\",\\"owners\\":[\\"@octocat\\"],\\"lines\\":0}
+{\\"path\\":\\"deep/nested-ignore/.gitignore\\",\\"owners\\":[\\"@global-owner1\\",\\"@global-owner2\\"],\\"lines\\":2}
+{\\"path\\":\\"deep/nested-ignore/overridden-ignore.js\\",\\"owners\\":[\\"@js-owner\\"],\\"lines\\":0}
+{\\"path\\":\\"default-wildcard-owners.md\\",\\"owners\\":[\\"@global-owner1\\",\\"@global-owner2\\"],\\"lines\\":0}
+{\\"path\\":\\"docs/non-recursive-dir-owner.md\\",\\"owners\\":[\\"@doctocat\\"],\\"lines\\":0}
+{\\"path\\":\\"src/ext-wildcard-owner.js\\",\\"owners\\":[\\"@js-owner\\"],\\"lines\\":0}
+"
+`;
+
 exports[`audit jsonl should show only unloved files when asked: stderr 1`] = `""`;
 
 exports[`audit jsonl should show only unloved files when asked: stdout 1`] = `""`;
@@ -154,6 +186,22 @@ src/ext-wildcard-owner.js	@js-owner
 "
 `;
 
+exports[`audit simple should only consider files tracked in git root when asked: stderr 1`] = `""`;
+
+exports[`audit simple should only consider files tracked in git root when asked: stdout 1`] = `
+".github/CODEOWNERS	@global-owner1	@global-owner2
+.gitignore	@global-owner1	@global-owner2
+build/logs/deep/recursive-root-dir-owner.log	@doctocat
+build/logs/recursive-root-dir-owner.log	@doctocat
+deep/apps/recursive-deep-dir-owner.ts	@octocat
+deep/nested-ignore/.gitignore	@global-owner1	@global-owner2
+deep/nested-ignore/overridden-ignore.js	@js-owner
+default-wildcard-owners.md	@global-owner1	@global-owner2
+docs/non-recursive-dir-owner.md	@doctocat
+src/ext-wildcard-owner.js	@js-owner
+"
+`;
+
 exports[`audit simple should show only unloved files when asked: stderr 1`] = `""`;
 
 exports[`audit simple should show only unloved files when asked: stdout 1`] = `""`;

src/commands/audit.test.int.ts

@@ -1,30 +1,39 @@
+import fs from 'fs';
+import path from 'path';
+import child_process from 'child_process';
+import util from 'util';
+
 import { v4 as uuidv4 } from 'uuid';
 import fixtures from './__fixtures__/default';
 import { generateProject } from './__fixtures__/project-builder.test.helper';
 
-import util from 'util';
-
-const exec = util.promisify(require('child_process').exec);
+const exec = util.promisify(child_process.exec);
+const writeFile = util.promisify(fs.writeFile);
 
 describe('audit', () => {
-  const testId = uuidv4();
-
   let testDir = 'not set';
 
-  beforeAll(async () => {
-    testDir = await generateProject(testId, fixtures);
-    // tslint:disable-next-line:no-console
-    console.log(`test scratch dir: ${testDir}`);
-  });
-
   const runCli = async (args: string) => {
     return exec(`node  ../../../dist/cli.js ${args}`, { cwd: testDir });
   };
 
+  const gitTrackProject = async () => {
+    await exec(`git init`, { cwd: testDir });
+    await exec(`git add .`, { cwd: testDir });
+    await exec(`git config user.email "github-codeowners@example.com"`, { cwd: testDir });
+    await exec(`git config user.name "github-codeowners"`, { cwd: testDir });
+    await exec(`git commit -m "integration tests"`, { cwd: testDir });
+  };
+
   const outputs = ['simple', 'jsonl', 'csv'];
 
   for (const output of outputs) {
     describe(output, () => {
+      beforeEach(async () => {
+        const testId = uuidv4();
+        testDir = await generateProject(testId, fixtures);
+      });
+
       it('should list ownership for all files', async () => {
         const { stdout, stderr } = await runCli(`audit -o ${output}`);
         expect(stdout).toMatchSnapshot('stdout');
@@ -49,6 +58,19 @@ describe('audit', () => {
         expect(stderr).toMatchSnapshot('stderr');
       });
 
+      it('should only consider files tracked in git root when asked', async () => {
+        // Arrange
+        await gitTrackProject();
+        await writeFile(path.resolve(testDir, 'git-untracked.txt'), 'not tracked in git');
+
+        // Act
+        const { stdout, stderr } = await runCli(`audit -g -o ${output}`);
+
+        // Assert
+        expect(stdout).toMatchSnapshot('stdout');
+        expect(stderr).toMatchSnapshot('stderr');
+      });
+
       it('should do all commands in combination when asked', async () => {
         const { stdout, stderr } = await runCli(`audit -us -r deep -o ${output}`);
         expect(stdout).toMatchSnapshot('stdout');

src/lib/ownership/file.ts

@@ -1,15 +1,15 @@
+import * as path from 'path';
 import { OwnedFile } from './lib/OwnedFile';
 import { OwnershipEngine } from './lib/OwnershipEngine';
 import { readDirRecursively } from './lib/readDirRecursively';
-import * as path from 'path';
-import { gitLsFiles } from './lib/gitLsFiles';
+import { readTrackedGitFiles } from './lib/readTrackedGitFiles';
 
 export const getFileOwnership = async (options: { codeowners: string, dir: string, onlyGit: boolean, root?: string }): Promise<OwnedFile[]> => {
   const engine = OwnershipEngine.FromCodeownersFile(options.codeowners);
 
   let filePaths;
   if (options.onlyGit) {
-    filePaths = await gitLsFiles(options.dir);
+    filePaths = await readTrackedGitFiles(options.dir);
   } else {
     filePaths = await readDirRecursively(options.dir, ['.git']);
   }

src/lib/ownership/lib/execFile.ts

@@ -0,0 +1,28 @@
+import { exec } from '../../util/exec';
+import { readTrackedGitFiles } from './readTrackedGitFiles';
+
+jest.mock('../../util/exec');
+const execFileMock = exec as jest.Mock;
+
+describe('readTrackedGitFiles', () => {
+  it('should return the expected list of files when called', async () => {
+    execFileMock.mockResolvedValue({ stdout: 'foo\nbar\n', stderr: '' });
+
+    const result = await readTrackedGitFiles('some/dir');
+
+    expect(result).toStrictEqual(['foo', 'bar']);
+  });
+
+  it('should call git ls-files with the correct directory', async () => {
+    execFileMock.mockResolvedValue({ stdout: '', stderr: '' });
+
+    const result = await readTrackedGitFiles('some/dir');
+
+    expect(exec).toHaveBeenCalledWith(
+      'git ls-files',
+      expect.objectContaining({
+        cwd: 'some/dir',
+      }),
+    );
+  });
+});

src/lib/ownership/lib/gitLsFiles.test.ts

@@ -0,0 +1,6 @@
+import { exec } from '../../util/exec';
+
+export const readTrackedGitFiles = async (dir: string): Promise<string[]> => {
+  const { stdout } = await exec('git ls-files', { cwd: dir });
+  return stdout.split('\n').filter(x => !!x);
+};

src/lib/ownership/lib/gitLsFiles.ts

@@ -0,0 +1,4 @@
+import { exec as realExec } from 'child_process';
+import { promisify } from 'util';
+
+export const exec = promisify(realExec);