From 3ef527264ac68164b301e0b46fb15d0472df423a Mon Sep 17 00:00:00 2001
From: Sergey Dolin <dsame@github.com>
Date: Wed, 14 Jun 2023 19:42:28 +0200
Subject: [PATCH] review updates second stage
---
__tests__/cache-restore.test.ts | 2 +-
__tests__/cache-utils.test.ts | 27 ++++++++++-----------------
dist/cache-save/index.js | 25 ++++---------------------
dist/setup/index.js | 27 +++++----------------------
src/cache-restore.ts | 2 +-
src/cache-utils.ts | 25 ++++---------------------
6 files changed, 25 insertions(+), 83 deletions(-)
diff --git a/__tests__/cache-restore.test.ts b/__tests__/cache-restore.test.ts
index 86dff3f3..90153a40 100644
--- a/__tests__/cache-restore.test.ts
+++ b/__tests__/cache-restore.test.ts
@@ -135,7 +135,7 @@ describe('cache-restore', () => {
await restoreCache(packageManager, '');
expect(hashFilesSpy).toHaveBeenCalled();
expect(infoSpy).toHaveBeenCalledWith(
- `Cache restored from key: node-cache-${platform}-${packageManager}-v2-${fileHash}`
+ `Cache restored from key: node-cache-${platform}-${packageManager}-${fileHash}`
);
expect(infoSpy).not.toHaveBeenCalledWith(
`${packageManager} cache is not found`
diff --git a/__tests__/cache-utils.test.ts b/__tests__/cache-utils.test.ts
index da9c793b..56f46425 100644
--- a/__tests__/cache-utils.test.ts
+++ b/__tests__/cache-utils.test.ts
@@ -6,8 +6,7 @@ import {
PackageManagerInfo,
isCacheFeatureAvailable,
supportedPackageManagers,
- getCommandOutput,
- memoizedCacheDependencies
+ getCommandOutput
} from '../src/cache-utils';
import fs from 'fs';
import * as cacheUtils from '../src/cache-utils';
@@ -104,10 +103,6 @@ describe('cache-utils', () => {
(pattern: string): Promise<Globber> =>
MockGlobber.create(['/foo', '/bar'])
);
-
- Object.keys(memoizedCacheDependencies).forEach(
- key => delete memoizedCacheDependencies[key]
- );
});
afterEach(() => {
@@ -175,25 +170,23 @@ describe('cache-utils', () => {
);
it.each([
- [supportedPackageManagers.npm, ''],
- [supportedPackageManagers.npm, '/dir/file.lock'],
- [supportedPackageManagers.npm, '/**/file.lock'],
- [supportedPackageManagers.pnpm, ''],
- [supportedPackageManagers.pnpm, '/dir/file.lock'],
- [supportedPackageManagers.pnpm, '/**/file.lock'],
- [supportedPackageManagers.yarn, ''],
[supportedPackageManagers.yarn, '/dir/file.lock'],
[supportedPackageManagers.yarn, '/**/file.lock']
])(
- 'getCacheDirectoriesPaths should throw in case of having not directories',
+ 'getCacheDirectoriesPaths should nothrow in case of having not directories',
async (packageManagerInfo, cacheDependency) => {
lstatSpy.mockImplementation(arg => ({
isDirectory: () => false
}));
- await expect(
- cacheUtils.getCacheDirectories(packageManagerInfo, cacheDependency)
- ).rejects.toThrow(); //'Could not get cache folder path for /dir');
+ await cacheUtils.getCacheDirectories(
+ packageManagerInfo,
+ cacheDependency
+ );
+ expect(warningSpy).toHaveBeenCalledTimes(1);
+ expect(warningSpy).toHaveBeenCalledWith(
+ `No existing directories found containing cache-dependency-path="${cacheDependency}"`
+ );
}
);
diff --git a/dist/cache-save/index.js b/dist/cache-save/index.js
index 2b16ee16..1170e332 100644
--- a/dist/cache-save/index.js
+++ b/dist/cache-save/index.js
@@ -60434,7 +60434,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
-exports.isCacheFeatureAvailable = exports.isGhes = exports.getCacheDirectories = exports.memoizedCacheDependencies = exports.getPackageManagerInfo = exports.getCommandOutputNotEmpty = exports.getCommandOutput = exports.supportedPackageManagers = void 0;
+exports.isCacheFeatureAvailable = exports.isGhes = exports.getCacheDirectories = exports.getPackageManagerInfo = exports.getCommandOutputNotEmpty = exports.getCommandOutput = exports.supportedPackageManagers = void 0;
const core = __importStar(__nccwpck_require__(2186));
const exec = __importStar(__nccwpck_require__(1514));
const cache = __importStar(__nccwpck_require__(7799));
@@ -60503,11 +60503,6 @@ const getPackageManagerInfo = (packageManager) => __awaiter(void 0, void 0, void
}
});
exports.getPackageManagerInfo = getPackageManagerInfo;
-/**
- * glob expanding memoized because it involves potentially very deep
- * traversing through the directories tree
- */
-exports.memoizedCacheDependencies = {};
/**
* Expands (converts) the string input `cache-dependency-path` to list of directories that
* may be project roots
@@ -60516,27 +60511,15 @@ exports.memoizedCacheDependencies = {};
* @return list of directories and possible
*/
const getProjectDirectoriesFromCacheDependencyPath = (cacheDependencyPath) => __awaiter(void 0, void 0, void 0, function* () {
- let cacheDependenciesPaths;
- // memoize unglobbed paths to avoid traversing FS
- const memoized = exports.memoizedCacheDependencies[cacheDependencyPath];
- if (memoized) {
- cacheDependenciesPaths = memoized;
- }
- else {
- const globber = yield glob.create(cacheDependencyPath);
- cacheDependenciesPaths = yield globber.glob();
- exports.memoizedCacheDependencies[cacheDependencyPath] = cacheDependenciesPaths;
- }
+ const globber = yield glob.create(cacheDependencyPath);
+ const cacheDependenciesPaths = yield globber.glob();
const existingDirectories = cacheDependenciesPaths
.map(path_1.default.dirname)
.filter(util_1.unique())
.filter(fs_1.default.existsSync)
.filter(directory => fs_1.default.lstatSync(directory).isDirectory());
- // if user explicitly pointed out some file, but it does not exist it is definitely
- // not he wanted, thus we should throw an error not trying to workaround with unexpected
- // result to the whole build
if (!existingDirectories.length)
- throw Error('No existing directories found containing `cache-dependency-path`="${cacheDependencyPath}"');
+ core.warning(`No existing directories found containing cache-dependency-path="${cacheDependencyPath}"`);
return existingDirectories;
});
/**
diff --git a/dist/setup/index.js b/dist/setup/index.js
index 24865a81..81ecf48f 100644
--- a/dist/setup/index.js
+++ b/dist/setup/index.js
@@ -71153,7 +71153,7 @@ const restoreCache = (packageManager, cacheDependencyPath) => __awaiter(void 0,
if (!fileHash) {
throw new Error('Some specified paths were not resolved, unable to cache dependencies.');
}
- const primaryKey = `node-cache-${platform}-${packageManager}-v2-${fileHash}`;
+ const primaryKey = `node-cache-${platform}-${packageManager}-${fileHash}`;
core.debug(`primary key is ${primaryKey}`);
core.saveState(constants_1.State.CachePrimaryKey, primaryKey);
const cacheKey = yield cache.restoreCache(cachePaths, primaryKey);
@@ -71217,7 +71217,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
-exports.isCacheFeatureAvailable = exports.isGhes = exports.getCacheDirectories = exports.memoizedCacheDependencies = exports.getPackageManagerInfo = exports.getCommandOutputNotEmpty = exports.getCommandOutput = exports.supportedPackageManagers = void 0;
+exports.isCacheFeatureAvailable = exports.isGhes = exports.getCacheDirectories = exports.getPackageManagerInfo = exports.getCommandOutputNotEmpty = exports.getCommandOutput = exports.supportedPackageManagers = void 0;
const core = __importStar(__nccwpck_require__(2186));
const exec = __importStar(__nccwpck_require__(1514));
const cache = __importStar(__nccwpck_require__(7799));
@@ -71286,11 +71286,6 @@ const getPackageManagerInfo = (packageManager) => __awaiter(void 0, void 0, void
}
});
exports.getPackageManagerInfo = getPackageManagerInfo;
-/**
- * glob expanding memoized because it involves potentially very deep
- * traversing through the directories tree
- */
-exports.memoizedCacheDependencies = {};
/**
* Expands (converts) the string input `cache-dependency-path` to list of directories that
* may be project roots
@@ -71299,27 +71294,15 @@ exports.memoizedCacheDependencies = {};
* @return list of directories and possible
*/
const getProjectDirectoriesFromCacheDependencyPath = (cacheDependencyPath) => __awaiter(void 0, void 0, void 0, function* () {
- let cacheDependenciesPaths;
- // memoize unglobbed paths to avoid traversing FS
- const memoized = exports.memoizedCacheDependencies[cacheDependencyPath];
- if (memoized) {
- cacheDependenciesPaths = memoized;
- }
- else {
- const globber = yield glob.create(cacheDependencyPath);
- cacheDependenciesPaths = yield globber.glob();
- exports.memoizedCacheDependencies[cacheDependencyPath] = cacheDependenciesPaths;
- }
+ const globber = yield glob.create(cacheDependencyPath);
+ const cacheDependenciesPaths = yield globber.glob();
const existingDirectories = cacheDependenciesPaths
.map(path_1.default.dirname)
.filter(util_1.unique())
.filter(fs_1.default.existsSync)
.filter(directory => fs_1.default.lstatSync(directory).isDirectory());
- // if user explicitly pointed out some file, but it does not exist it is definitely
- // not he wanted, thus we should throw an error not trying to workaround with unexpected
- // result to the whole build
if (!existingDirectories.length)
- throw Error('No existing directories found containing `cache-dependency-path`="${cacheDependencyPath}"');
+ core.warning(`No existing directories found containing cache-dependency-path="${cacheDependencyPath}"`);
return existingDirectories;
});
/**
diff --git a/src/cache-restore.ts b/src/cache-restore.ts
index e138707c..6ac2cc75 100644
--- a/src/cache-restore.ts
+++ b/src/cache-restore.ts
@@ -37,7 +37,7 @@ export const restoreCache = async (
);
}
- const primaryKey = `node-cache-${platform}-${packageManager}-v2-${fileHash}`;
+ const primaryKey = `node-cache-${platform}-${packageManager}-${fileHash}`;
core.debug(`primary key is ${primaryKey}`);
core.saveState(State.CachePrimaryKey, primaryKey);
diff --git a/src/cache-utils.ts b/src/cache-utils.ts
index c30e6502..0177fba0 100644
--- a/src/cache-utils.ts
+++ b/src/cache-utils.ts
@@ -110,11 +110,6 @@ export const getPackageManagerInfo = async (packageManager: string) => {
}
};
-/**
- * glob expanding memoized because it involves potentially very deep
- * traversing through the directories tree
- */
-export const memoizedCacheDependencies: Record<string, string[]> = {};
/**
* Expands (converts) the string input `cache-dependency-path` to list of directories that
* may be project roots
@@ -125,17 +120,8 @@ export const memoizedCacheDependencies: Record<string, string[]> = {};
const getProjectDirectoriesFromCacheDependencyPath = async (
cacheDependencyPath: string
): Promise<string[]> => {
- let cacheDependenciesPaths: string[];
-
- // memoize unglobbed paths to avoid traversing FS
- const memoized = memoizedCacheDependencies[cacheDependencyPath];
- if (memoized) {
- cacheDependenciesPaths = memoized;
- } else {
- const globber = await glob.create(cacheDependencyPath);
- cacheDependenciesPaths = await globber.glob();
- memoizedCacheDependencies[cacheDependencyPath] = cacheDependenciesPaths;
- }
+ const globber = await glob.create(cacheDependencyPath);
+ const cacheDependenciesPaths = await globber.glob();
const existingDirectories: string[] = cacheDependenciesPaths
.map(path.dirname)
@@ -143,12 +129,9 @@ const getProjectDirectoriesFromCacheDependencyPath = async (
.filter(fs.existsSync)
.filter(directory => fs.lstatSync(directory).isDirectory());
- // if user explicitly pointed out some file, but it does not exist it is definitely
- // not he wanted, thus we should throw an error not trying to workaround with unexpected
- // result to the whole build
if (!existingDirectories.length)
- throw Error(
- 'No existing directories found containing `cache-dependency-path`="${cacheDependencyPath}"'
+ core.warning(
+ `No existing directories found containing cache-dependency-path="${cacheDependencyPath}"`
);
return existingDirectories;