diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b84cd1..9bef826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file. + ## 0.5.7 - **not yet released** ### 🚀 Features @@ -14,7 +15,6 @@ All notable changes to this project will be documented in this file. - Bump version to test beta release ([1322d31](https://github.com/apify/apify-test-tools/commit/1322d31873b6d43e16a68e97bdc358752f813f79)) by [@metalwarrior665](https://github.com/metalwarrior665) - # Changelog @@ -91,4 +91,4 @@ feat: feat: add maxRetriesPerRequest test ### Cli - fix: parsing commits -- feat: add `--workspace` cli option \ No newline at end of file +- feat: add `--workspace` cli option diff --git a/bin/git.ts b/bin/git.ts index ceb69d6..707893f 100644 --- a/bin/git.ts +++ b/bin/git.ts @@ -17,11 +17,35 @@ export const getChangedFiles = (commits: Commit[]) => { return changedFiles; }; +const SHA_REGEX = /^[0-9a-f]{40}$/i; + +/** + * + * @param shaOrCommit Supports both a SHA string or a Commit object in JSON format. Can be empty. + * @returns The SHA string if valid, otherwise throws an error. + */ +export const parseBaseCommit = (shaOrCommit: string | undefined): string | undefined => { + if (!shaOrCommit) return undefined; + let sha: string; + if (shaOrCommit.startsWith('{')) { + sha = (JSON.parse(shaOrCommit) as Commit).sha; + } else { + sha = shaOrCommit; + } + if (!SHA_REGEX.test(sha)) { + throw new Error( + `Invalid base commit SHA: "${sha}". It should be a 40-character hexadecimal string, instead got input: "${shaOrCommit}".`, + ); + } + return sha; +}; + /** * Gets the commits between sourceBranch and targetBranch (exclusive). * - If baseCommit is provided, only returns commits after the baseCommit. */ -export const getCommits = ({ sourceBranch, targetBranch, baseCommit: baseCommitSha }: Config): Commit[] => { +export const getCommits = ({ sourceBranch, targetBranch, baseCommit }: Config): Commit[] => { + const baseCommitSha = parseBaseCommit(baseCommit); const commitsStrings = spawnCommandInGhWorkspace( `git log --pretty=format:'${GIT_LOG_FORMAT}' ${targetBranch}..${sourceBranch}`, ).split('\n'); diff --git a/lib/extend-expect.ts b/lib/extend-expect.ts index 7995066..666d289 100644 --- a/lib/extend-expect.ts +++ b/lib/extend-expect.ts @@ -291,7 +291,6 @@ const isWithinInterval = ( diffs.pass = false; diffs.actual.push(`${intervalOption}=${actual}`); diffs.expected.push(`${intervalOption}=${expected}`); - // eslint-disable-next-line consistent-return return false; } } else if (typeof expected === 'object') { @@ -301,7 +300,6 @@ const isWithinInterval = ( diffs.pass = false; diffs.actual.push(`${intervalOption}=${actual}`); diffs.expected.push(`${intervalOption}=<${min ?? ''},${max ?? ''}>`); - // eslint-disable-next-line consistent-return return false; } } diff --git a/test/unit/bin/git.test.ts b/test/unit/bin/git.test.ts index a3c7ba9..ea785a1 100644 --- a/test/unit/bin/git.test.ts +++ b/test/unit/bin/git.test.ts @@ -1,16 +1,20 @@ import type { MockInstance } from 'vitest'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { getChangedFiles, getCommits } from '../../../bin/git.js'; +import { getChangedFiles, getCommits, parseBaseCommit } from '../../../bin/git.js'; import * as Utils from '../../../bin/utils.js'; describe('getCommits', () => { const sourceBranch = 'feature-branch'; const targetBranch = 'main'; - const commit1 = 'Commit1»¦«Author1»¦«Date1»¦«First Change On Feature'; - const commit2 = 'Commit2»¦«Author1»¦«Date2»¦«Second Change On Feature'; - const commit3 = 'Commit3»¦«Author1»¦«Date3»¦«Third Change On Feature'; + const sha1 = '1'.repeat(40); + const sha2 = '2'.repeat(40); + const sha3 = '3'.repeat(40); + + const commit1 = `${sha1}»¦«Author1»¦«Date1»¦«First Change On Feature`; + const commit2 = `${sha2}»¦«Author1»¦«Date2»¦«Second Change On Feature`; + const commit3 = `${sha3}»¦«Author1»¦«Date3»¦«Third Change On Feature`; let gitCommandSpy: MockInstance; @@ -26,9 +30,9 @@ describe('getCommits', () => { // Assert expect(commits).toStrictEqual([ - { sha: 'Commit1', author: 'Author1', date: 'Date1', message: 'First Change On Feature' }, - { sha: 'Commit2', author: 'Author1', date: 'Date2', message: 'Second Change On Feature' }, - { sha: 'Commit3', author: 'Author1', date: 'Date3', message: 'Third Change On Feature' }, + { sha: sha1, author: 'Author1', date: 'Date1', message: 'First Change On Feature' }, + { sha: sha2, author: 'Author1', date: 'Date2', message: 'Second Change On Feature' }, + { sha: sha3, author: 'Author1', date: 'Date3', message: 'Third Change On Feature' }, ]); expect(gitCommandSpy).toHaveBeenCalledTimes(1); @@ -39,12 +43,12 @@ describe('getCommits', () => { it('should return commits after the base commit if provided', () => { // Act - const commits = getCommits({ sourceBranch, targetBranch, baseCommit: 'Commit1' }); + const commits = getCommits({ sourceBranch, targetBranch, baseCommit: sha1 }); // Assert expect(commits).toStrictEqual([ - { sha: 'Commit2', author: 'Author1', date: 'Date2', message: 'Second Change On Feature' }, - { sha: 'Commit3', author: 'Author1', date: 'Date3', message: 'Third Change On Feature' }, + { sha: sha2, author: 'Author1', date: 'Date2', message: 'Second Change On Feature' }, + { sha: sha3, author: 'Author1', date: 'Date3', message: 'Third Change On Feature' }, ]); expect(gitCommandSpy).toHaveBeenCalledTimes(1); @@ -55,13 +59,13 @@ describe('getCommits', () => { it('should return all commits if base commit is not found', () => { // Act - const commits = getCommits({ sourceBranch, targetBranch, baseCommit: 'NonExistingCommit' }); + const commits = getCommits({ sourceBranch, targetBranch, baseCommit: 'a'.repeat(40) }); // Assert expect(commits).toStrictEqual([ - { sha: 'Commit1', author: 'Author1', date: 'Date1', message: 'First Change On Feature' }, - { sha: 'Commit2', author: 'Author1', date: 'Date2', message: 'Second Change On Feature' }, - { sha: 'Commit3', author: 'Author1', date: 'Date3', message: 'Third Change On Feature' }, + { sha: sha1, author: 'Author1', date: 'Date1', message: 'First Change On Feature' }, + { sha: sha2, author: 'Author1', date: 'Date2', message: 'Second Change On Feature' }, + { sha: sha3, author: 'Author1', date: 'Date3', message: 'Third Change On Feature' }, ]); expect(gitCommandSpy).toHaveBeenCalledTimes(1); @@ -80,9 +84,11 @@ describe('getChangedFiles', () => { it('should return changed files between commits', () => { // Arrange + const firstSha = '1'.repeat(40); + const lastSha = '3'.repeat(40); const commits = [ - { sha: 'Commit1', author: '', date: '', message: '' }, - { sha: 'Commit3', author: '', date: '', message: '' }, + { sha: firstSha, author: '', date: '', message: '' }, + { sha: lastSha, author: '', date: '', message: '' }, ]; // Act @@ -92,12 +98,13 @@ describe('getChangedFiles', () => { expect(changedFiles).toStrictEqual(['file1.txt', 'folder/file2.txt']); expect(gitCommandSpy).toHaveBeenCalledTimes(1); - expect(gitCommandSpy).toHaveBeenCalledWith(`git diff --name-only Commit1~..Commit3`); + expect(gitCommandSpy).toHaveBeenCalledWith(`git diff --name-only ${firstSha}~..${lastSha}`); }); it('should handle only one commit', () => { // Arrange - const commits = [{ sha: 'Commit1', author: '', date: '', message: '' }]; + const onlySha = '1'.repeat(40); + const commits = [{ sha: onlySha, author: '', date: '', message: '' }]; // Act const changedFiles = getChangedFiles(commits); @@ -106,6 +113,36 @@ describe('getChangedFiles', () => { expect(changedFiles).toStrictEqual(['file1.txt', 'folder/file2.txt']); expect(gitCommandSpy).toHaveBeenCalledTimes(1); - expect(gitCommandSpy).toHaveBeenCalledWith(`git diff --name-only Commit1~..Commit1`); + expect(gitCommandSpy).toHaveBeenCalledWith(`git diff --name-only ${onlySha}~..${onlySha}`); + }); +}); + +const VALID_SHA = 'a'.repeat(40); +const VALID_JSON = JSON.stringify({ sha: VALID_SHA, author: 'test', date: 'now', message: 'msg' }); + +describe('parseBaseCommit', () => { + it('should return undefined for undefined input', () => { + expect(parseBaseCommit(undefined)).toBeUndefined(); + }); + + it('should return undefined for empty string', () => { + expect(parseBaseCommit('')).toBeUndefined(); + }); + + it('should accept a plain SHA string', () => { + expect(parseBaseCommit(VALID_SHA)).toBe(VALID_SHA); + }); + + it('should extract sha from a JSON commit object', () => { + expect(parseBaseCommit(VALID_JSON)).toBe(VALID_SHA); + }); + + it('should throw on an invalid SHA string', () => { + expect(() => parseBaseCommit('not-a-sha')).toThrow('Invalid base commit SHA'); + }); + + it('should throw when JSON contains an invalid sha field', () => { + const badJson = JSON.stringify({ sha: 'bad', author: 'test', date: 'now', message: 'msg' }); + expect(() => parseBaseCommit(badJson)).toThrow('Invalid base commit SHA'); }); });