Skip to content

Commit cf01cd0

Browse files
metalwarrior665metalwarrior665
authored
fix(bin): in --base-commit, allow both sha string and Commit object (#70)
1 parent c7a8670 commit cf01cd0

4 files changed

Lines changed: 83 additions & 24 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
All notable changes to this project will be documented in this file.
44

55
<!-- git-cliff-unreleased-start -->
6+
67
## 0.5.7 - **not yet released**
78

89
### 🚀 Features
@@ -14,7 +15,6 @@ All notable changes to this project will be documented in this file.
1415

1516
- Bump version to test beta release ([1322d31](https://github.com/apify/apify-test-tools/commit/1322d31873b6d43e16a68e97bdc358752f813f79)) by [@metalwarrior665](https://github.com/metalwarrior665)
1617

17-
1818
<!-- git-cliff-unreleased-end -->
1919

2020
# Changelog
@@ -91,4 +91,4 @@ feat: feat: add maxRetriesPerRequest test
9191
### Cli
9292

9393
- fix: parsing commits
94-
- feat: add `--workspace` cli option
94+
- feat: add `--workspace` cli option

‎bin/git.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,35 @@ export const getChangedFiles = (commits: Commit[]) => {
1717
return changedFiles;
1818
};
1919

20+
const SHA_REGEX = /^[0-9a-f]{40}$/i;
21+
22+
/**
23+
*
24+
* @param shaOrCommit Supports both a SHA string or a Commit object in JSON format. Can be empty.
25+
* @returns The SHA string if valid, otherwise throws an error.
26+
*/
27+
export const parseBaseCommit = (shaOrCommit: string | undefined): string | undefined => {
28+
if (!shaOrCommit) return undefined;
29+
let sha: string;
30+
if (shaOrCommit.startsWith('{')) {
31+
sha = (JSON.parse(shaOrCommit) as Commit).sha;
32+
} else {
33+
sha = shaOrCommit;
34+
}
35+
if (!SHA_REGEX.test(sha)) {
36+
throw new Error(
37+
`Invalid base commit SHA: "${sha}". It should be a 40-character hexadecimal string, instead got input: "${shaOrCommit}".`,
38+
);
39+
}
40+
return sha;
41+
};
42+
2043
/**
2144
* Gets the commits between sourceBranch and targetBranch (exclusive).
2245
* - If baseCommit is provided, only returns commits after the baseCommit.
2346
*/
24-
export const getCommits = ({ sourceBranch, targetBranch, baseCommit: baseCommitSha }: Config): Commit[] => {
47+
export const getCommits = ({ sourceBranch, targetBranch, baseCommit }: Config): Commit[] => {
48+
const baseCommitSha = parseBaseCommit(baseCommit);
2549
const commitsStrings = spawnCommandInGhWorkspace(
2650
`git log --pretty=format:'${GIT_LOG_FORMAT}' ${targetBranch}..${sourceBranch}`,
2751
).split('\n');

‎lib/extend-expect.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,6 @@ const isWithinInterval = <T extends string>(
291291
diffs.pass = false;
292292
diffs.actual.push(`${intervalOption}=${actual}`);
293293
diffs.expected.push(`${intervalOption}=${expected}`);
294-
// eslint-disable-next-line consistent-return
295294
return false;
296295
}
297296
} else if (typeof expected === 'object') {
@@ -301,7 +300,6 @@ const isWithinInterval = <T extends string>(
301300
diffs.pass = false;
302301
diffs.actual.push(`${intervalOption}=${actual}`);
303302
diffs.expected.push(`${intervalOption}=<${min ?? ''},${max ?? ''}>`);
304-
// eslint-disable-next-line consistent-return
305303
return false;
306304
}
307305
}

‎test/unit/bin/git.test.ts‎

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
import type { MockInstance } from 'vitest';
22
import { beforeEach, describe, expect, it, vi } from 'vitest';
33

4-
import { getChangedFiles, getCommits } from '../../../bin/git.js';
4+
import { getChangedFiles, getCommits, parseBaseCommit } from '../../../bin/git.js';
55
import * as Utils from '../../../bin/utils.js';
66

77
describe('getCommits', () => {
88
const sourceBranch = 'feature-branch';
99
const targetBranch = 'main';
1010

11-
const commit1 = 'Commit1»¦«Author1»¦«Date1»¦«First Change On Feature';
12-
const commit2 = 'Commit2»¦«Author1»¦«Date2»¦«Second Change On Feature';
13-
const commit3 = 'Commit3»¦«Author1»¦«Date3»¦«Third Change On Feature';
11+
const sha1 = '1'.repeat(40);
12+
const sha2 = '2'.repeat(40);
13+
const sha3 = '3'.repeat(40);
14+
15+
const commit1 = `${sha1}»¦«Author1»¦«Date1»¦«First Change On Feature`;
16+
const commit2 = `${sha2}»¦«Author1»¦«Date2»¦«Second Change On Feature`;
17+
const commit3 = `${sha3}»¦«Author1»¦«Date3»¦«Third Change On Feature`;
1418

1519
let gitCommandSpy: MockInstance;
1620

@@ -26,9 +30,9 @@ describe('getCommits', () => {
2630

2731
// Assert
2832
expect(commits).toStrictEqual([
29-
{ sha: 'Commit1', author: 'Author1', date: 'Date1', message: 'First Change On Feature' },
30-
{ sha: 'Commit2', author: 'Author1', date: 'Date2', message: 'Second Change On Feature' },
31-
{ sha: 'Commit3', author: 'Author1', date: 'Date3', message: 'Third Change On Feature' },
33+
{ sha: sha1, author: 'Author1', date: 'Date1', message: 'First Change On Feature' },
34+
{ sha: sha2, author: 'Author1', date: 'Date2', message: 'Second Change On Feature' },
35+
{ sha: sha3, author: 'Author1', date: 'Date3', message: 'Third Change On Feature' },
3236
]);
3337

3438
expect(gitCommandSpy).toHaveBeenCalledTimes(1);
@@ -39,12 +43,12 @@ describe('getCommits', () => {
3943

4044
it('should return commits after the base commit if provided', () => {
4145
// Act
42-
const commits = getCommits({ sourceBranch, targetBranch, baseCommit: 'Commit1' });
46+
const commits = getCommits({ sourceBranch, targetBranch, baseCommit: sha1 });
4347

4448
// Assert
4549
expect(commits).toStrictEqual([
46-
{ sha: 'Commit2', author: 'Author1', date: 'Date2', message: 'Second Change On Feature' },
47-
{ sha: 'Commit3', author: 'Author1', date: 'Date3', message: 'Third Change On Feature' },
50+
{ sha: sha2, author: 'Author1', date: 'Date2', message: 'Second Change On Feature' },
51+
{ sha: sha3, author: 'Author1', date: 'Date3', message: 'Third Change On Feature' },
4852
]);
4953

5054
expect(gitCommandSpy).toHaveBeenCalledTimes(1);
@@ -55,13 +59,13 @@ describe('getCommits', () => {
5559

5660
it('should return all commits if base commit is not found', () => {
5761
// Act
58-
const commits = getCommits({ sourceBranch, targetBranch, baseCommit: 'NonExistingCommit' });
62+
const commits = getCommits({ sourceBranch, targetBranch, baseCommit: 'a'.repeat(40) });
5963

6064
// Assert
6165
expect(commits).toStrictEqual([
62-
{ sha: 'Commit1', author: 'Author1', date: 'Date1', message: 'First Change On Feature' },
63-
{ sha: 'Commit2', author: 'Author1', date: 'Date2', message: 'Second Change On Feature' },
64-
{ sha: 'Commit3', author: 'Author1', date: 'Date3', message: 'Third Change On Feature' },
66+
{ sha: sha1, author: 'Author1', date: 'Date1', message: 'First Change On Feature' },
67+
{ sha: sha2, author: 'Author1', date: 'Date2', message: 'Second Change On Feature' },
68+
{ sha: sha3, author: 'Author1', date: 'Date3', message: 'Third Change On Feature' },
6569
]);
6670

6771
expect(gitCommandSpy).toHaveBeenCalledTimes(1);
@@ -80,9 +84,11 @@ describe('getChangedFiles', () => {
8084

8185
it('should return changed files between commits', () => {
8286
// Arrange
87+
const firstSha = '1'.repeat(40);
88+
const lastSha = '3'.repeat(40);
8389
const commits = [
84-
{ sha: 'Commit1', author: '', date: '', message: '' },
85-
{ sha: 'Commit3', author: '', date: '', message: '' },
90+
{ sha: firstSha, author: '', date: '', message: '' },
91+
{ sha: lastSha, author: '', date: '', message: '' },
8692
];
8793

8894
// Act
@@ -92,12 +98,13 @@ describe('getChangedFiles', () => {
9298
expect(changedFiles).toStrictEqual(['file1.txt', 'folder/file2.txt']);
9399

94100
expect(gitCommandSpy).toHaveBeenCalledTimes(1);
95-
expect(gitCommandSpy).toHaveBeenCalledWith(`git diff --name-only Commit1~..Commit3`);
101+
expect(gitCommandSpy).toHaveBeenCalledWith(`git diff --name-only ${firstSha}~..${lastSha}`);
96102
});
97103

98104
it('should handle only one commit', () => {
99105
// Arrange
100-
const commits = [{ sha: 'Commit1', author: '', date: '', message: '' }];
106+
const onlySha = '1'.repeat(40);
107+
const commits = [{ sha: onlySha, author: '', date: '', message: '' }];
101108

102109
// Act
103110
const changedFiles = getChangedFiles(commits);
@@ -106,6 +113,36 @@ describe('getChangedFiles', () => {
106113
expect(changedFiles).toStrictEqual(['file1.txt', 'folder/file2.txt']);
107114

108115
expect(gitCommandSpy).toHaveBeenCalledTimes(1);
109-
expect(gitCommandSpy).toHaveBeenCalledWith(`git diff --name-only Commit1~..Commit1`);
116+
expect(gitCommandSpy).toHaveBeenCalledWith(`git diff --name-only ${onlySha}~..${onlySha}`);
117+
});
118+
});
119+
120+
const VALID_SHA = 'a'.repeat(40);
121+
const VALID_JSON = JSON.stringify({ sha: VALID_SHA, author: 'test', date: 'now', message: 'msg' });
122+
123+
describe('parseBaseCommit', () => {
124+
it('should return undefined for undefined input', () => {
125+
expect(parseBaseCommit(undefined)).toBeUndefined();
126+
});
127+
128+
it('should return undefined for empty string', () => {
129+
expect(parseBaseCommit('')).toBeUndefined();
130+
});
131+
132+
it('should accept a plain SHA string', () => {
133+
expect(parseBaseCommit(VALID_SHA)).toBe(VALID_SHA);
134+
});
135+
136+
it('should extract sha from a JSON commit object', () => {
137+
expect(parseBaseCommit(VALID_JSON)).toBe(VALID_SHA);
138+
});
139+
140+
it('should throw on an invalid SHA string', () => {
141+
expect(() => parseBaseCommit('not-a-sha')).toThrow('Invalid base commit SHA');
142+
});
143+
144+
it('should throw when JSON contains an invalid sha field', () => {
145+
const badJson = JSON.stringify({ sha: 'bad', author: 'test', date: 'now', message: 'msg' });
146+
expect(() => parseBaseCommit(badJson)).toThrow('Invalid base commit SHA');
110147
});
111148
});

0 commit comments

Comments
 (0)