Skip to content

Commit

Permalink
Merge pull request #270 from actions/users/aiyan/zstd
Browse files Browse the repository at this point in the history
Prefer zstd over gzip
  • Loading branch information
Aiqiao Yan authored and GitHub committed May 4, 2020
2 parents 9ceee97 + 75cd46e commit 9eb452c
Show file tree
Hide file tree
Showing 13 changed files with 528 additions and 193 deletions.
22 changes: 20 additions & 2 deletions __tests__/cacheHttpsClient.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { getCacheVersion } from "../src/cacheHttpClient";
import { Inputs } from "../src/constants";
import { CompressionMethod, Inputs } from "../src/constants";
import * as testUtils from "../src/utils/testUtils";

afterEach(() => {
testUtils.clearInputs();
});

test("getCacheVersion with path input returns version", async () => {
test("getCacheVersion with path input and compression method undefined returns version", async () => {
testUtils.setInput(Inputs.Path, "node_modules");

const result = getCacheVersion();
Expand All @@ -16,6 +16,24 @@ test("getCacheVersion with path input returns version", async () => {
);
});

test("getCacheVersion with zstd compression returns version", async () => {
testUtils.setInput(Inputs.Path, "node_modules");
const result = getCacheVersion(CompressionMethod.Zstd);

expect(result).toEqual(
"273877e14fd65d270b87a198edbfa2db5a43de567c9a548d2a2505b408befe24"
);
});

test("getCacheVersion with gzip compression does not change vesion", async () => {
testUtils.setInput(Inputs.Path, "node_modules");
const result = getCacheVersion(CompressionMethod.Gzip);

expect(result).toEqual(
"b3e0c6cb5ecf32614eeb2997d905b9c297046d7cbf69062698f25b14b4cb0985"
);
});

test("getCacheVersion with no input throws", async () => {
expect(() => getCacheVersion()).toThrow();
});
56 changes: 44 additions & 12 deletions __tests__/restore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import * as core from "@actions/core";
import * as path from "path";

import * as cacheHttpClient from "../src/cacheHttpClient";
import { Events, Inputs } from "../src/constants";
import {
CacheFilename,
CompressionMethod,
Events,
Inputs
} from "../src/constants";
import { ArtifactCacheEntry } from "../src/contracts";
import run from "../src/restore";
import * as tar from "../src/tar";
Expand Down Expand Up @@ -30,6 +35,11 @@ beforeAll(() => {
const actualUtils = jest.requireActual("../src/utils/actionUtils");
return actualUtils.getSupportedEvents();
});

jest.spyOn(actionUtils, "getCacheFileName").mockImplementation(cm => {
const actualUtils = jest.requireActual("../src/utils/actionUtils");
return actualUtils.getCacheFileName(cm);
});
});

beforeEach(() => {
Expand Down Expand Up @@ -197,7 +207,7 @@ test("restore with restore keys and no cache found", async () => {
);
});

test("restore with cache found", async () => {
test("restore with gzip compressed cache found", async () => {
const key = "node-test";
testUtils.setInputs({
path: "node_modules",
Expand Down Expand Up @@ -227,7 +237,7 @@ test("restore with cache found", async () => {
return Promise.resolve(tempPath);
});

const archivePath = path.join(tempPath, "cache.tgz");
const archivePath = path.join(tempPath, CacheFilename.Gzip);
const setCacheStateMock = jest.spyOn(actionUtils, "setCacheState");
const downloadCacheMock = jest.spyOn(cacheHttpClient, "downloadCache");

Expand All @@ -240,10 +250,17 @@ test("restore with cache found", async () => {
const unlinkFileMock = jest.spyOn(actionUtils, "unlinkFile");
const setCacheHitOutputMock = jest.spyOn(actionUtils, "setCacheHitOutput");

const compression = CompressionMethod.Gzip;
const getCompressionMock = jest
.spyOn(actionUtils, "getCompressionMethod")
.mockReturnValue(Promise.resolve(compression));

await run();

expect(stateMock).toHaveBeenCalledWith("CACHE_KEY", key);
expect(getCacheMock).toHaveBeenCalledWith([key]);
expect(getCacheMock).toHaveBeenCalledWith([key], {
compressionMethod: compression
});
expect(setCacheStateMock).toHaveBeenCalledWith(cacheEntry);
expect(createTempDirectoryMock).toHaveBeenCalledTimes(1);
expect(downloadCacheMock).toHaveBeenCalledWith(
Expand All @@ -253,7 +270,7 @@ test("restore with cache found", async () => {
expect(getArchiveFileSizeMock).toHaveBeenCalledWith(archivePath);

expect(extractTarMock).toHaveBeenCalledTimes(1);
expect(extractTarMock).toHaveBeenCalledWith(archivePath);
expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression);

expect(unlinkFileMock).toHaveBeenCalledTimes(1);
expect(unlinkFileMock).toHaveBeenCalledWith(archivePath);
Expand All @@ -263,9 +280,10 @@ test("restore with cache found", async () => {

expect(infoMock).toHaveBeenCalledWith(`Cache restored from key: ${key}`);
expect(failedMock).toHaveBeenCalledTimes(0);
expect(getCompressionMock).toHaveBeenCalledTimes(1);
});

test("restore with a pull request event and cache found", async () => {
test("restore with a pull request event and zstd compressed cache found", async () => {
const key = "node-test";
testUtils.setInputs({
path: "node_modules",
Expand Down Expand Up @@ -297,7 +315,7 @@ test("restore with a pull request event and cache found", async () => {
return Promise.resolve(tempPath);
});

const archivePath = path.join(tempPath, "cache.tgz");
const archivePath = path.join(tempPath, CacheFilename.Zstd);
const setCacheStateMock = jest.spyOn(actionUtils, "setCacheState");
const downloadCacheMock = jest.spyOn(cacheHttpClient, "downloadCache");

Expand All @@ -308,11 +326,17 @@ test("restore with a pull request event and cache found", async () => {

const extractTarMock = jest.spyOn(tar, "extractTar");
const setCacheHitOutputMock = jest.spyOn(actionUtils, "setCacheHitOutput");
const compression = CompressionMethod.Zstd;
const getCompressionMock = jest
.spyOn(actionUtils, "getCompressionMethod")
.mockReturnValue(Promise.resolve(compression));

await run();

expect(stateMock).toHaveBeenCalledWith("CACHE_KEY", key);
expect(getCacheMock).toHaveBeenCalledWith([key]);
expect(getCacheMock).toHaveBeenCalledWith([key], {
compressionMethod: compression
});
expect(setCacheStateMock).toHaveBeenCalledWith(cacheEntry);
expect(createTempDirectoryMock).toHaveBeenCalledTimes(1);
expect(downloadCacheMock).toHaveBeenCalledWith(
Expand All @@ -323,13 +347,14 @@ test("restore with a pull request event and cache found", async () => {
expect(infoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`);

expect(extractTarMock).toHaveBeenCalledTimes(1);
expect(extractTarMock).toHaveBeenCalledWith(archivePath);
expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression);

expect(setCacheHitOutputMock).toHaveBeenCalledTimes(1);
expect(setCacheHitOutputMock).toHaveBeenCalledWith(true);

expect(infoMock).toHaveBeenCalledWith(`Cache restored from key: ${key}`);
expect(failedMock).toHaveBeenCalledTimes(0);
expect(getCompressionMock).toHaveBeenCalledTimes(1);
});

test("restore with cache found for restore key", async () => {
Expand Down Expand Up @@ -364,7 +389,7 @@ test("restore with cache found for restore key", async () => {
return Promise.resolve(tempPath);
});

const archivePath = path.join(tempPath, "cache.tgz");
const archivePath = path.join(tempPath, CacheFilename.Zstd);
const setCacheStateMock = jest.spyOn(actionUtils, "setCacheState");
const downloadCacheMock = jest.spyOn(cacheHttpClient, "downloadCache");

Expand All @@ -375,11 +400,17 @@ test("restore with cache found for restore key", async () => {

const extractTarMock = jest.spyOn(tar, "extractTar");
const setCacheHitOutputMock = jest.spyOn(actionUtils, "setCacheHitOutput");
const compression = CompressionMethod.Zstd;
const getCompressionMock = jest
.spyOn(actionUtils, "getCompressionMethod")
.mockReturnValue(Promise.resolve(compression));

await run();

expect(stateMock).toHaveBeenCalledWith("CACHE_KEY", key);
expect(getCacheMock).toHaveBeenCalledWith([key, restoreKey]);
expect(getCacheMock).toHaveBeenCalledWith([key, restoreKey], {
compressionMethod: compression
});
expect(setCacheStateMock).toHaveBeenCalledWith(cacheEntry);
expect(createTempDirectoryMock).toHaveBeenCalledTimes(1);
expect(downloadCacheMock).toHaveBeenCalledWith(
Expand All @@ -390,7 +421,7 @@ test("restore with cache found for restore key", async () => {
expect(infoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`);

expect(extractTarMock).toHaveBeenCalledTimes(1);
expect(extractTarMock).toHaveBeenCalledWith(archivePath);
expect(extractTarMock).toHaveBeenCalledWith(archivePath, compression);

expect(setCacheHitOutputMock).toHaveBeenCalledTimes(1);
expect(setCacheHitOutputMock).toHaveBeenCalledWith(false);
Expand All @@ -399,4 +430,5 @@ test("restore with cache found for restore key", async () => {
`Cache restored from key: ${restoreKey}`
);
expect(failedMock).toHaveBeenCalledTimes(0);
expect(getCompressionMock).toHaveBeenCalledTimes(1);
});
69 changes: 57 additions & 12 deletions __tests__/save.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import * as core from "@actions/core";
import * as path from "path";

import * as cacheHttpClient from "../src/cacheHttpClient";
import { CacheFilename, Events, Inputs } from "../src/constants";
import {
CacheFilename,
CompressionMethod,
Events,
Inputs
} from "../src/constants";
import { ArtifactCacheEntry } from "../src/contracts";
import run from "../src/save";
import * as tar from "../src/tar";
Expand Down Expand Up @@ -50,6 +55,11 @@ beforeAll(() => {
jest.spyOn(actionUtils, "createTempDirectory").mockImplementation(() => {
return Promise.resolve("/foo/bar");
});

jest.spyOn(actionUtils, "getCacheFileName").mockImplementation(cm => {
const actualUtils = jest.requireActual("../src/utils/actionUtils");
return actualUtils.getCacheFileName(cm);
});
});

beforeEach(() => {
Expand Down Expand Up @@ -201,20 +211,27 @@ test("save with large cache outputs warning", async () => {
jest.spyOn(actionUtils, "getArchiveFileSize").mockImplementationOnce(() => {
return cacheSize;
});
const compression = CompressionMethod.Gzip;
const getCompressionMock = jest
.spyOn(actionUtils, "getCompressionMethod")
.mockReturnValue(Promise.resolve(compression));

await run();

const archiveFolder = "/foo/bar";

expect(createTarMock).toHaveBeenCalledTimes(1);
expect(createTarMock).toHaveBeenCalledWith(archiveFolder, cachePaths);

expect(createTarMock).toHaveBeenCalledWith(
archiveFolder,
cachePaths,
compression
);
expect(logWarningMock).toHaveBeenCalledTimes(1);
expect(logWarningMock).toHaveBeenCalledWith(
"Cache size of ~6144 MB (6442450944 B) is over the 5GB limit, not saving cache."
);

expect(failedMock).toHaveBeenCalledTimes(0);
expect(getCompressionMock).toHaveBeenCalledTimes(1);
});

test("save with reserve cache failure outputs warning", async () => {
Expand Down Expand Up @@ -250,13 +267,18 @@ test("save with reserve cache failure outputs warning", async () => {
});

const createTarMock = jest.spyOn(tar, "createTar");

const saveCacheMock = jest.spyOn(cacheHttpClient, "saveCache");
const compression = CompressionMethod.Zstd;
const getCompressionMock = jest
.spyOn(actionUtils, "getCompressionMethod")
.mockReturnValue(Promise.resolve(compression));

await run();

expect(reserveCacheMock).toHaveBeenCalledTimes(1);
expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey);
expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, {
compressionMethod: compression
});

expect(infoMock).toHaveBeenCalledWith(
`Unable to reserve cache with key ${primaryKey}, another job may be creating this cache.`
Expand All @@ -266,6 +288,7 @@ test("save with reserve cache failure outputs warning", async () => {
expect(saveCacheMock).toHaveBeenCalledTimes(0);
expect(logWarningMock).toHaveBeenCalledTimes(0);
expect(failedMock).toHaveBeenCalledTimes(0);
expect(getCompressionMock).toHaveBeenCalledTimes(1);
});

test("save with server error outputs warning", async () => {
Expand Down Expand Up @@ -308,17 +331,27 @@ test("save with server error outputs warning", async () => {
.mockImplementationOnce(() => {
throw new Error("HTTP Error Occurred");
});
const compression = CompressionMethod.Zstd;
const getCompressionMock = jest
.spyOn(actionUtils, "getCompressionMethod")
.mockReturnValue(Promise.resolve(compression));

await run();

expect(reserveCacheMock).toHaveBeenCalledTimes(1);
expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey);
expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, {
compressionMethod: compression
});

const archiveFolder = "/foo/bar";
const archiveFile = path.join(archiveFolder, CacheFilename);
const archiveFile = path.join(archiveFolder, CacheFilename.Zstd);

expect(createTarMock).toHaveBeenCalledTimes(1);
expect(createTarMock).toHaveBeenCalledWith(archiveFolder, cachePaths);
expect(createTarMock).toHaveBeenCalledWith(
archiveFolder,
cachePaths,
compression
);

expect(saveCacheMock).toHaveBeenCalledTimes(1);
expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile);
Expand All @@ -327,6 +360,7 @@ test("save with server error outputs warning", async () => {
expect(logWarningMock).toHaveBeenCalledWith("HTTP Error Occurred");

expect(failedMock).toHaveBeenCalledTimes(0);
expect(getCompressionMock).toHaveBeenCalledTimes(1);
});

test("save with valid inputs uploads a cache", async () => {
Expand Down Expand Up @@ -364,20 +398,31 @@ test("save with valid inputs uploads a cache", async () => {
const createTarMock = jest.spyOn(tar, "createTar");

const saveCacheMock = jest.spyOn(cacheHttpClient, "saveCache");
const compression = CompressionMethod.Zstd;
const getCompressionMock = jest
.spyOn(actionUtils, "getCompressionMethod")
.mockReturnValue(Promise.resolve(compression));

await run();

expect(reserveCacheMock).toHaveBeenCalledTimes(1);
expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey);
expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, {
compressionMethod: compression
});

const archiveFolder = "/foo/bar";
const archiveFile = path.join(archiveFolder, CacheFilename);
const archiveFile = path.join(archiveFolder, CacheFilename.Zstd);

expect(createTarMock).toHaveBeenCalledTimes(1);
expect(createTarMock).toHaveBeenCalledWith(archiveFolder, cachePaths);
expect(createTarMock).toHaveBeenCalledWith(
archiveFolder,
cachePaths,
compression
);

expect(saveCacheMock).toHaveBeenCalledTimes(1);
expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile);

expect(failedMock).toHaveBeenCalledTimes(0);
expect(getCompressionMock).toHaveBeenCalledTimes(1);
});
Loading

0 comments on commit 9eb452c

Please sign in to comment.