Remove validation failures and warning annotations (#108)

* Update warnings behavior

* Add void return type

__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;

__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);
 });

__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);
 });

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "cache",
-  "version": "1.0.2",
+  "version": "1.0.3",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

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",

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) {

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);
     }
 }
 

src/utils/actionUtils.ts

@@ -77,6 +77,11 @@ export function getCacheState(): ArtifactCacheEntry | undefined {
     return undefined;
 }
 
+export function logWarning(message: string): void {
+    const warningPrefix = "[warning]";
+    core.info(`${warningPrefix}${message}`);
+}
+
 export function resolvePath(filePath: string): string {
     if (filePath[0] === "~") {
         const home = os.homedir();