Skip to content

Commit

Permalink
review updates second stage
Browse files Browse the repository at this point in the history
  • Loading branch information
dsame committed Jun 14, 2023
1 parent 6aacde7 commit 3ef5272
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 83 deletions.
2 changes: 1 addition & 1 deletion __tests__/cache-restore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
27 changes: 10 additions & 17 deletions __tests__/cache-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -104,10 +103,6 @@ describe('cache-utils', () => {
(pattern: string): Promise<Globber> =>
MockGlobber.create(['/foo', '/bar'])
);

Object.keys(memoizedCacheDependencies).forEach(
key => delete memoizedCacheDependencies[key]
);
});

afterEach(() => {
Expand Down Expand Up @@ -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}"`
);
}
);

Expand Down
25 changes: 4 additions & 21 deletions dist/cache-save/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand All @@ -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;
});
/**
Expand Down
27 changes: 5 additions & 22 deletions dist/setup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand All @@ -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;
});
/**
Expand Down
2 changes: 1 addition & 1 deletion src/cache-restore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
25 changes: 4 additions & 21 deletions src/cache-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -125,30 +120,18 @@ 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)
.filter(unique())
.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;
Expand Down

0 comments on commit 3ef5272

Please sign in to comment.