Skip to content

Commit 2eb8bed

Browse files
committed
Test including lockfile information for hasChanges
1 parent a5c3d16 commit 2eb8bed

File tree

8 files changed

+1009
-29
lines changed

8 files changed

+1009
-29
lines changed

.github/actions/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
"@actions/artifact": "^2.3.2",
1414
"@actions/core": "^1.11.1",
1515
"@actions/exec": "^1.1.1",
16-
"lodash": "^4.17.21"
16+
"lodash": "^4.17.21",
17+
"snyk-nodejs-lockfile-parser": "^2.4.2"
1718
},
1819
"scripts": {
1920
"build": "node ./build.js",

.github/actions/src/__tests__/commons.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ vi.mock(import('lodash/memoize.js'), () => ({
88

99
const mockedExecOutput = vi.spyOn(exec, 'getExecOutput');
1010

11-
describe(commons.checkForChanges, () => {
11+
describe(commons.checkDirForChanges, () => {
1212
function mockChanges(value: boolean) {
1313
mockedExecOutput.mockResolvedValueOnce({
1414
exitCode: value ? 1 : 0, stdout: '', stderr: ''
@@ -17,14 +17,14 @@ describe(commons.checkForChanges, () => {
1717

1818
it('should return true if git diff exits with non zero code', async () => {
1919
mockChanges(true);
20-
await expect(commons.checkForChanges('/')).resolves.toEqual(true);
20+
await expect(commons.checkDirForChanges('/')).resolves.toEqual(true);
2121
expect(mockedExecOutput).toHaveBeenCalledOnce();
2222
});
2323

2424
it('should return false if git diff exits with 0', async () => {
2525
mockChanges(false);
2626

27-
await expect(commons.checkForChanges('/')).resolves.toEqual(false);
27+
await expect(commons.checkDirForChanges('/')).resolves.toEqual(false);
2828
expect(mockedExecOutput).toHaveBeenCalledOnce();
2929
});
3030
});

.github/actions/src/commons.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,12 @@ export function isPackageRecord(obj: unknown): obj is PackageRecord {
6060
return true;
6161
}
6262

63-
// Not using the repotools version since this uses @action/exec instead of
64-
// calling execFile from child_process
65-
export async function getGitRoot() {
66-
const { stdout } = await getExecOutput('git rev-parse --show-toplevel');
67-
return stdout.trim();
68-
}
69-
7063
/**
7164
* Returns `true` if there are changes present in the given directory relative to
7265
* the master branch\
7366
* Used to determine, particularly for libraries, if running tests and tsc are necessary
7467
*/
75-
export const checkForChanges = memoize(async (directory: string) => {
68+
export const checkDirForChanges = memoize(async (directory: string) => {
7669
const { exitCode } = await getExecOutput(
7770
'git',
7871
['--no-pager', 'diff', '--quiet', 'origin/master', '--', directory],

.github/actions/src/gitRoot.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { getExecOutput } from '@actions/exec';
2+
3+
// Not using the repotools version since this uses @action/exec instead of
4+
// calling execFile from child_process
5+
export async function getGitRoot() {
6+
const { stdout } = await getExecOutput('git rev-parse --show-toplevel');
7+
return stdout.trim();
8+
}
9+
10+
/**
11+
* Path to the root of the git repository
12+
*/
13+
export const gitRoot = await getGitRoot();

.github/actions/src/info/__tests__/index.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@ import pathlib from 'path';
44
import * as core from '@actions/core';
55
import { describe, expect, test, vi } from 'vitest';
66
import * as git from '../../commons.js';
7+
import * as lockfiles from '../../lockfiles.js';
78
import { getAllPackages, getRawPackages, main } from '../index.js';
89

9-
const mockedCheckChanges = vi.spyOn(git, 'checkForChanges');
10+
const mockedCheckChanges = vi.spyOn(git, 'checkDirForChanges');
1011

1112
vi.mock(import('path'), async importOriginal => {
1213
const { posix } = await importOriginal();
@@ -17,6 +18,10 @@ vi.mock(import('path'), async importOriginal => {
1718
};
1819
});
1920

21+
vi.mock(import('../../gitRoot.js'), () => ({
22+
gitRoot: 'root'
23+
}));
24+
2025
class NodeError extends Error {
2126
constructor(public readonly code: string) {
2227
super();
@@ -116,6 +121,7 @@ function mockReadFile(path: string) {
116121

117122
vi.spyOn(fs, 'readdir').mockImplementation(mockReaddir as any);
118123
vi.spyOn(fs, 'readFile').mockImplementation(mockReadFile as any);
124+
vi.spyOn(lockfiles, 'hasLockFileChanged').mockResolvedValue(false);
119125

120126
describe(getRawPackages, () => {
121127
test('maxDepth = 1', async () => {
@@ -127,7 +133,7 @@ describe(getRawPackages, () => {
127133
const [[name, packageData]] = results;
128134
expect(name).toEqual('@sourceacademy/modules');
129135
expect(packageData.hasChanges).toEqual(true);
130-
expect(git.checkForChanges).toHaveBeenCalledOnce();
136+
expect(git.checkDirForChanges).toHaveBeenCalledOnce();
131137
});
132138

133139
test('maxDepth = 3', async () => {
@@ -215,7 +221,6 @@ describe(getAllPackages, () => {
215221
});
216222

217223
describe(main, () => {
218-
vi.spyOn(git, 'getGitRoot').mockResolvedValue('root');
219224
const mockedSetOutput = vi.spyOn(core, 'setOutput');
220225

221226
vi.spyOn(core.summary, 'addHeading').mockImplementation(() => core.summary);

.github/actions/src/info/index.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import utils from 'util';
44
import * as core from '@actions/core';
55
import type { SummaryTableRow } from '@actions/core/lib/summary.js';
66
import packageJson from '../../../../package.json' with { type: 'json' };
7-
import { checkForChanges, getGitRoot, type PackageRecord, type RawPackageRecord } from '../commons.js';
7+
import { checkDirForChanges, type PackageRecord, type RawPackageRecord } from '../commons.js';
8+
import { getPackagesWithResolutionChanges, hasLockFileChanged } from '../lockfiles.js'
89
import { topoSortPackages } from './sorter.js';
10+
import { gitRoot } from '../gitRoot';
911

1012
const packageNameRE = /^@sourceacademy\/(.+?)-(.+)$/u;
1113

@@ -14,6 +16,14 @@ const packageNameRE = /^@sourceacademy\/(.+?)-(.+)$/u;
1416
* an unprocessed format
1517
*/
1618
export async function getRawPackages(gitRoot: string, maxDepth?: number) {
19+
let packagesWithResolutionChanges: string[] = [];
20+
21+
// If there are lock file changes we need to set hasChanges to true for
22+
// that package even if that package's directory has no changes
23+
if (await hasLockFileChanged()) {
24+
packagesWithResolutionChanges = await getPackagesWithResolutionChanges();
25+
}
26+
1727
const output: Record<string, RawPackageRecord> = {};
1828

1929
/**
@@ -26,14 +36,14 @@ export async function getRawPackages(gitRoot: string, maxDepth?: number) {
2636
if (item.name === 'package.json') {
2737
try {
2838
const [hasChanges, packageJson] = await Promise.all([
29-
checkForChanges(currentDir),
39+
checkDirForChanges(currentDir),
3040
fs.readFile(pathlib.join(currentDir, 'package.json'), 'utf-8')
3141
.then(JSON.parse)
3242
]);
3343

3444
output[packageJson.name] = {
3545
directory: currentDir,
36-
hasChanges,
46+
hasChanges: hasChanges || packagesWithResolutionChanges.includes(packageJson.name),
3747
package: packageJson
3848
};
3949
} catch (error) {
@@ -216,7 +226,6 @@ function setOutputs(
216226
}
217227

218228
export async function main() {
219-
const gitRoot = await getGitRoot();
220229
const { packages, bundles, tabs, libs } = await getAllPackages(gitRoot);
221230

222231
const { repository } = packageJson;
@@ -265,7 +274,7 @@ export async function main() {
265274
packages['@sourceacademy/modules-docserver']
266275
);
267276

268-
const workflows = await checkForChanges(pathlib.join(gitRoot, '.github/workflows'));
277+
const workflows = await checkDirForChanges(pathlib.join(gitRoot, '.github/workflows'));
269278
core.setOutput('workflows', workflows);
270279
}
271280

.github/actions/src/lockfiles.ts

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
import fs from 'fs/promises';
2+
import pathlib from 'path';
3+
import memoize from 'lodash/memoize';
4+
import uniq from 'lodash/uniq';
5+
import * as core from '@actions/core';
6+
import { getExecOutput } from '@actions/exec';
7+
import { extractPkgsFromYarnLockV2 } from "snyk-nodejs-lockfile-parser";
8+
import { gitRoot } from './gitRoot.js';
9+
10+
/**
11+
* Parses and lockfile's contents and extracts all the different dependencies and
12+
* versions
13+
*/
14+
function processLockFileText(contents: string) {
15+
const lockFile = extractPkgsFromYarnLockV2(contents);
16+
return Object.entries(lockFile).reduce<Record<string, Set<string>>>((res, [key, { version }]) => {
17+
const newKey = key.split('@')[0];
18+
19+
if (!(newKey in res)) {
20+
res[newKey] = new Set()
21+
}
22+
23+
res[newKey].add(version);
24+
return res;
25+
}, {});
26+
}
27+
28+
/**
29+
* Retrieves the contents of the lockfile on the master branch
30+
*/
31+
async function getMasterLockFile() {
32+
const { stdout, exitCode } = await getExecOutput(
33+
'git',
34+
[
35+
'--no-pager',
36+
'show',
37+
'origin/master:yarn.lock'
38+
],
39+
);
40+
41+
if (exitCode !== 0) {
42+
throw new Error('git show exited with non-zero error-code');
43+
}
44+
45+
return processLockFileText(stdout);
46+
}
47+
48+
/**
49+
* Retrieves the contents of the lockfile in the repo
50+
*/
51+
async function getCurrentLockFile() {
52+
const lockFilePath = pathlib.join(gitRoot, 'yarn.lock');
53+
const contents = await fs.readFile(lockFilePath, 'utf-8');
54+
return processLockFileText(contents);
55+
}
56+
57+
/**
58+
* Determines the names of the packages that have changed versions
59+
*/
60+
async function getPackageDiffs() {
61+
const [currentLockFile, masterLockFile] = await Promise.all([
62+
getCurrentLockFile(),
63+
getMasterLockFile()
64+
]);
65+
66+
const packages = [];
67+
for (const [depName, versions] of Object.entries(currentLockFile)) {
68+
if (!(depName in masterLockFile)) {
69+
core.info(`${depName} in current lockfile, not in master lock file`);
70+
continue;
71+
}
72+
73+
const args = depName.split('@');
74+
let needToAdd = false;
75+
for (const version of versions) {
76+
if (!masterLockFile[depName].has(version)) {
77+
core.info(`${depName} has ${version} in current lockfile but not in master`);
78+
needToAdd = true;
79+
}
80+
}
81+
82+
for (const version of masterLockFile[depName]) {
83+
if (!versions.has(version)) {
84+
core.info(`${depName} has ${version} in master lockfile but not in current`);
85+
needToAdd = true;
86+
}
87+
}
88+
89+
if (needToAdd) {
90+
packages.push(args[0]);
91+
}
92+
}
93+
94+
return uniq(packages);
95+
}
96+
97+
/**
98+
* Using `yarn why`, determine why the given package is present in the
99+
* lockfile.
100+
*/
101+
async function getPackageReason(pkg: string) {
102+
const packageNameRE = /^(@?sourceacademy\/.+)@.+$/
103+
104+
const { stdout, exitCode } = await getExecOutput('yarn why', [pkg, '-R', '--json']);
105+
if (exitCode !== 0) {
106+
throw new Error('yarn why exited with non-zero exit code!');
107+
}
108+
109+
return stdout.trim().split('\n').map(each => {
110+
const entry = JSON.parse(each).value;
111+
const match = packageNameRE.exec(entry);
112+
if (!match) {
113+
throw new Error('failed to identify a package!');
114+
}
115+
116+
return match[1];
117+
});
118+
}
119+
120+
/**
121+
* Returns `true` if there are changes present in the given directory relative to
122+
* the master branch\
123+
* Used to determine, particularly for libraries, if running tests and tsc are necessary
124+
*/
125+
export const hasLockFileChanged = memoize(async () => {
126+
const { exitCode } = await getExecOutput(
127+
'git',
128+
['--no-pager', 'diff', '--quiet', 'origin/master', '--', 'yarn.lock'],
129+
{
130+
failOnStdErr: false,
131+
ignoreReturnCode: true
132+
}
133+
);
134+
return exitCode !== 0;
135+
});
136+
137+
/**
138+
* Gets a list of packages that have had some dependency of theirs
139+
* change according to the lockfile but not their package.json
140+
*/
141+
export async function getPackagesWithResolutionChanges() {
142+
const packages = await getPackageDiffs();
143+
const workspaces = await Promise.all(packages.map(getPackageReason));
144+
return uniq(workspaces.flat());
145+
}

0 commit comments

Comments
 (0)