refactor!: remove legacy get-commits, get-latest-commit and get-changed-files

CONTRIBUTING.md

@@ -55,7 +55,7 @@ To work on the library, run the cli from the root of your testing repo:
 ```sh
 cd /path/to/testing-repo-for-github-actions
 npx tsx /path/to/apify-test-tools/bin/main.ts --help
-npx tsx /path/to/apify-test-tools/bin/main.ts get-commits --target-branch master --source-branch feat/testing-feature-branch
+npx tsx /path/to/apify-test-tools/bin/main.ts get-affected-actors --base master
 ```
 
 Or clone a test repo into `local-clone/` inside this repo and run `npm run start:dev -- <command>`.

bin/git.ts

@@ -1,5 +1,5 @@
 import { logger } from './logger.js';
-import type { Commit, Config } from './types.js';
+import type { Commit } from './types.js';
 import { GitCommandError, runGitCommand } from './utils.js';
 
 export const GIT_FORMAT_SEPARATOR = '»¦«';
@@ -9,19 +9,6 @@ const GIT_LOG_FORMAT = ['%H', '%aN<%aE>', '%aD', '%s'].join(GIT_FORMAT_SEPARATOR
 // so the Actor whose context it left wouldn't count as changed. --no-renames lists it as deleted and added instead.
 const LIST_CHANGED_PATHS = ['--name-only', '--no-renames'];
 
-/**
- * Gets the list of changed files between the given commits (inclusive), so it diffs from the parent of the oldest.
- */
-export const getChangedFiles = async (commits: Commit[]) => {
-    // getCommits never returns an empty list (it throws on an empty range, and the rerun check returns all
-    // commits when the base commit is the branch HEAD), so this signals a programmer error
-    if (commits.length === 0) {
-        throw new Error('Cannot get changed files: the commit list is empty. This should never happen.');
-    }
-
-    return getChangedFilesBetween(`${commits[0].sha}~`, commits[commits.length - 1].sha);
-};
-
 /**
  * Files that differ between two commits, i.e. changed by the commits in the range (fromRef, toRef]. fromRef's own
  * changes are not included: to include them, pass its parent, `${sha}~` (as getChangedFiles does).
@@ -56,29 +43,6 @@ export const findCommitSha = async (ref: string): Promise<string | undefined> =>
         throw error;
     });
 
-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;
-};
-
 /**
  * Commits reachable from toRef but not from fromRef, oldest first: the range (fromRef, toRef], so fromRef itself is
  * not included. To include it, pass its parent, `${sha}~`.
@@ -93,49 +57,6 @@ export const getCommitsBetween = async (fromRef: string, toRef: string): Promise
     return commits;
 };
 
-/**
- * Gets the commits between sourceBranch and targetBranch (exclusive).
- * - If baseCommit is provided, only returns commits after the baseCommit.
- */
-export const getCommits = async ({
-    sourceBranch,
-    targetBranch,
-    baseCommit,
-}: Pick<Config, 'sourceBranch' | 'targetBranch' | 'baseCommit'>): Promise<Commit[]> => {
-    const baseCommitSha = parseBaseCommit(baseCommit);
-    const commits = await getCommitsBetween(targetBranch, sourceBranch);
-    if (commits.length === 0) {
-        throw new Error(`${sourceBranch} has no commits that are not on ${targetBranch}.`);
-    }
-
-    // The last validated (base) commit being the branch HEAD means nothing new was pushed since the last
-    // validation — the dev reran the workflow (or force-pushed to the same state) to trigger a clean test
-    const headSha = commits[commits.length - 1]?.sha;
-    if (baseCommitSha !== undefined && baseCommitSha === headSha) {
-        logger.info(
-            `Detected rerun with the same commit that we already validated. This usually means the user wants to rerun the Action from scratch, ignoring last validated commit ${baseCommitSha} and returning all commits`,
-        );
-        logger.info(`Commits being returned: ${commits.map((c) => c.sha).join(', ')}`);
-        return commits;
-    }
-
-    const baseCommitIndex = commits.findIndex((commit) => commit.sha === baseCommitSha);
-
-    const hasBaseCommit = baseCommitIndex !== -1;
-    if (hasBaseCommit) {
-        const commitsUpToBaseCommit = commits.slice(baseCommitIndex + 1);
-        logger.info(
-            `Found base commit ${baseCommitSha} at index ${baseCommitIndex}, returning ${commitsUpToBaseCommit.length} commits after it`,
-        );
-        logger.info(`Commits being returned: ${commitsUpToBaseCommit.map((c) => c.sha).join(', ')}`);
-        return commitsUpToBaseCommit;
-    }
-
-    logger.info(`Base commit ${baseCommitSha} not found in the commit range, returning all ${commits.length} commits`);
-    logger.info(`Commits being returned: ${commits.map((c) => c.sha).join(', ')}`);
-    return commits;
-};
-
 export const parseCommit = (commitString: string): Commit => {
     const splits = commitString.split(GIT_FORMAT_SEPARATOR);
     if (splits.length !== 4) {
@@ -223,6 +144,8 @@ export const getRepoName = (repoUrl: string): string => {
     return normalizeRepoUrl(repoUrl).split('/').pop()!;
 };
 
+const SHA_REGEX = /^[0-9a-f]{40}$/i;
+
 // Sent by e.g. GitHub as the "before" commit of a push that created the branch
 const ZERO_SHA_REGEX = /^0{40}$/;
 
@@ -232,31 +155,27 @@ const ZERO_SHA_REGEX = /^0{40}$/;
  * would rebuild the latest build of every Actor and post it to Slack, so every problem is an error.
  */
 export const resolveReleaseBaseCommit = async (baseCommit: string, head = 'HEAD'): Promise<string> => {
-    const sha = parseBaseCommit(baseCommit);
-    if (!sha) {
-        throw new Error('A base commit is required for release. See the "Releasing Actors" section of docs/cli.md.');
+    if (!SHA_REGEX.test(baseCommit)) {
+        throw new Error(
+            `Invalid base commit SHA: "${baseCommit}". It should be a 40-character hexadecimal string. ` +
+                'See the "Releasing Actors" section of docs/cli.md.',
+        );
     }
+    const sha = baseCommit;
     if (ZERO_SHA_REGEX.test(sha)) {
         throw new Error(
             `Base commit is ${sha}, which means the branch was just created and there is no previous release to diff against. ` +
                 `Rerun with the base commit set to the last commit before the changes you want to release.`,
         );
     }
-    // --quiet suppresses Git's error text for a missing commit, but Git still exits with code 1.
-    const verifiedSha = await runGitCommand(['rev-parse', '--verify', '--quiet', `${sha}^{commit}`], {
-        operation: 'verify the release base commit in local Git history',
-    }).catch((error: unknown) => {
-        if (error instanceof GitCommandError && error.exitCode === 1) return '';
-        throw error;
-    });
-    if (!verifiedSha) {
+    if (!(await findCommitSha(sha))) {
         throw new Error(
             `Base commit ${sha} is not in the local git history. Either the checkout is shallow ` +
                 `(fetch the full history, e.g. fetch-depth: 0 in actions/checkout) or the branch was force-pushed.`,
         );
     }
     // git merge-base A B outputs the common ancestor. If that equals A, then A is an ancestor of B.
-    const mergeBase = await runGitCommand(['merge-base', verifiedSha, head], {
+    const mergeBase = await runGitCommand(['merge-base', sha, head], {
         operation: `check whether the release base commit is an ancestor of ${head}`,
     }).catch((error: unknown) => {
         if (error instanceof GitCommandError && error.exitCode === 1) return '';

bin/main.ts

@@ -9,7 +9,6 @@ import { hideBin } from 'yargs/helpers';
 import { getAffectedActors } from './affected-actors.js';
 import { buildActors, DEFAULT_VERSION_KEYWORD, deleteOldBuilds } from './build.js';
 import { runBuildsFromLocal } from './build-from-local.js';
-import { getChangedFiles, getCommits } from './git.js';
 import { logger, logLevels } from './logger.js';
 import { notify, notifyOnChoices } from './notify.js';
 import { writeJson } from './output.js';
@@ -24,18 +23,11 @@ const middlewares = [(args: { logLevel: (typeof logLevels)[number] }) => logger.
 
 /*
  * Option groups shared across commands. Each is a plain object passed to `.options()`, so a command's
- * builder composes them by chaining (`y.options(gitRangeOptions).options(actorSelectionOptions)`)
+ * builder composes them by chaining (`y.options(affectedRangeOptions).options(actorSelectionOptions)`)
  * instead of nesting wrapper functions. `as const` keeps literals like `demandOption: true` so yargs
  * can infer non-optional types; `satisfies` validates the shape without widening it.
  */
 
-/** Commit range to inspect: everything on `source-branch` since it diverged from `target-branch`. */
-const gitRangeOptions = {
-    'target-branch': { type: 'string', demandOption: true },
-    'source-branch': { type: 'string', demandOption: true },
-    'base-commit': { type: 'string', demandOption: false },
-} as const satisfies Record<string, Options>;
-
 /**
  * Where to build the Actors from. Defaults to the `origin` remote, which is then checked against each
  * Actor's default version. Passing it explicitly skips that check (see assertRepoUrlMatchesDefaultVersion).
@@ -47,8 +39,7 @@ const repoUrlOptions = {
 /**
  * Actor-selection flags, applied to every command that reads the actor config so a caller can
  * narrow the set it operates on (e.g. two-stage releases: `--ignore X`, then `--actors X`).
- * Kept separate from `gitRangeOptions` so the read-only git commands don't advertise flags they ignore.
- */
+ *  */
 const actorSelectionOptions = {
     actors: { type: 'string', array: true, default: [] as string[] },
     ignore: { type: 'string', array: true, default: [] as string[] },
@@ -174,36 +165,6 @@ await yargs()
     .scriptName('public-actors-utils')
     .options(globalOptions)
     .middleware(middlewares)
-    .command(
-        'get-commits',
-        '',
-        (y) => y.options(gitRangeOptions),
-        async (args) => {
-            const commits = await getCommits(args);
-            writeJson(commits);
-        },
-    )
-    .command(
-        'get-latest-commit',
-        '',
-        (y) => y.options(gitRangeOptions),
-        async (args) => {
-            const commits = await getCommits(args);
-            if (commits.length > 0) {
-                writeJson(commits[commits.length - 1]);
-            }
-        },
-    )
-    .command(
-        'get-changed-files',
-        '',
-        (y) => y.options(gitRangeOptions),
-        async (args) => {
-            const commits = await getCommits(args);
-            const changedFiles = await getChangedFiles(commits);
-            writeJson(changedFiles);
-        },
-    )
     .command(
         'get-actor-configs',
         '',

bin/types.ts

@@ -1,15 +1,6 @@
 import type { ActorJsonPaths } from './utils/config/actor-json.js';
 import type { ExistingDir, RelativeDir } from './utils/path/repo-relative.js';
 
-export interface Config {
-    targetBranch: string;
-    sourceBranch: string;
-    baseCommit?: string;
-    actors: string[];
-    ignore: string[];
-    testFilesGlob?: string;
-}
-
 /** The changes from `fromRef` (excluded) to `toRef`, compared as trees like `git diff fromRef toRef`. */
 export interface GitRange {
     fromRef: string;

docs/cli.md

@@ -447,10 +447,3 @@ Then run the tests against the builds with [`test`](#running-tests-test), as oft
 ```bash
 TESTER_APIFY_TOKEN=apify_api_yyy npx apify-test-tools test < builds.json
 ```
-
-## Git range commands
-
-`get-commits`, `get-latest-commit` and `get-changed-files` print the commits, the newest commit, or the changed files
-on `--source-branch` since it diverged from `--target-branch` (optionally only after `--base-commit`).
-[`get-affected-actors`](#finding-affected-actors-get-affected-actors) reports the same data together with the
-affected Actors, so prefer it.

test/bundle/cli-bundle.test.ts

@@ -46,8 +46,6 @@ const runCliWithInput = (input: string, ...args: string[]) => {
 
 const runCli = (...args: string[]) => runCliWithInput('', ...args);
 
-const gitRange = ['--source-branch', 'feature', '--target-branch', 'main'];
-
 describe('CLI bundle', () => {
     // Two Actors on `main`; `feature` changes only tester/foo and the root README.
     beforeAll(async () => {
@@ -91,9 +89,6 @@ describe('CLI bundle', () => {
         const { status, stdout } = runCli('--help');
         expect(status).toBe(0);
         for (const command of [
-            'get-commits',
-            'get-latest-commit',
-            'get-changed-files',
             'get-actor-configs',
             'get-affected-actors',
             'test [filters..]',
@@ -106,27 +101,6 @@ describe('CLI bundle', () => {
         }
     });
 
-    it('get-commits returns the commits on the source branch', () => {
-        const { status, stdout, stderr } = runCli('get-commits', ...gitRange);
-        expect(status, stderr).toBe(0);
-        expect(JSON.parse(stdout).map((commit: { message: string }) => commit.message)).toEqual([
-            'fix(foo): change foo',
-            'docs: readme',
-        ]);
-    });
-
-    it('get-latest-commit returns the head of the source branch', () => {
-        const { status, stdout, stderr } = runCli('get-latest-commit', ...gitRange);
-        expect(status, stderr).toBe(0);
-        expect(JSON.parse(stdout)).toMatchObject({ sha: headSha, message: 'docs: readme' });
-    });
-
-    it('get-changed-files returns the files changed on the source branch', () => {
-        const { status, stdout, stderr } = runCli('get-changed-files', ...gitRange);
-        expect(status, stderr).toBe(0);
-        expect(JSON.parse(stdout)).toEqual(['README.md', 'actors/foo/main.js']);
-    });
-
     it('get-actor-configs reads both Actors from the config file', () => {
         const { status, stdout, stderr } = runCli('get-actor-configs');
         expect(status, stderr).toBe(0);
@@ -149,6 +123,14 @@ describe('CLI bundle', () => {
         });
     });
 
+    it.each(['info', 'silent'])('keeps JSON results intact with --log-level %s', (level) => {
+        const { status, stdout, stderr } = runCli('get-affected-actors', '--base', 'main', '--log-level', level);
+        expect(status, stderr).toBe(0);
+        expect(stderr).toEqual(level === 'silent' ? '' : expect.stringContaining('Changed files'));
+        expect(JSON.parse(stdout)).toMatchObject({ head: headSha });
+        expect(stdout.endsWith('\n')).toBe(true);
+    });
+
     it('build --dry-run with nothing to build needs no token', () => {
         const { status, stdout, stderr } = runCliWithInput(
             JSON.stringify({ actors: [] }),