From c48b49d8ee0791efa0a72ba6dcce5f0e46c4e0bb Mon Sep 17 00:00:00 2001
From: Josh Gross <joshmgross@github.com>
Date: Wed, 20 Nov 2019 13:19:40 -0500
Subject: [PATCH] Update warnings behavior
---
__tests__/actionUtils.test.ts | 10 ++++++
__tests__/restore.test.ts | 22 +++++---------
__tests__/save.test.ts | 57 ++++++++++++++++++++++++-----------
package-lock.json | 2 +-
package.json | 2 +-
src/restore.ts | 5 +--
src/save.ts | 19 +++++++++---
src/utils/actionUtils.ts | 5 +++
8 files changed, 82 insertions(+), 40 deletions(-)
diff --git a/__tests__/actionUtils.test.ts b/__tests__/actionUtils.test.ts
index 4688b5d..f46d65d 100644
--- a/__tests__/actionUtils.test.ts
+++ b/__tests__/actionUtils.test.ts
@@ -162,6 +162,16 @@ test("getCacheState with valid state", () => {
expect(getStateMock).toHaveBeenCalledTimes(1);
});
+test("logWarning logs a message with a warning prefix", () => {
+ const message = "A warning occurred.";
+
+ const infoMock = jest.spyOn(core, "info");
+
+ actionUtils.logWarning(message);
+
+ expect(infoMock).toHaveBeenCalledWith(`[warning]${message}`);
+});
+
test("isValidEvent returns false for unknown event", () => {
const event = "foo";
process.env[Events.Key] = event;
diff --git a/__tests__/restore.test.ts b/__tests__/restore.test.ts
index 1919e30..78b00ec 100644
--- a/__tests__/restore.test.ts
+++ b/__tests__/restore.test.ts
@@ -50,14 +50,16 @@ afterEach(() => {
delete process.env[Events.Key];
});
-test("restore with invalid event", async () => {
+test("restore with invalid event outputs warning", async () => {
+ const logWarningMock = jest.spyOn(actionUtils, "logWarning");
const failedMock = jest.spyOn(core, "setFailed");
const invalidEvent = "commit_comment";
process.env[Events.Key] = invalidEvent;
await run();
- expect(failedMock).toHaveBeenCalledWith(
+ expect(logWarningMock).toHaveBeenCalledWith(
`Event Validation Error: The event type ${invalidEvent} is not supported. Only push, pull_request events are supported at this time.`
);
+ expect(failedMock).toHaveBeenCalledTimes(0);
});
test("restore with no path should fail", async () => {
@@ -126,7 +128,6 @@ test("restore with no cache found", async () => {
});
const infoMock = jest.spyOn(core, "info");
- const warningMock = jest.spyOn(core, "warning");
const failedMock = jest.spyOn(core, "setFailed");
const stateMock = jest.spyOn(core, "saveState");
@@ -138,7 +139,6 @@ test("restore with no cache found", async () => {
await run();
expect(stateMock).toHaveBeenCalledWith("CACHE_KEY", key);
- expect(warningMock).toHaveBeenCalledTimes(0);
expect(failedMock).toHaveBeenCalledTimes(0);
expect(infoMock).toHaveBeenCalledWith(
@@ -153,7 +153,7 @@ test("restore with server error should fail", async () => {
key
});
- const warningMock = jest.spyOn(core, "warning");
+ const logWarningMock = jest.spyOn(actionUtils, "logWarning");
const failedMock = jest.spyOn(core, "setFailed");
const stateMock = jest.spyOn(core, "saveState");
@@ -168,8 +168,8 @@ test("restore with server error should fail", async () => {
expect(stateMock).toHaveBeenCalledWith("CACHE_KEY", key);
- expect(warningMock).toHaveBeenCalledTimes(1);
- expect(warningMock).toHaveBeenCalledWith("HTTP Error Occurred");
+ expect(logWarningMock).toHaveBeenCalledTimes(1);
+ expect(logWarningMock).toHaveBeenCalledWith("HTTP Error Occurred");
expect(setCacheHitOutputMock).toHaveBeenCalledTimes(1);
expect(setCacheHitOutputMock).toHaveBeenCalledWith(false);
@@ -187,7 +187,6 @@ test("restore with restore keys and no cache found", async () => {
});
const infoMock = jest.spyOn(core, "info");
- const warningMock = jest.spyOn(core, "warning");
const failedMock = jest.spyOn(core, "setFailed");
const stateMock = jest.spyOn(core, "saveState");
@@ -199,7 +198,6 @@ test("restore with restore keys and no cache found", async () => {
await run();
expect(stateMock).toHaveBeenCalledWith("CACHE_KEY", key);
- expect(warningMock).toHaveBeenCalledTimes(0);
expect(failedMock).toHaveBeenCalledTimes(0);
expect(infoMock).toHaveBeenCalledWith(
@@ -216,7 +214,6 @@ test("restore with cache found", async () => {
});
const infoMock = jest.spyOn(core, "info");
- const warningMock = jest.spyOn(core, "warning");
const failedMock = jest.spyOn(core, "setFailed");
const stateMock = jest.spyOn(core, "saveState");
@@ -281,7 +278,6 @@ test("restore with cache found", async () => {
expect(setCacheHitOutputMock).toHaveBeenCalledWith(true);
expect(infoMock).toHaveBeenCalledWith(`Cache restored from key: ${key}`);
- expect(warningMock).toHaveBeenCalledTimes(0);
expect(failedMock).toHaveBeenCalledTimes(0);
});
@@ -296,7 +292,6 @@ test("restore with a pull request event and cache found", async () => {
process.env[Events.Key] = Events.PullRequest;
const infoMock = jest.spyOn(core, "info");
- const warningMock = jest.spyOn(core, "warning");
const failedMock = jest.spyOn(core, "setFailed");
const stateMock = jest.spyOn(core, "saveState");
@@ -362,7 +357,6 @@ test("restore with a pull request event and cache found", async () => {
expect(setCacheHitOutputMock).toHaveBeenCalledWith(true);
expect(infoMock).toHaveBeenCalledWith(`Cache restored from key: ${key}`);
- expect(warningMock).toHaveBeenCalledTimes(0);
expect(failedMock).toHaveBeenCalledTimes(0);
});
@@ -377,7 +371,6 @@ test("restore with cache found for restore key", async () => {
});
const infoMock = jest.spyOn(core, "info");
- const warningMock = jest.spyOn(core, "warning");
const failedMock = jest.spyOn(core, "setFailed");
const stateMock = jest.spyOn(core, "saveState");
@@ -445,6 +438,5 @@ test("restore with cache found for restore key", async () => {
expect(infoMock).toHaveBeenCalledWith(
`Cache restored from key: ${restoreKey}`
);
- expect(warningMock).toHaveBeenCalledTimes(0);
expect(failedMock).toHaveBeenCalledTimes(0);
});
diff --git a/__tests__/save.test.ts b/__tests__/save.test.ts
index 67b13d2..89657c4 100644
--- a/__tests__/save.test.ts
+++ b/__tests__/save.test.ts
@@ -3,7 +3,7 @@ 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 { Events, Inputs } from "../src/constants";
import { ArtifactCacheEntry } from "../src/contracts";
import run from "../src/save";
import * as actionUtils from "../src/utils/actionUtils";
@@ -32,6 +32,16 @@ beforeAll(() => {
}
);
+ jest.spyOn(actionUtils, "isValidEvent").mockImplementation(() => {
+ const actualUtils = jest.requireActual("../src/utils/actionUtils");
+ return actualUtils.isValidEvent();
+ });
+
+ jest.spyOn(actionUtils, "getSupportedEvents").mockImplementation(() => {
+ const actualUtils = jest.requireActual("../src/utils/actionUtils");
+ return actualUtils.getSupportedEvents();
+ });
+
jest.spyOn(actionUtils, "resolvePath").mockImplementation(filePath => {
return path.resolve(filePath);
});
@@ -45,12 +55,29 @@ beforeAll(() => {
});
});
+beforeEach(() => {
+ process.env[Events.Key] = Events.Push;
+});
+
afterEach(() => {
testUtils.clearInputs();
+ delete process.env[Events.Key];
+});
+
+test("save with invalid event outputs warning", async () => {
+ const logWarningMock = jest.spyOn(actionUtils, "logWarning");
+ const failedMock = jest.spyOn(core, "setFailed");
+ const invalidEvent = "commit_comment";
+ process.env[Events.Key] = invalidEvent;
+ await run();
+ expect(logWarningMock).toHaveBeenCalledWith(
+ `Event Validation Error: The event type ${invalidEvent} is not supported. Only push, pull_request events are supported at this time.`
+ );
+ expect(failedMock).toHaveBeenCalledTimes(0);
});
test("save with no primary key in state outputs warning", async () => {
- const warningMock = jest.spyOn(core, "warning");
+ const logWarningMock = jest.spyOn(actionUtils, "logWarning");
const failedMock = jest.spyOn(core, "setFailed");
const cacheEntry: ArtifactCacheEntry = {
@@ -72,16 +99,15 @@ test("save with no primary key in state outputs warning", async () => {
await run();
- expect(warningMock).toHaveBeenCalledWith(
+ expect(logWarningMock).toHaveBeenCalledWith(
`Error retrieving key from state.`
);
- expect(warningMock).toHaveBeenCalledTimes(1);
+ expect(logWarningMock).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";
@@ -112,12 +138,11 @@ test("save with exact match returns early", async () => {
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 logWarningMock = jest.spyOn(actionUtils, "logWarning");
const failedMock = jest.spyOn(core, "setFailed");
const primaryKey = "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43";
@@ -140,15 +165,15 @@ test("save with missing input outputs warning", async () => {
await run();
- expect(warningMock).toHaveBeenCalledWith(
+ expect(logWarningMock).toHaveBeenCalledWith(
"Input required and not supplied: path"
);
- expect(warningMock).toHaveBeenCalledTimes(1);
+ expect(logWarningMock).toHaveBeenCalledTimes(1);
expect(failedMock).toHaveBeenCalledTimes(0);
});
test("save with large cache outputs warning", async () => {
- const warningMock = jest.spyOn(core, "warning");
+ const logWarningMock = jest.spyOn(actionUtils, "logWarning");
const failedMock = jest.spyOn(core, "setFailed");
const primaryKey = "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43";
@@ -200,8 +225,8 @@ test("save with large cache outputs warning", async () => {
expect(execMock).toHaveBeenCalledTimes(1);
expect(execMock).toHaveBeenCalledWith(`"tar"`, args);
- expect(warningMock).toHaveBeenCalledTimes(1);
- expect(warningMock).toHaveBeenCalledWith(
+ expect(logWarningMock).toHaveBeenCalledTimes(1);
+ expect(logWarningMock).toHaveBeenCalledWith(
"Cache size of ~1024 MB (1073741824 B) is over the 400MB limit, not saving cache."
);
@@ -209,7 +234,7 @@ test("save with large cache outputs warning", async () => {
});
test("save with server error outputs warning", async () => {
- const warningMock = jest.spyOn(core, "warning");
+ const logWarningMock = jest.spyOn(actionUtils, "logWarning");
const failedMock = jest.spyOn(core, "setFailed");
const primaryKey = "Linux-node-bb828da54c148048dd17899ba9fda624811cfb43";
@@ -265,14 +290,13 @@ test("save with server error outputs warning", async () => {
expect(saveCacheMock).toHaveBeenCalledTimes(1);
expect(saveCacheMock).toHaveBeenCalledWith(primaryKey, archivePath);
- expect(warningMock).toHaveBeenCalledTimes(1);
- expect(warningMock).toHaveBeenCalledWith("HTTP Error Occurred");
+ expect(logWarningMock).toHaveBeenCalledTimes(1);
+ expect(logWarningMock).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";
@@ -324,6 +348,5 @@ test("save with valid inputs uploads a cache", async () => {
expect(saveCacheMock).toHaveBeenCalledTimes(1);
expect(saveCacheMock).toHaveBeenCalledWith(primaryKey, archivePath);
- expect(warningMock).toHaveBeenCalledTimes(0);
expect(failedMock).toHaveBeenCalledTimes(0);
});
diff --git a/package-lock.json b/package-lock.json
index 9821cb1..2e8413e 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
{
"name": "cache",
- "version": "1.0.2",
+ "version": "1.0.3",
"lockfileVersion": 1,
"requires": true,
"dependencies": {
diff --git a/package.json b/package.json
index dd09d47..42fbdbe 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
{
"name": "cache",
- "version": "1.0.2",
+ "version": "1.0.3",
"private": true,
"description": "Cache dependencies and build outputs",
"main": "dist/restore/index.js",
diff --git a/src/restore.ts b/src/restore.ts
index 87a2684..15570cd 100644
--- a/src/restore.ts
+++ b/src/restore.ts
@@ -10,13 +10,14 @@ async function run(): Promise<void> {
try {
// Validate inputs, this can cause task failure
if (!utils.isValidEvent()) {
- core.setFailed(
+ utils.logWarning(
`Event Validation Error: The event type ${
process.env[Events.Key]
} is not supported. Only ${utils
.getSupportedEvents()
.join(", ")} events are supported at this time.`
);
+ return;
}
const cachePath = utils.resolvePath(
@@ -118,7 +119,7 @@ async function run(): Promise<void> {
`Cache restored from key: ${cacheEntry && cacheEntry.cacheKey}`
);
} catch (error) {
- core.warning(error.message);
+ utils.logWarning(error.message);
utils.setCacheHitOutput(false);
}
} catch (error) {
diff --git a/src/save.ts b/src/save.ts
index d660064..21f32d3 100644
--- a/src/save.ts
+++ b/src/save.ts
@@ -3,17 +3,28 @@ import { exec } from "@actions/exec";
import * as io from "@actions/io";
import * as path from "path";
import * as cacheHttpClient from "./cacheHttpClient";
-import { Inputs, State } from "./constants";
+import { Events, Inputs, State } from "./constants";
import * as utils from "./utils/actionUtils";
async function run(): Promise<void> {
try {
+ if (!utils.isValidEvent()) {
+ utils.logWarning(
+ `Event Validation Error: The event type ${
+ process.env[Events.Key]
+ } is not supported. Only ${utils
+ .getSupportedEvents()
+ .join(", ")} events are supported at this time.`
+ );
+ return;
+ }
+
const state = utils.getCacheState();
// Inputs are re-evaluted before the post action, so we want the original key used for restore
const primaryKey = core.getState(State.CacheKey);
if (!primaryKey) {
- core.warning(`Error retrieving key from state.`);
+ utils.logWarning(`Error retrieving key from state.`);
return;
}
@@ -58,7 +69,7 @@ async function run(): Promise<void> {
const archiveFileSize = utils.getArchiveFileSize(archivePath);
core.debug(`File Size: ${archiveFileSize}`);
if (archiveFileSize > fileSizeLimit) {
- core.warning(
+ utils.logWarning(
`Cache size of ~${Math.round(
archiveFileSize / (1024 * 1024)
)} MB (${archiveFileSize} B) is over the 400MB limit, not saving cache.`
@@ -68,7 +79,7 @@ async function run(): Promise<void> {
await cacheHttpClient.saveCache(primaryKey, archivePath);
} catch (error) {
- core.warning(error.message);
+ utils.logWarning(error.message);
}
}
diff --git a/src/utils/actionUtils.ts b/src/utils/actionUtils.ts
index ba5b2ea..c97d94f 100644
--- a/src/utils/actionUtils.ts
+++ b/src/utils/actionUtils.ts
@@ -77,6 +77,11 @@ export function getCacheState(): ArtifactCacheEntry | undefined {
return undefined;
}
+export function logWarning(message: string) {
+ const warningPrefix = "[warning]";
+ core.info(`${warningPrefix}${message}`);
+}
+
export function resolvePath(filePath: string): string {
if (filePath[0] === "~") {
const home = os.homedir();