From 8d14a2150b09e885a026191fd0643f4746224cc0 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Thu, 14 Nov 2019 17:14:16 -0500 Subject: [PATCH] Add unit tests for save (#98) * Clean up args and arrange imports * Arrange args in restore tests * Add unit tests for save * Use const instead of let (linting) --- __tests__/restore.test.ts | 48 +++--- __tests__/save.test.ts | 329 ++++++++++++++++++++++++++++++++++++++ src/cacheHttpClient.ts | 6 +- src/restore.ts | 22 +-- src/save.ts | 29 ++-- 5 files changed, 390 insertions(+), 44 deletions(-) create mode 100644 __tests__/save.test.ts diff --git a/__tests__/restore.test.ts b/__tests__/restore.test.ts index d2f7dd8..1919e30 100644 --- a/__tests__/restore.test.ts +++ b/__tests__/restore.test.ts @@ -263,12 +263,16 @@ test("restore with cache found", async () => { expect(mkdirMock).toHaveBeenCalledWith(cachePath); const IS_WINDOWS = process.platform === "win32"; - const tarArchivePath = IS_WINDOWS - ? archivePath.replace(/\\/g, "/") - : archivePath; - const tarCachePath = IS_WINDOWS ? cachePath.replace(/\\/g, "/") : cachePath; - const args = IS_WINDOWS ? ["-xz", "--force-local"] : ["-xz"]; - args.push(...["-f", tarArchivePath, "-C", tarCachePath]); + const args = IS_WINDOWS + ? [ + "-xz", + "--force-local", + "-f", + archivePath.replace(/\\/g, "/"), + "-C", + cachePath.replace(/\\/g, "/") + ] + : ["-xz", "-f", archivePath, "-C", cachePath]; expect(execMock).toHaveBeenCalledTimes(1); expect(execMock).toHaveBeenCalledWith(`"tar"`, args); @@ -340,12 +344,16 @@ test("restore with a pull request event and cache found", async () => { expect(mkdirMock).toHaveBeenCalledWith(cachePath); const IS_WINDOWS = process.platform === "win32"; - const tarArchivePath = IS_WINDOWS - ? archivePath.replace(/\\/g, "/") - : archivePath; - const tarCachePath = IS_WINDOWS ? cachePath.replace(/\\/g, "/") : cachePath; - const args = IS_WINDOWS ? ["-xz", "--force-local"] : ["-xz"]; - args.push(...["-f", tarArchivePath, "-C", tarCachePath]); + const args = IS_WINDOWS + ? [ + "-xz", + "--force-local", + "-f", + archivePath.replace(/\\/g, "/"), + "-C", + cachePath.replace(/\\/g, "/") + ] + : ["-xz", "-f", archivePath, "-C", cachePath]; expect(execMock).toHaveBeenCalledTimes(1); expect(execMock).toHaveBeenCalledWith(`"tar"`, args); @@ -417,12 +425,16 @@ test("restore with cache found for restore key", async () => { expect(mkdirMock).toHaveBeenCalledWith(cachePath); const IS_WINDOWS = process.platform === "win32"; - const tarArchivePath = IS_WINDOWS - ? archivePath.replace(/\\/g, "/") - : archivePath; - const tarCachePath = IS_WINDOWS ? cachePath.replace(/\\/g, "/") : cachePath; - const args = IS_WINDOWS ? ["-xz", "--force-local"] : ["-xz"]; - args.push(...["-f", tarArchivePath, "-C", tarCachePath]); + const args = IS_WINDOWS + ? [ + "-xz", + "--force-local", + "-f", + archivePath.replace(/\\/g, "/"), + "-C", + cachePath.replace(/\\/g, "/") + ] + : ["-xz", "-f", archivePath, "-C", cachePath]; expect(execMock).toHaveBeenCalledTimes(1); expect(execMock).toHaveBeenCalledWith(`"tar"`, args); diff --git a/__tests__/save.test.ts b/__tests__/save.test.ts new file mode 100644 index 0000000..67b13d2 --- /dev/null +++ b/__tests__/save.test.ts @@ -0,0 +1,329 @@ +import * as core from "@actions/core"; +import * as exec from "@actions/exec"; +import * as io from "@actions/io"; +import * as path from "path"; +import * as cacheHttpClient from "../src/cacheHttpClient"; +import { Inputs } from "../src/constants"; +import { ArtifactCacheEntry } from "../src/contracts"; +import run from "../src/save"; +import * as actionUtils from "../src/utils/actionUtils"; +import * as testUtils from "../src/utils/testUtils"; + +jest.mock("@actions/core"); +jest.mock("@actions/exec"); +jest.mock("@actions/io"); +jest.mock("../src/utils/actionUtils"); +jest.mock("../src/cacheHttpClient"); + +beforeAll(() => { + jest.spyOn(core, "getInput").mockImplementation((name, options) => { + return jest.requireActual("@actions/core").getInput(name, options); + }); + + jest.spyOn(actionUtils, "getCacheState").mockImplementation(() => { + return jest.requireActual("../src/utils/actionUtils").getCacheState(); + }); + + jest.spyOn(actionUtils, "isExactKeyMatch").mockImplementation( + (key, cacheResult) => { + return jest + .requireActual("../src/utils/actionUtils") + .isExactKeyMatch(key, cacheResult); + } + ); + + jest.spyOn(actionUtils, "resolvePath").mockImplementation(filePath => { + return path.resolve(filePath); + }); + + jest.spyOn(actionUtils, "createTempDirectory").mockImplementation(() => { + return Promise.resolve("/foo/bar"); + }); + + jest.spyOn(io, "which").mockImplementation(tool => { + return Promise.resolve(tool); + }); +}); + +afterEach(() => { + testUtils.clearInputs(); +}); + +test("save with no primary key in state outputs warning", async () => { + const warningMock = jest.spyOn(core, "warning"); + const failedMock = jest.spyOn(core, "setFailed"); + + const cacheEntry: ArtifactCacheEntry = { + cacheKey: "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43", + scope: "refs/heads/master", + creationTime: "2019-11-13T19:18:02+00:00", + archiveLocation: "www.actionscache.test/download" + }; + + jest.spyOn(core, "getState") + // Cache Entry State + .mockImplementationOnce(() => { + return JSON.stringify(cacheEntry); + }) + // Cache Key State + .mockImplementationOnce(() => { + return ""; + }); + + await run(); + + expect(warningMock).toHaveBeenCalledWith( + `Error retrieving key from state.` + ); + expect(warningMock).toHaveBeenCalledTimes(1); + expect(failedMock).toHaveBeenCalledTimes(0); +}); + +test("save with exact match returns early", async () => { + const infoMock = jest.spyOn(core, "info"); + const warningMock = jest.spyOn(core, "warning"); + const failedMock = jest.spyOn(core, "setFailed"); + + const primaryKey = "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43"; + const cacheEntry: ArtifactCacheEntry = { + cacheKey: primaryKey, + scope: "refs/heads/master", + creationTime: "2019-11-13T19:18:02+00:00", + archiveLocation: "www.actionscache.test/download" + }; + + jest.spyOn(core, "getState") + // Cache Entry State + .mockImplementationOnce(() => { + return JSON.stringify(cacheEntry); + }) + // Cache Key State + .mockImplementationOnce(() => { + return primaryKey; + }); + + const execMock = jest.spyOn(exec, "exec"); + + await run(); + + expect(infoMock).toHaveBeenCalledWith( + `Cache hit occurred on the primary key ${primaryKey}, not saving cache.` + ); + + expect(execMock).toHaveBeenCalledTimes(0); + + expect(warningMock).toHaveBeenCalledTimes(0); + expect(failedMock).toHaveBeenCalledTimes(0); +}); + +test("save with missing input outputs warning", async () => { + const warningMock = jest.spyOn(core, "warning"); + const failedMock = jest.spyOn(core, "setFailed"); + + const primaryKey = "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43"; + const cacheEntry: ArtifactCacheEntry = { + cacheKey: "Linux-node-", + scope: "refs/heads/master", + creationTime: "2019-11-13T19:18:02+00:00", + archiveLocation: "www.actionscache.test/download" + }; + + jest.spyOn(core, "getState") + // Cache Entry State + .mockImplementationOnce(() => { + return JSON.stringify(cacheEntry); + }) + // Cache Key State + .mockImplementationOnce(() => { + return primaryKey; + }); + + await run(); + + expect(warningMock).toHaveBeenCalledWith( + "Input required and not supplied: path" + ); + expect(warningMock).toHaveBeenCalledTimes(1); + expect(failedMock).toHaveBeenCalledTimes(0); +}); + +test("save with large cache outputs warning", async () => { + const warningMock = jest.spyOn(core, "warning"); + const failedMock = jest.spyOn(core, "setFailed"); + + const primaryKey = "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43"; + const cacheEntry: ArtifactCacheEntry = { + cacheKey: "Linux-node-", + scope: "refs/heads/master", + creationTime: "2019-11-13T19:18:02+00:00", + archiveLocation: "www.actionscache.test/download" + }; + + jest.spyOn(core, "getState") + // Cache Entry State + .mockImplementationOnce(() => { + return JSON.stringify(cacheEntry); + }) + // Cache Key State + .mockImplementationOnce(() => { + return primaryKey; + }); + + const inputPath = "node_modules"; + const cachePath = path.resolve(inputPath); + testUtils.setInput(Inputs.Path, inputPath); + + const execMock = jest.spyOn(exec, "exec"); + + const cacheSize = 1024 * 1024 * 1024; //~1GB, over the 400MB limit + jest.spyOn(actionUtils, "getArchiveFileSize").mockImplementationOnce(() => { + return cacheSize; + }); + + await run(); + + const archivePath = path.join("/foo/bar", "cache.tgz"); + + const IS_WINDOWS = process.platform === "win32"; + const args = IS_WINDOWS + ? [ + "-cz", + "--force-local", + "-f", + archivePath.replace(/\\/g, "/"), + "-C", + cachePath.replace(/\\/g, "/"), + "." + ] + : ["-cz", "-f", archivePath, "-C", cachePath, "."]; + + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + + expect(warningMock).toHaveBeenCalledTimes(1); + expect(warningMock).toHaveBeenCalledWith( + "Cache size of ~1024 MB (1073741824 B) is over the 400MB limit, not saving cache." + ); + + expect(failedMock).toHaveBeenCalledTimes(0); +}); + +test("save with server error outputs warning", async () => { + const warningMock = jest.spyOn(core, "warning"); + const failedMock = jest.spyOn(core, "setFailed"); + + const primaryKey = "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43"; + const cacheEntry: ArtifactCacheEntry = { + cacheKey: "Linux-node-", + scope: "refs/heads/master", + creationTime: "2019-11-13T19:18:02+00:00", + archiveLocation: "www.actionscache.test/download" + }; + + jest.spyOn(core, "getState") + // Cache Entry State + .mockImplementationOnce(() => { + return JSON.stringify(cacheEntry); + }) + // Cache Key State + .mockImplementationOnce(() => { + return primaryKey; + }); + + const inputPath = "node_modules"; + const cachePath = path.resolve(inputPath); + testUtils.setInput(Inputs.Path, inputPath); + + const execMock = jest.spyOn(exec, "exec"); + + const saveCacheMock = jest + .spyOn(cacheHttpClient, "saveCache") + .mockImplementationOnce(() => { + throw new Error("HTTP Error Occurred"); + }); + + await run(); + + const archivePath = path.join("/foo/bar", "cache.tgz"); + + const IS_WINDOWS = process.platform === "win32"; + const args = IS_WINDOWS + ? [ + "-cz", + "--force-local", + "-f", + archivePath.replace(/\\/g, "/"), + "-C", + cachePath.replace(/\\/g, "/"), + "." + ] + : ["-cz", "-f", archivePath, "-C", cachePath, "."]; + + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + + expect(saveCacheMock).toHaveBeenCalledTimes(1); + expect(saveCacheMock).toHaveBeenCalledWith(primaryKey, archivePath); + + expect(warningMock).toHaveBeenCalledTimes(1); + expect(warningMock).toHaveBeenCalledWith("HTTP Error Occurred"); + + expect(failedMock).toHaveBeenCalledTimes(0); +}); + +test("save with valid inputs uploads a cache", async () => { + const warningMock = jest.spyOn(core, "warning"); + const failedMock = jest.spyOn(core, "setFailed"); + + const primaryKey = "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43"; + const cacheEntry: ArtifactCacheEntry = { + cacheKey: "Linux-node-", + scope: "refs/heads/master", + creationTime: "2019-11-13T19:18:02+00:00", + archiveLocation: "www.actionscache.test/download" + }; + + jest.spyOn(core, "getState") + // Cache Entry State + .mockImplementationOnce(() => { + return JSON.stringify(cacheEntry); + }) + // Cache Key State + .mockImplementationOnce(() => { + return primaryKey; + }); + + const inputPath = "node_modules"; + const cachePath = path.resolve(inputPath); + testUtils.setInput(Inputs.Path, inputPath); + + const execMock = jest.spyOn(exec, "exec"); + + const saveCacheMock = jest.spyOn(cacheHttpClient, "saveCache"); + + await run(); + + const archivePath = path.join("/foo/bar", "cache.tgz"); + + const IS_WINDOWS = process.platform === "win32"; + const args = IS_WINDOWS + ? [ + "-cz", + "--force-local", + "-f", + archivePath.replace(/\\/g, "/"), + "-C", + cachePath.replace(/\\/g, "/"), + "." + ] + : ["-cz", "-f", archivePath, "-C", cachePath, "."]; + + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith(`"tar"`, args); + + expect(saveCacheMock).toHaveBeenCalledTimes(1); + expect(saveCacheMock).toHaveBeenCalledWith(primaryKey, archivePath); + + expect(warningMock).toHaveBeenCalledTimes(0); + expect(failedMock).toHaveBeenCalledTimes(0); +}); diff --git a/src/cacheHttpClient.ts b/src/cacheHttpClient.ts index b3885e4..e448157 100644 --- a/src/cacheHttpClient.ts +++ b/src/cacheHttpClient.ts @@ -93,9 +93,11 @@ export async function downloadCache( } export async function saveCache( - stream: NodeJS.ReadableStream, - key: string + key: string, + archivePath: string ): Promise { + const stream = fs.createReadStream(archivePath); + const cacheUrl = getCacheUrl(); const token = process.env["ACTIONS_RUNTIME_TOKEN"] || ""; const bearerCredentialHandler = new BearerCredentialHandler(token); diff --git a/src/restore.ts b/src/restore.ts index 6936652..5d9da4a 100644 --- a/src/restore.ts +++ b/src/restore.ts @@ -19,7 +19,7 @@ async function run(): Promise { ); } - let cachePath = utils.resolvePath( + const cachePath = utils.resolvePath( core.getInput(Inputs.Path, { required: true }) ); core.debug(`Cache Path: ${cachePath}`); @@ -67,7 +67,7 @@ async function run(): Promise { return; } - let archivePath = path.join( + const archivePath = path.join( await utils.createTempDirectory(), "cache.tgz" ); @@ -90,15 +90,17 @@ async function run(): Promise { // http://man7.org/linux/man-pages/man1/tar.1.html // tar [-options] [files or directories which to add into archive] - const args = ["-xz"]; - const IS_WINDOWS = process.platform === "win32"; - if (IS_WINDOWS) { - args.push("--force-local"); - archivePath = archivePath.replace(/\\/g, "/"); - cachePath = cachePath.replace(/\\/g, "/"); - } - args.push(...["-f", archivePath, "-C", cachePath]); + const args = IS_WINDOWS + ? [ + "-xz", + "--force-local", + "-f", + archivePath.replace(/\\/g, "/"), + "-C", + cachePath.replace(/\\/g, "/") + ] + : ["-xz", "-f", archivePath, "-C", cachePath]; const tarPath = await io.which("tar", true); core.debug(`Tar Path: ${tarPath}`); diff --git a/src/save.ts b/src/save.ts index c4d5d61..d660064 100644 --- a/src/save.ts +++ b/src/save.ts @@ -1,7 +1,6 @@ import * as core from "@actions/core"; import { exec } from "@actions/exec"; import * as io from "@actions/io"; -import * as fs from "fs"; import * as path from "path"; import * as cacheHttpClient from "./cacheHttpClient"; import { Inputs, State } from "./constants"; @@ -25,12 +24,12 @@ async function run(): Promise { return; } - let cachePath = utils.resolvePath( + const cachePath = utils.resolvePath( core.getInput(Inputs.Path, { required: true }) ); core.debug(`Cache Path: ${cachePath}`); - let archivePath = path.join( + const archivePath = path.join( await utils.createTempDirectory(), "cache.tgz" ); @@ -38,22 +37,25 @@ async function run(): Promise { // http://man7.org/linux/man-pages/man1/tar.1.html // tar [-options] [files or directories which to add into archive] - const args = ["-cz"]; const IS_WINDOWS = process.platform === "win32"; - if (IS_WINDOWS) { - args.push("--force-local"); - archivePath = archivePath.replace(/\\/g, "/"); - cachePath = cachePath.replace(/\\/g, "/"); - } - - args.push(...["-f", archivePath, "-C", cachePath, "."]); + const args = IS_WINDOWS + ? [ + "-cz", + "--force-local", + "-f", + archivePath.replace(/\\/g, "/"), + "-C", + cachePath.replace(/\\/g, "/"), + "." + ] + : ["-cz", "-f", archivePath, "-C", cachePath, "."]; const tarPath = await io.which("tar", true); core.debug(`Tar Path: ${tarPath}`); await exec(`"${tarPath}"`, args); const fileSizeLimit = 400 * 1024 * 1024; // 400MB - const archiveFileSize = fs.statSync(archivePath).size; + const archiveFileSize = utils.getArchiveFileSize(archivePath); core.debug(`File Size: ${archiveFileSize}`); if (archiveFileSize > fileSizeLimit) { core.warning( @@ -64,8 +66,7 @@ async function run(): Promise { return; } - const stream = fs.createReadStream(archivePath); - await cacheHttpClient.saveCache(stream, primaryKey); + await cacheHttpClient.saveCache(primaryKey, archivePath); } catch (error) { core.warning(error.message); }