From a6f1f4b32eec85780fedc5b354a583e9b2999100 Mon Sep 17 00:00:00 2001 From: Dave Hadka Date: Fri, 2 Oct 2020 09:59:55 -0500 Subject: [PATCH 1/4] Adds input for upload chunk size --- __tests__/actionUtils.test.ts | 15 +++++++++++++++ __tests__/save.test.ts | 31 +++++++++++++++++++++++++++---- dist/restore/index.js | 11 ++++++++++- dist/save/index.js | 15 +++++++++++++-- package-lock.json | 2 +- package.json | 2 +- src/constants.ts | 3 ++- src/save.ts | 4 +++- src/utils/actionUtils.ts | 11 +++++++++++ src/utils/testUtils.ts | 1 + 10 files changed, 84 insertions(+), 11 deletions(-) diff --git a/__tests__/actionUtils.test.ts b/__tests__/actionUtils.test.ts index 7cd935c..1eed4e8 100644 --- a/__tests__/actionUtils.test.ts +++ b/__tests__/actionUtils.test.ts @@ -15,6 +15,7 @@ beforeAll(() => { afterEach(() => { delete process.env[Events.Key]; delete process.env[RefKey]; + testUtils.clearInputs(); }); test("isGhes returns true if server url is not github.com", () => { @@ -212,3 +213,17 @@ test("getInputAsArray handles empty lines correctly", () => { testUtils.setInput("foo", "\n\nbar\n\nbaz\n\n"); expect(actionUtils.getInputAsArray("foo")).toEqual(["bar", "baz"]); }); + +test("getInputAsInt returns undefined if input not set", () => { + expect(actionUtils.getInputAsInt("foo")).toBeUndefined(); +}); + +test("getInputAsInt returns value if input is valid", () => { + testUtils.setInput("foo", "8"); + expect(actionUtils.getInputAsInt("foo")).toBe(8); +}); + +test("getInputAsInt returns undefined if input is invalid or NaN", () => { + testUtils.setInput("foo", "bar"); + expect(actionUtils.getInputAsInt("foo")).toBeUndefined(); +}); diff --git a/__tests__/save.test.ts b/__tests__/save.test.ts index ac9d96d..b806d50 100644 --- a/__tests__/save.test.ts +++ b/__tests__/save.test.ts @@ -27,6 +27,14 @@ beforeAll(() => { } ); + jest.spyOn(actionUtils, "getInputAsInt").mockImplementation( + (name, options) => { + return jest + .requireActual("../src/utils/actionUtils") + .getInputAsInt(name, options); + } + ); + jest.spyOn(actionUtils, "isExactKeyMatch").mockImplementation( (key, cacheResult) => { return jest @@ -193,7 +201,11 @@ test("save with large cache outputs warning", async () => { await run(); expect(saveCacheMock).toHaveBeenCalledTimes(1); - expect(saveCacheMock).toHaveBeenCalledWith([inputPath], primaryKey); + expect(saveCacheMock).toHaveBeenCalledWith( + [inputPath], + primaryKey, + expect.anything() + ); expect(logWarningMock).toHaveBeenCalledTimes(1); expect(logWarningMock).toHaveBeenCalledWith( @@ -236,7 +248,11 @@ test("save with reserve cache failure outputs warning", async () => { await run(); expect(saveCacheMock).toHaveBeenCalledTimes(1); - expect(saveCacheMock).toHaveBeenCalledWith([inputPath], primaryKey); + expect(saveCacheMock).toHaveBeenCalledWith( + [inputPath], + primaryKey, + expect.anything() + ); expect(infoMock).toHaveBeenCalledWith( `Unable to reserve cache with key ${primaryKey}, another job may be creating this cache.` @@ -274,7 +290,11 @@ test("save with server error outputs warning", async () => { await run(); expect(saveCacheMock).toHaveBeenCalledTimes(1); - expect(saveCacheMock).toHaveBeenCalledWith([inputPath], primaryKey); + expect(saveCacheMock).toHaveBeenCalledWith( + [inputPath], + primaryKey, + expect.anything() + ); expect(logWarningMock).toHaveBeenCalledTimes(1); expect(logWarningMock).toHaveBeenCalledWith("HTTP Error Occurred"); @@ -300,6 +320,7 @@ test("save with valid inputs uploads a cache", async () => { const inputPath = "node_modules"; testUtils.setInput(Inputs.Path, inputPath); + testUtils.setInput(Inputs.UploadChunkSize, "4000000"); const cacheId = 4; const saveCacheMock = jest @@ -311,7 +332,9 @@ test("save with valid inputs uploads a cache", async () => { await run(); expect(saveCacheMock).toHaveBeenCalledTimes(1); - expect(saveCacheMock).toHaveBeenCalledWith([inputPath], primaryKey); + expect(saveCacheMock).toHaveBeenCalledWith([inputPath], primaryKey, { + uploadChunkSize: 4000000 + }); expect(failedMock).toHaveBeenCalledTimes(0); }); diff --git a/dist/restore/index.js b/dist/restore/index.js index 373baa0..e7ea6bc 100644 --- a/dist/restore/index.js +++ b/dist/restore/index.js @@ -31296,7 +31296,7 @@ var __importStar = (this && this.__importStar) || function (mod) { return result; }; Object.defineProperty(exports, "__esModule", { value: true }); -exports.getInputAsArray = exports.isValidEvent = exports.logWarning = exports.getCacheState = exports.setOutputAndState = exports.setCacheHitOutput = exports.setCacheState = exports.isExactKeyMatch = exports.isGhes = void 0; +exports.getInputAsInt = exports.getInputAsArray = exports.isValidEvent = exports.logWarning = exports.getCacheState = exports.setOutputAndState = exports.setCacheHitOutput = exports.setCacheState = exports.isExactKeyMatch = exports.isGhes = void 0; const core = __importStar(__webpack_require__(470)); const constants_1 = __webpack_require__(694); function isGhes() { @@ -31353,6 +31353,14 @@ function getInputAsArray(name, options) { .filter(x => x !== ""); } exports.getInputAsArray = getInputAsArray; +function getInputAsInt(name, options) { + const value = Number(core.getInput(name, options)); + if (Number.isNaN(value) || value < 0) { + return undefined; + } + return value; +} +exports.getInputAsInt = getInputAsInt; /***/ }), @@ -38485,6 +38493,7 @@ var Inputs; Inputs["Key"] = "key"; Inputs["Path"] = "path"; Inputs["RestoreKeys"] = "restore-keys"; + Inputs["UploadChunkSize"] = "upload-chunk-size"; })(Inputs = exports.Inputs || (exports.Inputs = {})); var Outputs; (function (Outputs) { diff --git a/dist/save/index.js b/dist/save/index.js index e9d765d..175ab6f 100644 --- a/dist/save/index.js +++ b/dist/save/index.js @@ -31296,7 +31296,7 @@ var __importStar = (this && this.__importStar) || function (mod) { return result; }; Object.defineProperty(exports, "__esModule", { value: true }); -exports.getInputAsArray = exports.isValidEvent = exports.logWarning = exports.getCacheState = exports.setOutputAndState = exports.setCacheHitOutput = exports.setCacheState = exports.isExactKeyMatch = exports.isGhes = void 0; +exports.getInputAsInt = exports.getInputAsArray = exports.isValidEvent = exports.logWarning = exports.getCacheState = exports.setOutputAndState = exports.setCacheHitOutput = exports.setCacheState = exports.isExactKeyMatch = exports.isGhes = void 0; const core = __importStar(__webpack_require__(470)); const constants_1 = __webpack_require__(694); function isGhes() { @@ -31353,6 +31353,14 @@ function getInputAsArray(name, options) { .filter(x => x !== ""); } exports.getInputAsArray = getInputAsArray; +function getInputAsInt(name, options) { + const value = Number(core.getInput(name, options)); + if (Number.isNaN(value) || value < 0) { + return undefined; + } + return value; +} +exports.getInputAsInt = getInputAsInt; /***/ }), @@ -38353,7 +38361,9 @@ function run() { required: true }); try { - yield cache.saveCache(cachePaths, primaryKey); + yield cache.saveCache(cachePaths, primaryKey, { + uploadChunkSize: utils.getInputAsInt(constants_1.Inputs.UploadChunkSize) + }); } catch (error) { if (error.name === cache.ValidationError.name) { @@ -38574,6 +38584,7 @@ var Inputs; Inputs["Key"] = "key"; Inputs["Path"] = "path"; Inputs["RestoreKeys"] = "restore-keys"; + Inputs["UploadChunkSize"] = "upload-chunk-size"; })(Inputs = exports.Inputs || (exports.Inputs = {})); var Outputs; (function (Outputs) { diff --git a/package-lock.json b/package-lock.json index fafa5e1..2af5c86 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "cache", - "version": "2.1.1", + "version": "2.1.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index db6f22e..e4564ed 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cache", - "version": "2.1.1", + "version": "2.1.2", "private": true, "description": "Cache dependencies and build outputs", "main": "dist/restore/index.js", diff --git a/src/constants.ts b/src/constants.ts index a190e00..133f47d 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,7 +1,8 @@ export enum Inputs { Key = "key", Path = "path", - RestoreKeys = "restore-keys" + RestoreKeys = "restore-keys", + UploadChunkSize = "upload-chunk-size" } export enum Outputs { diff --git a/src/save.ts b/src/save.ts index 32e9f2f..1efbc24 100644 --- a/src/save.ts +++ b/src/save.ts @@ -41,7 +41,9 @@ async function run(): Promise { }); try { - await cache.saveCache(cachePaths, primaryKey); + await cache.saveCache(cachePaths, primaryKey, { + uploadChunkSize: utils.getInputAsInt(Inputs.UploadChunkSize) + }); } catch (error) { if (error.name === cache.ValidationError.name) { throw error; diff --git a/src/utils/actionUtils.ts b/src/utils/actionUtils.ts index 017d357..5f975ec 100644 --- a/src/utils/actionUtils.ts +++ b/src/utils/actionUtils.ts @@ -63,3 +63,14 @@ export function getInputAsArray( .map(s => s.trim()) .filter(x => x !== ""); } + +export function getInputAsInt( + name: string, + options?: core.InputOptions +): number | undefined { + const value = Number(core.getInput(name, options)); + if (Number.isNaN(value) || value < 0) { + return undefined; + } + return value; +} diff --git a/src/utils/testUtils.ts b/src/utils/testUtils.ts index b1d20d0..9e2134f 100644 --- a/src/utils/testUtils.ts +++ b/src/utils/testUtils.ts @@ -26,4 +26,5 @@ export function clearInputs(): void { delete process.env[getInputName(Inputs.Path)]; delete process.env[getInputName(Inputs.Key)]; delete process.env[getInputName(Inputs.RestoreKeys)]; + delete process.env[getInputName(Inputs.UploadChunkSize)]; } From 4bceb75b5b7743784c63c94b81c50a485cbdcda0 Mon Sep 17 00:00:00 2001 From: Dave Hadka Date: Fri, 2 Oct 2020 10:55:30 -0500 Subject: [PATCH 2/4] Use parseInt instead of Number to handle empty strings --- __tests__/actionUtils.test.ts | 9 +++++++-- dist/restore/index.js | 4 ++-- dist/save/index.js | 4 ++-- src/utils/actionUtils.ts | 4 ++-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/__tests__/actionUtils.test.ts b/__tests__/actionUtils.test.ts index 1eed4e8..419881e 100644 --- a/__tests__/actionUtils.test.ts +++ b/__tests__/actionUtils.test.ts @@ -15,7 +15,6 @@ beforeAll(() => { afterEach(() => { delete process.env[Events.Key]; delete process.env[RefKey]; - testUtils.clearInputs(); }); test("isGhes returns true if server url is not github.com", () => { @@ -215,7 +214,7 @@ test("getInputAsArray handles empty lines correctly", () => { }); test("getInputAsInt returns undefined if input not set", () => { - expect(actionUtils.getInputAsInt("foo")).toBeUndefined(); + expect(actionUtils.getInputAsInt("undefined")).toBeUndefined(); }); test("getInputAsInt returns value if input is valid", () => { @@ -227,3 +226,9 @@ test("getInputAsInt returns undefined if input is invalid or NaN", () => { testUtils.setInput("foo", "bar"); expect(actionUtils.getInputAsInt("foo")).toBeUndefined(); }); + +test("getInputAsInt throws if required and value missing", () => { + expect(() => + actionUtils.getInputAsInt("undefined", { required: true }) + ).toThrowError(); +}); diff --git a/dist/restore/index.js b/dist/restore/index.js index e7ea6bc..87269d9 100644 --- a/dist/restore/index.js +++ b/dist/restore/index.js @@ -31354,8 +31354,8 @@ function getInputAsArray(name, options) { } exports.getInputAsArray = getInputAsArray; function getInputAsInt(name, options) { - const value = Number(core.getInput(name, options)); - if (Number.isNaN(value) || value < 0) { + const value = parseInt(core.getInput(name, options)); + if (isNaN(value) || value < 0) { return undefined; } return value; diff --git a/dist/save/index.js b/dist/save/index.js index 175ab6f..dda81de 100644 --- a/dist/save/index.js +++ b/dist/save/index.js @@ -31354,8 +31354,8 @@ function getInputAsArray(name, options) { } exports.getInputAsArray = getInputAsArray; function getInputAsInt(name, options) { - const value = Number(core.getInput(name, options)); - if (Number.isNaN(value) || value < 0) { + const value = parseInt(core.getInput(name, options)); + if (isNaN(value) || value < 0) { return undefined; } return value; diff --git a/src/utils/actionUtils.ts b/src/utils/actionUtils.ts index 5f975ec..a4d712d 100644 --- a/src/utils/actionUtils.ts +++ b/src/utils/actionUtils.ts @@ -68,8 +68,8 @@ export function getInputAsInt( name: string, options?: core.InputOptions ): number | undefined { - const value = Number(core.getInput(name, options)); - if (Number.isNaN(value) || value < 0) { + const value = parseInt(core.getInput(name, options)); + if (isNaN(value) || value < 0) { return undefined; } return value; From cce3c03a74623545a53c433d301f3f7725c72454 Mon Sep 17 00:00:00 2001 From: Dave Hadka Date: Fri, 2 Oct 2020 11:01:24 -0500 Subject: [PATCH 3/4] Add new input to action.yml --- action.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/action.yml b/action.yml index e50c38b..0ba67bc 100644 --- a/action.yml +++ b/action.yml @@ -11,6 +11,9 @@ inputs: restore-keys: description: 'An ordered list of keys to use for restoring the cache if no cache hit occurred for key' required: false + upload-chunk-size: + description: 'The chunk size used when uploading large cache files' + required: false outputs: cache-hit: description: 'A boolean value to indicate an exact match was found for the primary key' From 68cfb2ccb73b1982be3fa55e3d7c842697d7f1ed Mon Sep 17 00:00:00 2001 From: Dave Hadka Date: Fri, 2 Oct 2020 11:22:20 -0500 Subject: [PATCH 4/4] Add units to description --- action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/action.yml b/action.yml index 0ba67bc..323a4bd 100644 --- a/action.yml +++ b/action.yml @@ -12,7 +12,7 @@ inputs: description: 'An ordered list of keys to use for restoring the cache if no cache hit occurred for key' required: false upload-chunk-size: - description: 'The chunk size used when uploading large cache files' + description: 'The chunk size used to split up large files during upload, in bytes' required: false outputs: cache-hit: