refactor to handle file modes

Author: ds300

property-based-tests/executeTestCase.ts

@@ -3,7 +3,7 @@ import * as path from "path"
 
 import { spawnSafeSync } from "../src/spawnSafe"
 import { executeEffects } from "../src/patch/apply"
-import { parsePatch } from "../src/patch/parse"
+import { parsePatchFile } from "../src/patch/parse"
 import { reversePatch } from "../src/patch/reverse"
 
 import { TestCase, Files } from "./testCases"
@@ -38,6 +38,11 @@ jest.mock("fs-extra", () => {
       getWorkingFiles()[to] = getWorkingFiles()[from]
       delete getWorkingFiles()[from]
     }),
+    statSync: jest.fn(path => getWorkingFiles()[path]),
+    chmodSync: jest.fn((path, mode) => {
+      const { contents } = getWorkingFiles()[path]
+      getWorkingFiles()[path] = { contents, mode }
+    }),
   }
 })
 
@@ -85,12 +90,14 @@ export function executeTestCase(testCase: TestCase) {
   writeFiles(tmpDir.name, testCase.cleanFiles)
 
   spawnSafeSync("git", ["add", "-A"], { cwd: tmpDir.name })
-  spawnSafeSync("git", ["commit", "-m", "blah"], {
-    cwd: tmpDir.name,
-  })
-  spawnSafeSync("git", ["rm", "-rf", "*"], {
+  spawnSafeSync("git", ["commit", "--allow-empty", "-m", "blah"], {
     cwd: tmpDir.name,
   })
+  if (Object.keys(testCase.cleanFiles).length > 0) {
+    spawnSafeSync("git", ["rm", "-rf", "*"], {
+      cwd: tmpDir.name,
+    })
+  }
 
   writeFiles(tmpDir.name, testCase.modifiedFiles)
   spawnSafeSync("git", ["add", "-A"], { cwd: tmpDir.name })
@@ -113,8 +120,8 @@ export function executeTestCase(testCase: TestCase) {
 
   it("looks the same whether parsed with blank lines or not", () => {
     reportingFailures(() => {
-      expect(parsePatch(patchFileContents)).toEqual(
-        parsePatch(patchFileContentsWithBlankLines),
+      expect(parsePatchFile(patchFileContents)).toEqual(
+        parsePatchFile(patchFileContentsWithBlankLines),
       )
     })
   })
@@ -124,7 +131,7 @@ export function executeTestCase(testCase: TestCase) {
   it("works forwards", () => {
     fs.setWorkingFiles({ ...testCase.cleanFiles })
     reportingFailures(() => {
-      const effects = parsePatch(patchFileContents)
+      const effects = parsePatchFile(patchFileContents)
       executeEffects(effects, { dryRun: false })
       expect(fs.getWorkingFiles()).toEqual(testCase.modifiedFiles)
     })
@@ -133,7 +140,7 @@ export function executeTestCase(testCase: TestCase) {
   it("works backwards", () => {
     fs.setWorkingFiles({ ...testCase.modifiedFiles })
     reportingFailures(() => {
-      const effects = reversePatch(parsePatch(patchFileContents))
+      const effects = reversePatch(parsePatchFile(patchFileContents))
       executeEffects(effects, { dryRun: false })
       expect(fs.getWorkingFiles()).toEqual(testCase.cleanFiles)
     })

property-based-tests/regressionTests.test.ts

@@ -181,4 +181,16 @@ describe("regression tests", () => {
       modifiedFiles: { cpz: { contents: "E\n", mode: 420 } },
     })
   })
+
+  describe("18", () => {
+    executeTestCase({
+      cleanFiles: {},
+      modifiedFiles: {
+        ww: {
+          contents: "banana",
+          mode: 420,
+        },
+      },
+    })
+  })
 })

property-based-tests/testCases.ts

@@ -1,4 +1,9 @@
 import { generate } from "randomstring"
+import {
+  NON_EXECUTABLE_FILE_MODE,
+  EXECUTABLE_FILE_MODE,
+} from "../src/patch/parse"
+import { assertNever } from "../src/assertNever"
 
 export interface File {
   contents: string
@@ -31,7 +36,7 @@ function makeFileContents(): File {
         length: Math.floor(Math.random() * 1000),
         charset: fileCharSet,
       }) || "",
-    mode: 0o644,
+    mode: Math.random() > 0.5 ? 0o644 : 0o755,
   }
 }
 
@@ -69,11 +74,13 @@ type MutationKind =
   | "deleteLine"
   | "insertLine"
   | "renameFile"
+  | "changeFileMode"
 
 const mutationKindLikelihoods: Array<[MutationKind, number]> = [
   ["deleteFile", 1],
   ["createFile", 1],
   ["renameFile", 1],
+  ["changeFileMode", 1],
   ["deleteLine", 10],
   ["insertLine", 10],
 ]
@@ -126,7 +133,8 @@ function mutateFiles(files: Files): Files {
   const numMutations = Math.ceil(Math.random() * 1000)
 
   for (let i = 0; i < numMutations; i++) {
-    switch (getNextMutationKind()) {
+    const mutationKind = getNextMutationKind()
+    switch (mutationKind) {
       case "deleteFile": {
         if (Object.keys(mutatedFiles).length === 1) {
           break
@@ -160,6 +168,19 @@ function mutateFiles(files: Files): Files {
           mutatedFiles[pathToRename]
         delete mutatedFiles[pathToRename]
         break
+      case "changeFileMode":
+        const pathToChange = selectRandomElement(Object.keys(mutatedFiles))
+        const { mode, contents } = mutatedFiles[pathToChange]
+        mutatedFiles[pathToChange] = {
+          contents,
+          mode:
+            mode === NON_EXECUTABLE_FILE_MODE
+              ? EXECUTABLE_FILE_MODE
+              : NON_EXECUTABLE_FILE_MODE,
+        }
+        break
+      default:
+        assertNever(mutationKind)
     }
   }
 

src/applyPatches.ts

@@ -5,7 +5,7 @@ import { existsSync, readFileSync } from "fs-extra"
 import { join, resolve } from "./path"
 import { posix } from "path"
 import { getPackageDetailsFromPatchFilename } from "./PackageDetails"
-import { parsePatch } from "./patch/parse"
+import { parsePatchFile } from "./patch/parse"
 import { reversePatch } from "./patch/reverse"
 
 function findPatchFiles(patchesDirectory: string): string[] {
@@ -115,7 +115,7 @@ export const applyPatch = (
   reverse: boolean,
 ): boolean => {
   const patchFileContents = readFileSync(patchFilePath).toString()
-  const patch = parsePatch(patchFileContents)
+  const patch = parsePatchFile(patchFileContents)
   try {
     executeEffects(reverse ? reversePatch(patch) : patch, { dryRun: false })
   } catch (e) {

src/assertNever.ts

@@ -0,0 +1,3 @@
+export function assertNever(x: never): never {
+  throw new Error("Unexpected object: " + x)
+}