From 057d9de72353922b554c966904a259d55be7a72a Mon Sep 17 00:00:00 2001 From: Ethan Dennis Date: Fri, 6 Mar 2020 14:39:03 -0800 Subject: [PATCH] unit test coverage for caching multiple dirs --- .github/workflows/cache.yml | 29 --------- .github/workflows/workflow.yml | 10 +-- __tests__/actionUtils.test.ts | 116 +++++++++++++++++++++++++++++---- __tests__/restore.test.ts | 2 +- __tests__/save.test.ts | 20 +++--- __tests__/tar.test.ts | 56 ++++++++++------ dist/restore/index.js | 2 +- dist/save/index.js | 2 +- salt.txt | 1 - src/tar.ts | 2 +- src/utils/actionUtils.ts | 1 + 11 files changed, 161 insertions(+), 80 deletions(-) delete mode 100644 .github/workflows/cache.yml delete mode 100644 salt.txt diff --git a/.github/workflows/cache.yml b/.github/workflows/cache.yml deleted file mode 100644 index c3ff3e1..0000000 --- a/.github/workflows/cache.yml +++ /dev/null @@ -1,29 +0,0 @@ -name: Cache - -on: - pull_request: - branches: - - master - -jobs: - build: - runs-on: self-hosted - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Setup Node.js - uses: actions/setup-node@v1 - with: - node-version: '12.x' - - name: Restore npm cache - uses: ./ - id: cache - with: - path: | - node_modules - dist - key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}-${{ hashFiles('salt.txt') }} - - run: npm install - if: steps.cache.outputs.cache-hit != 'true' - - run: npm run build - if: steps.cache.outputs.cache-hit != 'true' \ No newline at end of file diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index 0b19474..c394023 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -1,11 +1,11 @@ name: Tests on: - # pull_request: - # branches: - # - master - # paths-ignore: - # - '**.md' + pull_request: + branches: + - master + paths-ignore: + - '**.md' push: branches: - master diff --git a/__tests__/actionUtils.test.ts b/__tests__/actionUtils.test.ts index 7292001..f51978a 100644 --- a/__tests__/actionUtils.test.ts +++ b/__tests__/actionUtils.test.ts @@ -1,7 +1,8 @@ import * as core from "@actions/core"; -import * as glob from "@actions/glob"; +import * as io from "@actions/io"; import * as os from "os"; import * as path from "path"; +import { promises as fs } from "fs"; import { Events, Outputs, State } from "../src/constants"; import { ArtifactCacheEntry } from "../src/contracts"; @@ -10,10 +11,19 @@ import * as actionUtils from "../src/utils/actionUtils"; jest.mock("@actions/core"); jest.mock("os"); +function getTempDir(): string { + return path.join(__dirname, "_temp", "actionUtils"); +} + afterEach(() => { delete process.env[Events.Key]; }); +afterAll(async () => { + delete process.env["GITHUB_WORKSPACE"]; + await io.rmRF(getTempDir()); +}); + test("getArchiveFileSize returns file size", () => { const filePath = path.join(__dirname, "__fixtures__", "helloWorld.txt"); @@ -183,13 +193,34 @@ test("isValidEvent returns false for unknown event", () => { }); test("resolvePaths with no ~ in path", async () => { - // TODO: these test paths will need to exist - const filePath = ".cache/yarn"; + const filePath = ".cache"; - const resolvedPath = await actionUtils.resolvePaths([filePath]); + // Create the following layout: + // cwd + // cwd/.cache + // cwd/.cache/file.txt - const expectedPath = [path.resolve(filePath)]; - expect(resolvedPath).toStrictEqual(expectedPath); + const root = path.join(getTempDir(), "no-tilde"); + // tarball entries will be relative to workspace + process.env["GITHUB_WORKSPACE"] = root; + + await fs.mkdir(root, { recursive: true }); + const cache = path.join(root, ".cache"); + await fs.mkdir(cache, { recursive: true }); + await fs.writeFile(path.join(cache, "file.txt"), "cached"); + + const originalCwd = process.cwd(); + + try { + process.chdir(root); + + const resolvedPath = await actionUtils.resolvePaths([filePath]); + + const expectedPath = [filePath]; + expect(resolvedPath).toStrictEqual(expectedPath); + } finally { + process.chdir(originalCwd); + } }); test("resolvePaths with ~ in path", async () => { @@ -201,26 +232,87 @@ test("resolvePaths with ~ in path", async () => { return homedir; }); + const root = getTempDir(); + process.env["GITHUB_WORKSPACE"] = root; + const resolvedPath = await actionUtils.resolvePaths([filePath]); - const expectedPath = [path.join(homedir, ".cache/yarn")]; + const expectedPath = [ + path.relative(root, path.join(homedir, ".cache/yarn")) + ]; expect(resolvedPath).toStrictEqual(expectedPath); }); -test("resolvePaths with home not found", () => { +test("resolvePaths with home not found", async () => { const filePath = "~/.cache/yarn"; const homedirMock = jest.spyOn(os, "homedir"); homedirMock.mockImplementation(() => { return ""; }); - // const globMock = jest.spyOn(glob, "homedir"); - // globMock.mockImplementation(() => ""); - expect(async () => await actionUtils.resolvePaths([filePath])).toThrow( - "Unable to resolve `~` to HOME" + await expect(actionUtils.resolvePaths([filePath])).rejects.toThrow( + "Unable to determine HOME directory" ); }); +test("resolvePaths inclusion pattern returns found", async () => { + const pattern = "*.ts"; + // Create the following layout: + // inclusion-patterns + // inclusion-patterns/miss.txt + // inclusion-patterns/test.ts + + const root = path.join(getTempDir(), "inclusion-patterns"); + // tarball entries will be relative to workspace + process.env["GITHUB_WORKSPACE"] = root; + + await fs.mkdir(root, { recursive: true }); + await fs.writeFile(path.join(root, "miss.txt"), "no match"); + await fs.writeFile(path.join(root, "test.ts"), "match"); + + const originalCwd = process.cwd(); + + try { + process.chdir(root); + + const resolvedPath = await actionUtils.resolvePaths([pattern]); + + const expectedPath = ["test.ts"]; + expect(resolvedPath).toStrictEqual(expectedPath); + } finally { + process.chdir(originalCwd); + } +}); + +test("resolvePaths exclusion pattern returns not found", async () => { + const patterns = ["*.ts", "!test.ts"]; + // Create the following layout: + // exclusion-patterns + // exclusion-patterns/miss.txt + // exclusion-patterns/test.ts + + const root = path.join(getTempDir(), "exclusion-patterns"); + // tarball entries will be relative to workspace + process.env["GITHUB_WORKSPACE"] = root; + + await fs.mkdir(root, { recursive: true }); + await fs.writeFile(path.join(root, "miss.txt"), "no match"); + await fs.writeFile(path.join(root, "test.ts"), "no match"); + + const originalCwd = process.cwd(); + + try { + process.chdir(root); + + const resolvedPath = await actionUtils.resolvePaths(patterns); + + const expectedPath = []; + expect(resolvedPath).toStrictEqual(expectedPath); + } finally { + process.chdir(originalCwd); + } +}); + test("isValidEvent returns true for push event", () => { const event = Events.Push; process.env[Events.Key] = event; diff --git a/__tests__/restore.test.ts b/__tests__/restore.test.ts index f759a32..0f003ac 100644 --- a/__tests__/restore.test.ts +++ b/__tests__/restore.test.ts @@ -55,7 +55,7 @@ test("restore with invalid event outputs warning", async () => { test("restore with no path should fail", async () => { const failedMock = jest.spyOn(core, "setFailed"); await run(); - // TODO: this shouldn't be necessary if tarball contains entries relative to workspace + // this input isn't necessary for restore b/c tarball contains entries relative to workspace expect(failedMock).not.toHaveBeenCalledWith( "Input required and not supplied: path" ); diff --git a/__tests__/save.test.ts b/__tests__/save.test.ts index 8b0af81..dbdeec0 100644 --- a/__tests__/save.test.ts +++ b/__tests__/save.test.ts @@ -1,7 +1,7 @@ import * as core from "@actions/core"; import * as path from "path"; import * as cacheHttpClient from "../src/cacheHttpClient"; -import { Events, Inputs } from "../src/constants"; +import { Events, Inputs, CacheFilename } from "../src/constants"; import { ArtifactCacheEntry } from "../src/contracts"; import run from "../src/save"; import * as tar from "../src/tar"; @@ -204,10 +204,10 @@ test("save with large cache outputs warning", async () => { await run(); - const archivePath = path.join("/foo/bar", "cache.tgz"); + const archiveFolder = "/foo/bar"; expect(createTarMock).toHaveBeenCalledTimes(1); - expect(createTarMock).toHaveBeenCalledWith(archivePath, cachePaths); + expect(createTarMock).toHaveBeenCalledWith(archiveFolder, cachePaths); expect(logWarningMock).toHaveBeenCalledTimes(1); expect(logWarningMock).toHaveBeenCalledWith( @@ -314,13 +314,14 @@ test("save with server error outputs warning", async () => { expect(reserveCacheMock).toHaveBeenCalledTimes(1); expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey); - const archivePath = path.join("/foo/bar", "cache.tgz"); + const archiveFolder = "/foo/bar"; + const archiveFile = path.join(archiveFolder, CacheFilename); expect(createTarMock).toHaveBeenCalledTimes(1); - expect(createTarMock).toHaveBeenCalledWith(archivePath, cachePaths); + expect(createTarMock).toHaveBeenCalledWith(archiveFolder, cachePaths); expect(saveCacheMock).toHaveBeenCalledTimes(1); - expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archivePath); + expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile); expect(logWarningMock).toHaveBeenCalledTimes(1); expect(logWarningMock).toHaveBeenCalledWith("HTTP Error Occurred"); @@ -369,13 +370,14 @@ test("save with valid inputs uploads a cache", async () => { expect(reserveCacheMock).toHaveBeenCalledTimes(1); expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey); - const archivePath = path.join("/foo/bar", "cache.tgz"); + const archiveFolder = "/foo/bar"; + const archiveFile = path.join(archiveFolder, CacheFilename); expect(createTarMock).toHaveBeenCalledTimes(1); - expect(createTarMock).toHaveBeenCalledWith(archivePath, cachePaths); + expect(createTarMock).toHaveBeenCalledWith(archiveFolder, cachePaths); expect(saveCacheMock).toHaveBeenCalledTimes(1); - expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archivePath); + expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile); expect(failedMock).toHaveBeenCalledTimes(0); }); diff --git a/__tests__/tar.test.ts b/__tests__/tar.test.ts index 570f66e..fbf2718 100644 --- a/__tests__/tar.test.ts +++ b/__tests__/tar.test.ts @@ -1,20 +1,29 @@ import * as exec from "@actions/exec"; import * as io from "@actions/io"; +import { promises as fs } from "fs"; +import * as path from "path"; import * as tar from "../src/tar"; +import { CacheFilename } from "../src/constants"; jest.mock("@actions/exec"); jest.mock("@actions/io"); -beforeAll(() => { +function getTempDir(): string { + return path.join(__dirname, "_temp", "tar"); +} + +beforeAll(async () => { jest.spyOn(io, "which").mockImplementation(tool => { return Promise.resolve(tool); }); process.env["GITHUB_WORKSPACE"] = process.cwd(); + await jest.requireActual("@actions/io").rmRF(getTempDir()); }); -afterAll(() => { +afterAll(async () => { delete process.env["GITHUB_WORKSPACE"]; + await jest.requireActual("@actions/io").rmRF(getTempDir()); }); test("extract tar", async () => { @@ -33,36 +42,43 @@ test("extract tar", async () => { ? `${process.env["windir"]}\\System32\\tar.exe` : "tar"; expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock).toHaveBeenCalledWith(`"${tarPath}"`, [ - "-xz", - "-f", - archivePath, - "-P", - "-C", - workspace - ]); + expect(execMock).toHaveBeenCalledWith( + `"${tarPath}"`, + ["-xz", "-f", archivePath, "-P", "-C", workspace], + { cwd: undefined } + ); }); test("create tar", async () => { const execMock = jest.spyOn(exec, "exec"); - const archivePath = "cache.tar"; + const archiveFolder = getTempDir(); const workspace = process.env["GITHUB_WORKSPACE"]; const sourceDirectories = ["~/.npm/cache", `${workspace}/dist`]; - await tar.createTar(archivePath, sourceDirectories); + await fs.mkdir(archiveFolder, { recursive: true }); + + await tar.createTar(archiveFolder, sourceDirectories); const IS_WINDOWS = process.platform === "win32"; const tarPath = IS_WINDOWS ? `${process.env["windir"]}\\System32\\tar.exe` : "tar"; + expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock).toHaveBeenCalledWith(`"${tarPath}"`, [ - "-cz", - "-f", - archivePath, - "-C", - workspace, - sourceDirectories.join(" ") - ]); + expect(execMock).toHaveBeenCalledWith( + `"${tarPath}"`, + [ + "-cz", + "-f", + CacheFilename, + "-C", + workspace, + "--files-from", + "manifest.txt" + ], + { + cwd: archiveFolder + } + ); }); diff --git a/dist/restore/index.js b/dist/restore/index.js index 0ddb919..2f7436c 100644 --- a/dist/restore/index.js +++ b/dist/restore/index.js @@ -4975,7 +4975,7 @@ function extractTar(archivePath) { exports.extractTar = extractTar; function createTar(archiveFolder, sourceDirectories) { return __awaiter(this, void 0, void 0, function* () { - // TODO: will want to stream sourceDirectories into tar + // Write source directories to manifest.txt to avoid command length limits const manifestFilename = "manifest.txt"; fs_1.writeFileSync(path.join(archiveFolder, manifestFilename), sourceDirectories.join("\n")); const workingDirectory = getWorkingDirectory(); diff --git a/dist/save/index.js b/dist/save/index.js index a037876..d9ae2b3 100644 --- a/dist/save/index.js +++ b/dist/save/index.js @@ -4963,7 +4963,7 @@ function extractTar(archivePath) { exports.extractTar = extractTar; function createTar(archiveFolder, sourceDirectories) { return __awaiter(this, void 0, void 0, function* () { - // TODO: will want to stream sourceDirectories into tar + // Write source directories to manifest.txt to avoid command length limits const manifestFilename = "manifest.txt"; fs_1.writeFileSync(path.join(archiveFolder, manifestFilename), sourceDirectories.join("\n")); const workingDirectory = getWorkingDirectory(); diff --git a/salt.txt b/salt.txt deleted file mode 100644 index c5d6d97..0000000 --- a/salt.txt +++ /dev/null @@ -1 +0,0 @@ -Fri Mar 6 11:28:08 PST 2020 diff --git a/src/tar.ts b/src/tar.ts index b275215..99624da 100644 --- a/src/tar.ts +++ b/src/tar.ts @@ -48,7 +48,7 @@ export async function createTar( archiveFolder: string, sourceDirectories: string[] ): Promise { - // TODO: will want to stream sourceDirectories into tar + // Write source directories to manifest.txt to avoid command length limits const manifestFilename = "manifest.txt"; writeFileSync( path.join(archiveFolder, manifestFilename), diff --git a/src/utils/actionUtils.ts b/src/utils/actionUtils.ts index 692b42c..cdd1d7f 100644 --- a/src/utils/actionUtils.ts +++ b/src/utils/actionUtils.ts @@ -2,6 +2,7 @@ import * as core from "@actions/core"; import * as io from "@actions/io"; import * as glob from "@actions/glob"; import * as fs from "fs"; +import * as os from "os"; import * as path from "path"; import * as uuidV4 from "uuid/v4";