Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions bin/actor-filtering.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import type { ActorConfig } from './types.js';

/**
* Restricts a set of actors to those selected via `--actors` and not excluded via `--ignore`.
* Both filters match on `actorFullName` (`owner/name`). `--actors` is applied first (empty means
* "all"), then `--ignore` removes from the result. A name that doesn't exist in the config throws —
* a malformed selection must never silently build/release/delete the wrong set. The caller is
* responsible for turning that into a non-zero exit.
*/
export function selectActors({ actors, ignore }: { actors: string[]; ignore: string[] }, actorConfigs: ActorConfig[]) {
const fullNames = actorConfigs.map((actor) => actor.actorFullName);
const missing = [...actors, ...ignore].filter((name) => !fullNames.includes(name));
if (missing.length > 0) {
throw new Error(`The following actors from the filter config do not exist: ${missing.join(', ')}`);
}

const afterOnly = actors.length
? actorConfigs.filter((actor) => actors.includes(actor.actorFullName))
: actorConfigs;
return afterOnly.filter((actor) => !ignore.includes(actor.actorFullName));
}
6 changes: 5 additions & 1 deletion bin/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ const fetchAllBranchCommits = (sourceBranch: string, targetBranch: string): Comm
* Gets the commits between sourceBranch and targetBranch (exclusive).
* - If baseCommit is provided, only returns commits after the baseCommit.
*/
export const getCommits = ({ sourceBranch, targetBranch, baseCommit }: Config): Commit[] => {
export const getCommits = ({
sourceBranch,
targetBranch,
baseCommit,
}: Pick<Config, 'sourceBranch' | 'targetBranch' | 'baseCommit'>): Commit[] => {
const baseCommitSha = parseBaseCommit(baseCommit);
const commits = fetchAllBranchCommits(sourceBranch, targetBranch);

Expand Down
126 changes: 65 additions & 61 deletions bin/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { readConfigFile, setCwd, spawnCommandInGhWorkspace } from './utils.js';
*/
const middlewares = [setCwd];

const buildOptions = (y: Argv) => {
export const buildOptions = <T>(y: Argv<T>) => {
return y
.option('target-branch', {
type: 'string',
Expand All @@ -37,27 +37,44 @@ const buildOptions = (y: Argv) => {
})
.option('base-commit', {
type: 'string',
demandOption: false,
});
};

const resolveChangedActors = async (
{ targetBranch, sourceBranch, baseCommit }: Config,
{ isLatest }: { isLatest: boolean },
) => {
const actorConfigs = await readConfigFile();
/**
* 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 `buildOptions` so the read-only git commands don't advertise flags they ignore.
*/
export const actorSelectionOptions = <T>(y: Argv<T>) => {
return y
.option('actors', {
type: 'string',
array: true,
default: [] as string[],
})
.option('ignore', {
type: 'string',
array: true,
default: [] as string[],
});
};

const resolveChangedActors = async (config: Config, { isLatest }: { isLatest: boolean }) => {
const actorConfigs = await readConfigFile(config);

// This is an optimization for the common case where a branch only has cosmetic changes but had to merge in
// This is an optimization for the common case where a branch only has cosmetic changes but had to smerge in
// functional changes from master (being up-to-date is a CI requirement). Master is already validated, and
// since the branch has no functional changes of its own, there is nothing new to validate.
// Exception: if the branch has any functional changes alongside the merge, we must re-test — even
// individually validated changes can have novel interactions when combined.
if (hasMergeFromTarget(sourceBranch, targetBranch)) {
if (hasMergeFromTarget(config.sourceBranch, config.targetBranch)) {
console.error(
'[MERGE-FROM-TARGET-OPTIMIZATION]: There is merge from target branch, checking if there are no functional changes in our own branch. If so, we can skip tests',
);
const branchOnlyFiles = getBranchOnlyChangedFiles(sourceBranch, targetBranch);
const branchOnlyFiles = getBranchOnlyChangedFiles(config.sourceBranch, config.targetBranch);
// Omit baseCommit to get full branch history. Validated functional commits can still interact with merged ones
const allBranchCommits = getCommits({ sourceBranch, targetBranch, baseCommit: undefined });
const allBranchCommits = getCommits({ ...config, baseCommit: undefined });
const branchOnlyActorsChanged = getChangedActors({
filepathsChanged: branchOnlyFiles,
actorConfigs,
Expand All @@ -73,7 +90,7 @@ const resolveChangedActors = async (
}

// If the optimization doesn't apply, we check all branch commits including merges for full coverage. We don't reuse the merge optimization results because here we can apply baseCommit and check merge commits (they might be functional or just cosmetic)
const commits = getCommits({ targetBranch, sourceBranch, baseCommit });
const commits = getCommits(config);
const changedFiles = getChangedFiles(commits);
return getChangedActors({ filepathsChanged: changedFiles, actorConfigs, isLatest, commits });
};
Expand Down Expand Up @@ -103,22 +120,19 @@ await yargs()
const changedFiles = getChangedFiles(commits);
console.log(JSON.stringify(changedFiles));
})
.command('get-actor-configs', '', actorSelectionOptions, async ({ actors, ignore }) => {
const actorConfigs = await readConfigFile({ actors, ignore });
console.log(JSON.stringify(actorConfigs));
})
.command(
'get-actor-configs',
'get-affected-actors',
'',
(_) => _,
async () => {
const actorConfigs = await readConfigFile();
console.log(JSON.stringify(actorConfigs));
(args) => actorSelectionOptions(buildOptions(args)),
async (config) => {
const actorsChanged = await resolveChangedActors(config, { isLatest: false });
console.log(JSON.stringify(actorsChanged));
},
)
.command('get-affected-actors', '', buildOptions, async ({ targetBranch, sourceBranch, baseCommit }) => {
const actorsChanged = await resolveChangedActors(
{ targetBranch, sourceBranch, baseCommit },
{ isLatest: false },
);
console.log(JSON.stringify(actorsChanged));
})
.command(
'report-tests',
'',
Expand All @@ -135,12 +149,9 @@ await yargs()
.command(
'build',
'',
(args) => buildOptions(args).option('dry-run', { type: 'boolean', default: false }),
async ({ targetBranch, sourceBranch, baseCommit, dryRun, useDockerCache }) => {
const actorsChanged = await resolveChangedActors(
{ targetBranch, sourceBranch, baseCommit },
{ isLatest: false },
);
(args) => actorSelectionOptions(buildOptions(args)).option('dry-run', { type: 'boolean', default: false }),
async (config) => {
const actorsChanged = await resolveChangedActors(config, { isLatest: false });
// https://github.com/apify-store/google-maps#:actors/lukaskrivka_google-maps-with-contact-details
// git@github.com:apify-store/google-maps#:actors/lukaskrivka_google-maps-with-contact-details
const repoUrl = spawnCommandInGhWorkspace(`git remote get-url origin`).replace(
Expand All @@ -151,9 +162,9 @@ await yargs()
const builds = await runBuilds({
repoUrl,
actorConfigs: actorsChanged,
branch: sourceBranch.replace('origin/', ''),
dryRun,
useDockerCache,
branch: config.sourceBranch.replace('origin/', ''),
dryRun: config.dryRun,
useDockerCache: config.useDockerCache,
});
console.log(JSON.stringify(builds));
},
Expand All @@ -162,7 +173,7 @@ await yargs()
'release',
'',
(args) =>
args
actorSelectionOptions(args)
.option('push-event-path', { type: 'string', demandOption: true })
.option('dry-run', { type: 'boolean', default: false })
.option('report-slack-channel', { type: 'string' })
Expand All @@ -173,7 +184,7 @@ await yargs()
args.pushEventPath,
);
const isLatest = true;
const actorConfigs = await readConfigFile();
const actorConfigs = await readConfigFile(args);
const actorsChanged = getChangedActors({
filepathsChanged: changedFiles,
actorConfigs,
Expand Down Expand Up @@ -206,37 +217,30 @@ await yargs()
.command(
'build-from-local',
'',
(args) =>
args
.option('actors', {
type: 'string',
description:
'Comma-separated actor names (owner/name) to build. Defaults to all actors in the repo.',
})
.option('dry-run', { type: 'boolean', default: false }),
async ({ actors, dryRun }) => {
const allActorConfigs = await readConfigFile();
const actorConfigs = actors
? actors.split(',').map((name) => {
const trimmed = name.trim();
const config = allActorConfigs.find((c) => c.actorFullName === trimmed);
if (!config) throw new Error(`Actor "${trimmed}" not found in repo`);
return config;
})
: allActorConfigs;
(args) => actorSelectionOptions(args).option('dry-run', { type: 'boolean', default: false }),
async ({ actors, ignore, dryRun }) => {
const actorConfigs = await readConfigFile({ actors, ignore });
const builds = await runBuildsFromLocal({ actorConfigs, dryRun });
console.log(JSON.stringify(builds));
},
)
.command(
'delete-old-builds',
'',
(_) => _,
async () => {
const actorConfigs = await readConfigFile();
await deleteOldBuilds(actorConfigs);
},
)
.command('delete-old-builds', '', actorSelectionOptions, async ({ actors, ignore }) => {
const actorConfigs = await readConfigFile({ actors, ignore });
await deleteOldBuilds(actorConfigs);
})
.strictCommands()
.demandCommand(1, 'Command is required')
.fail((msg, err, yargsInstance) => {
// Errors thrown from a command handler (e.g. an unknown actor passed to --actors/--ignore,
// or a missing config file) arrive here as `err`. A malformed selection must fail loudly
// rather than silently operate on the wrong set of actors — print the message, no stack.
if (err) {
console.error(`[ERROR]: ${err.message}`);
} else {
// Argument-parsing/validation failure — keep yargs' usage output.
console.error(yargsInstance.help());
console.error(`\n${msg}`);
}
process.exit(1);
})
.parse(hideBin(process.argv));
2 changes: 2 additions & 0 deletions bin/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ export interface Config {
sourceBranch: string;
baseCommit?: string;
workspace?: string;
actors: string[];
ignore: string[];
}

export type Commit = {
Expand Down
5 changes: 3 additions & 2 deletions bin/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { ActorVersionSourceFile } from 'apify-client';

import { SOURCE_FILE_FORMATS } from '@apify/consts';

import { selectActors } from './actor-filtering.js';
import { isPathWithinScope } from './path-utils.js';
import type { ActorConfig, ActorConfigFile } from './types.js';

Expand Down Expand Up @@ -113,7 +114,7 @@ const findOverlappingContextPaths = (contextPaths: string[]): [string, string] |
return undefined;
};

export const readConfigFile = async (): Promise<ActorConfig[]> => {
export const readConfigFile = async (selection: { actors: string[]; ignore: string[] }): Promise<ActorConfig[]> => {
let raw: string;
try {
raw = await fs.readFile(CONFIG_FILE_NAME, 'utf-8');
Expand Down Expand Up @@ -225,7 +226,7 @@ export const readConfigFile = async (): Promise<ActorConfig[]> => {
});
}

return actorConfigs;
return selectActors(selection, actorConfigs);
};

export const setCwd = ({ workspace }: { workspace: string | undefined }) => {
Expand Down
52 changes: 52 additions & 0 deletions test/unit/bin/actor-filtering.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { describe, expect, it } from 'vitest';

import { selectActors } from '../../../bin/actor-filtering.js';
import type { ActorConfig } from '../../../bin/types.js';

const actor = (actorFullName: string): ActorConfig => ({
actorFullName,
folder: actorFullName.split('/')[1],
tokenEnvVar: 'TOKEN',
dockerContextDir: '.',
contextPaths: [],
});

const configs = [actor('owner/a'), actor('owner/b'), actor('owner/c')];
const names = (result: ActorConfig[]) => result.map((c) => c.actorFullName);

describe('selectActors', () => {
it('returns all actors when neither filter is set', () => {
expect(names(selectActors({ actors: [], ignore: [] }, configs))).toStrictEqual([
'owner/a',
'owner/b',
'owner/c',
]);
});

it('keeps only the actors listed in --actors', () => {
expect(names(selectActors({ actors: ['owner/a', 'owner/c'], ignore: [] }, configs))).toStrictEqual([
'owner/a',
'owner/c',
]);
});

it('drops the actors listed in --ignore', () => {
expect(names(selectActors({ actors: [], ignore: ['owner/b'] }, configs))).toStrictEqual(['owner/a', 'owner/c']);
});

it('applies --actors first, then removes --ignore from that subset', () => {
expect(names(selectActors({ actors: ['owner/a', 'owner/b'], ignore: ['owner/b'] }, configs))).toStrictEqual([
'owner/a',
]);
});

it('can select down to nothing when --actors and --ignore overlap fully', () => {
expect(selectActors({ actors: ['owner/a'], ignore: ['owner/a'] }, configs)).toStrictEqual([]);
});

it('throws listing every unknown name across both filters', () => {
expect(() => selectActors({ actors: ['owner/x'], ignore: ['owner/y'] }, configs)).toThrow(
'The following actors from the filter config do not exist: owner/x, owner/y',
);
});
});
Loading
Loading