diff --git a/README.md b/README.md index cee23f2..84ceeb9 100644 --- a/README.md +++ b/README.md @@ -4,29 +4,99 @@ ## Getting Started -1. Install the package `npm i -D apify-test-tools` - - because it uses [annotate](https://vitest.dev/guide/test-context.html#annotate), `vitest` version to be at least `3.2.0` - - make sure that `target` and `module` in your `tsconfig.json`'s `compilerOptions` are set to `ES2022` -2. create test directories: `mkdir -p test/platform/core` - - core (hourly) tests should go to `test/platform/core` - - daily tests should go to `test/platform` -3. setup github worklows TODO +### 1. Install the package -File structure: +```bash +npm i -D apify-test-tools +``` + +- Requires `vitest` version `3.2.0` or later (uses [annotate](https://vitest.dev/guide/test-context.html#annotate)) +- Make sure `target` and `module` in your `tsconfig.json`'s `compilerOptions` are set to `ES2022` + +### 2. Create the config file + +Every repo that uses `apify-test-tools` must have an `apify-test-tools.config.json` file at the root. This file tells the tool which actors live in the repo, how to identify them, and which token to use. + +```json +{ + "actors": [ + { + "folder": "actors/web-scraper", + "actorFullName": "myteam/web-scraper", + "tokenEnvVar": "APIFY_TOKEN_MYTEAM" + }, + { + "folder": "actors/email-sender", + "actorFullName": "myteam/email-sender", + "tokenEnvVar": "APIFY_TOKEN_MYTEAM", + "overrideActorContext": ["actors/email-sender", "packages/shared"] + } + ] +} +``` + +Each entry has: + +| Field | Required | Description | +| ---------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `folder` | yes | Relative path from repo root to the actor's own project directory — the folder that directly contains `.actor/actor.json` (i.e. `/.actor/actor.json`), the actor's README/CHANGELOG, and its source. Use `"."` for a single-actor repo where `.actor/` is at the root. | +| `actorFullName` | yes | Full actor identifier in `owner/name` format (e.g. `"apify/web-scraper"`). This is the source of truth for the actor name — the `name` field in `actor.json` is not used. | +| `tokenEnvVar` | yes | Name of the environment variable holding the Apify API token for this actor. No fallback — if the env var is not set at build time, the build fails. | +| `overrideActorContext` | no | Array of paths (relative to repo root) that define which files are relevant to this actor. When set, replaces the `dockerContextDir` from `actor.json` for change detection. Useful when an actor depends on shared packages outside its Docker build context. Entries must not be prefixes of one another (e.g. `["", "code"]` or `["actors", "actors/foo"]` are rejected). The actor's own `folder` is always part of its context — if none of the listed entries reach it, it's added automatically. | + +### 3. Set up actor folders + +Each actor in the config must have a `.actor/actor.json` file. The `dockerContextDir` field in `actor.json` defines the build context boundary — this is what the tool uses to determine which files can affect the actor's build. ``` -google-maps +my-repo +├── apify-test-tools.config.json ├── actors -└── src +│ ├── web-scraper +│ │ ├── .actor +│ │ │ └── actor.json +│ │ └── src/ +│ └── email-sender +│ ├── .actor +│ │ └── actor.json +│ └── src/ └── test ├── unit └── platform - ├── core <- Core tests need to be inside core directory + ├── core <- Core (hourly) tests │ └── core.test.ts - ├── some.test.ts <- Other tests can be defined anywhere inside platform directory + ├── some.test.ts <- Daily tests can be anywhere inside platform/ └── some-other.test.ts ``` +For a single-actor repo, set `"folder": "."` in the config and place `.actor/actor.json` at the repo root. + +### Change detection + +When a PR is opened or code is pushed, the tool determines which actors need to be built and tested based on the changed files. For each changed file, for each actor: + +1. **Sibling exclusion** — files inside another actor's `folder` are excluded first. This prevents an actor with broad context from being triggered by changes that belong to a sibling actor. +2. **CHANGELOG classification** — a `CHANGELOG.md` file is always `cosmetic` (only triggers a release build, not tests), for every actor, regardless of context or folder. (See [issue #106](https://github.com/apify/apify-test-tools/issues/106).) +3. **Context matching** — the file must fall within one of the actor's context paths (`dockerContextDir` from `actor.json` by default, or `overrideActorContext` from config if set). Files outside every context path are skipped. +4. **Hardcoded ignore list, context-aware** — the file path is first "hoisted" relative to the context path it matched (e.g. a standalone actor's own `.eslintrc` is checked as just `.eslintrc`, not the full repo-root-relative path), then checked against repo-level dev file patterns (`.vscode/`, `.gitignore`, `.husky/`, `.eslintrc`, `eslint.config.mjs`, `.prettierrc`, `.editorconfig`). There's no hardcoded special-casing for legacy `code/`/`shared/` layouts — repos that need those directories treated as top-level must list them explicitly in `overrideActorContext`. +5. **`.dockerignore` filtering** — if a `.dockerignore` exists at the root of the actor's `dockerContextDir`, matching files are ignored. Patterns are resolved relative to `dockerContextDir`, matching Docker's own behavior. +6. **README classification** — a `README.md` file is `cosmetic` (only triggers a release build, not tests) if it lives inside the actor's own `folder`; otherwise it's ignored entirely, since it isn't documentation for this actor. +7. **Cosmetic JSON classification** — `.json` files inside the actor's own `.actor/` directory with only cosmetic schema changes (whitespace, key ordering) only trigger a release build. +8. **Functional** — everything else triggers both build and tests. + +### 4. Create test directories + +```bash +mkdir -p test/platform/core +``` + +- Core (hourly) tests go in `test/platform/core` +- Daily tests go anywhere in `test/platform` + +### 5. Set up GitHub workflows + +See the [GitHub workflows](#github-worklows) section below. + ## Github worklows There should be 4 GH workflow files in `.github/workflows`. @@ -343,7 +413,7 @@ GITHUB_WORKSPACE=. \ Remove `--dry-run` to actually trigger builds and update the branch names/ The command outputs a JSON array of build objects to stdout: ```json -[{ "buildId": "...", "actorId": "...", "buildNumber": "...", "actorName": "john.doe/my-actor" }] +[{ "buildId": "...", "actorRawId": "...", "buildNumber": "...", "actorFullName": "john.doe/my-actor" }] ``` #### Build from local source (no push needed) diff --git a/bin/build-from-local.ts b/bin/build-from-local.ts index 0c3d3f2..cdb26a0 100644 --- a/bin/build-from-local.ts +++ b/bin/build-from-local.ts @@ -5,14 +5,10 @@ import path from 'node:path'; import type { ActorVersionSourceFile } from 'apify-client'; import { ApifyBuilder, waitAndSummarizeBuilds } from './build.js'; +import { buildDockerIgnoreMatcher } from './dockerignore.js'; +import { isPathWithinScope } from './path-utils.js'; import type { ActorConfig, BuildData } from './types.js'; -import { - getDockerignoredPaths, - getGitignoredPaths, - isOutsideDir, - listRepoFilePaths, - toActorVersionSourceFile, -} from './utils.js'; +import { getGitignoredPaths, isOutsideDir, listRepoFilePaths, toActorVersionSourceFile } from './utils.js'; // JUST IN CASE. File patterns that commonly hold credentials — never ship these into a build, regardless // of sourceType or of whether the repo's .gitignore happens to list them. Everything else that should be @@ -34,11 +30,13 @@ export const collectSourceFiles = async (actorName: string, actorDir: string): P const contextAbsDir = rawContextDir ? path.resolve(absActorDir, '.actor', rawContextDir) : undefined; const isMonorepoActor = !!contextAbsDir && isOutsideDir(contextAbsDir, absActorDir); - const collectRootDir = isMonorepoActor ? contextAbsDir! : absActorDir; - const keptFilePaths = collectNonIgnoredFiles(collectRootDir, repoRoot); + const dockerContextDirAbs = isMonorepoActor ? contextAbsDir! : absActorDir; + const keptFilePaths = collectNonIgnoredFiles(dockerContextDirAbs, repoRoot); if (!isMonorepoActor) { - return Promise.all(keptFilePaths.map(async (filePath) => toActorVersionSourceFile(filePath, collectRootDir))); + return Promise.all( + keptFilePaths.map(async (filePath) => toActorVersionSourceFile(filePath, dockerContextDirAbs)), + ); } const { tempDir, filePaths } = await flattenMonorepoContext( @@ -47,7 +45,6 @@ export const collectSourceFiles = async (actorName: string, actorDir: string): P contextAbsDir!, actorJson, keptFilePaths, - repoRoot, ); try { return await Promise.all(filePaths.map(async (filePath) => toActorVersionSourceFile(filePath, tempDir))); @@ -61,21 +58,21 @@ export const collectSourceFiles = async (actorName: string, actorDir: string): P // manual directory walk — nested .gitignore files, `.git/info/exclude`, and global excludes are // all honored since this delegates to git itself instead of re-implementing gitignore matching, // and .git/ is never walked because git never lists its own internals here. `.dockerignore` at -// `rootDir` (the Docker build context) is honored the same way, since those files would never -// reach a real Docker build either. `.actor/` (the Actor specification folder) is always kept +// `dockerContextDir` (the Docker build context) is honored the same way, since those files would +// never reach a real Docker build either. `.actor/` (the Actor specification folder) is always kept // regardless of .gitignore/.dockerignore, matching Apify CLI's own behavior. Files matching the // hardcoded secret-pattern backstop (keys, certs, .env variants) are dropped unconditionally, // .actor/ included, since those should never ship regardless of what the ignore files say. -export const collectNonIgnoredFiles = (rootDir: string, repoRoot: string): string[] => { - const relativePaths = listRepoFilePaths(repoRoot, rootDir); +export const collectNonIgnoredFiles = (dockerContextDir: string, repoRoot: string): string[] => { + const relativePaths = listRepoFilePaths(repoRoot, dockerContextDir); const ignoredPaths = getGitignoredPaths(relativePaths); const rootRelativePaths = new Map( relativePaths.map((relPath) => [ relPath, - path.relative(rootDir, path.join(repoRoot, relPath)).split(path.sep).join('/'), + path.relative(dockerContextDir, path.join(repoRoot, relPath)).split(path.sep).join('/'), ]), ); - const dockerIgnoredPaths = getDockerignoredPaths(rootDir, [...rootRelativePaths.values()]); + const isDockerIgnored = buildDockerIgnoreMatcher(dockerContextDir); return relativePaths .filter((relPath) => { @@ -83,7 +80,7 @@ export const collectNonIgnoredFiles = (rootDir: string, repoRoot: string): strin const isUnderActorDir = relPath.split('/').includes('.actor'); if (isUnderActorDir) return true; if (ignoredPaths.has(relPath)) return false; - return !dockerIgnoredPaths.has(rootRelativePaths.get(relPath)!); + return !isDockerIgnored(rootRelativePaths.get(relPath)!); }) .map((relPath) => path.join(repoRoot, relPath)); }; @@ -92,8 +89,8 @@ export const collectNonIgnoredFiles = (rootDir: string, repoRoot: string): strin // collect the actor directory of a monorepo actor — the platform would reject any path // escaping it. Fix: create a temporary "flattened" directory where: // - the Docker context's non-ignored files (repo root) are copied to the temp dir root -// - the actor's .actor/ directory is overlaid at the temp dir root (through the same -// gitignore/secret-pattern filter as the rest of the context — see collectNonIgnoredFiles) +// - the actor's own .actor/ directory (already present within those same non-ignored files) +// is overlaid at the temp dir root instead of its original nested position // - actor.json path fields are rewritten to be relative to the new location // // Result: the collected root IS the Docker context, .actor/ is at that root, and @@ -105,7 +102,6 @@ export const flattenMonorepoContext = async ( contextAbsDir: string, actorJson: Record, keptContextFiles: string[], - repoRoot: string, ): Promise<{ tempDir: string; filePaths: string[] }> => { console.error(`[${actorName}]: monorepo actor detected — flattening from Docker context`); @@ -124,11 +120,13 @@ export const flattenMonorepoContext = async ( }), ); - // Step 2: overlay the actor's .actor/ directory at the temp dir root. collectNonIgnoredFiles - // always keeps .actor/ paths regardless of .gitignore, but still drops the hardcoded secret - // patterns — so this isn't a raw copy, a stray secret file living inside .actor/ is still dropped. + // Step 2: overlay the actor's own .actor/ directory at the temp dir root. Its files are already + // present in keptContextFiles (collectNonIgnoredFiles keeps .actor/ paths unconditionally — see + // step 1 above) — pick out this actor's own subset (ignoring any sibling actors' .actor/ folders + // that might also appear in the broader context) and hoist each to its position relative to + // .actor/ itself, so it lands under tempDir/.actor/ instead of its original nested location. const actorMetaDir = path.join(absActorDir, '.actor'); - const keptActorFiles = collectNonIgnoredFiles(actorMetaDir, repoRoot); + const keptActorFiles = keptContextFiles.filter((absFilePath) => isPathWithinScope(absFilePath, actorMetaDir)); await Promise.all( keptActorFiles.map(async (absFilePath) => { const relPath = path.relative(actorMetaDir, absFilePath); @@ -196,26 +194,29 @@ export const runBuildsFromLocal = async ({ }): Promise => { if (dryRun) { console.error('[DRY RUN] Would build from local source:'); - for (const { actorName, folder } of actorConfigs) { - console.error(` ${actorName} (${folder})`); + for (const { actorFullName, folder } of actorConfigs) { + console.error(` ${actorFullName} (${folder})`); } - return actorConfigs.map(({ actorName }) => ({ + return actorConfigs.map(({ actorFullName }) => ({ buildId: 'dry-run', - actorId: 'dry-run', + actorRawId: 'dry-run', buildNumber: '0.98.0', - actorName, + actorFullName, })); } console.error('========================================='); console.error('STARTED LOCAL BUILDS:'); + const buildersByActorFullName = new Map( + actorConfigs.map((actorConfig) => [actorConfig.actorFullName, ApifyBuilder.fromActorConfig(actorConfig)]), + ); const startedBuilds = await Promise.all( - actorConfigs.map(async ({ actorName, folder }) => { - const builder = ApifyBuilder.fromActorName(actorName); - const sourceFiles = await collectSourceFiles(actorName, folder); + actorConfigs.map(async ({ actorFullName, folder }) => { + const builder = buildersByActorFullName.get(actorFullName)!; + const sourceFiles = await collectSourceFiles(actorFullName, folder); return builder.startActorBuildFromSourceFiles(sourceFiles); }), ); - return waitAndSummarizeBuilds(startedBuilds, 'LOCAL BUILDS'); + return waitAndSummarizeBuilds(startedBuilds, buildersByActorFullName, 'LOCAL BUILDS'); }; diff --git a/bin/build.ts b/bin/build.ts index 8cf9f16..e01e05b 100644 --- a/bin/build.ts +++ b/bin/build.ts @@ -9,13 +9,13 @@ type BuildPrActorOptions = { buildTag?: string; versionNumber: string; gitRepoUrl: string; - actorName: string; + actorConfig: ActorConfig; useDockerCache: boolean; }; export class ApifyBuilder { private constructor( private readonly apifyClient: ApifyClient, - private readonly actorName: string, + private readonly actorFullName: string, ) {} // Usually 'latest' but not necessarily (can be e.g. 'version-0') @@ -24,35 +24,35 @@ export class ApifyBuilder { defaultVersionNumber: string; defaultBuildTag: string; }> => { - const actorClient = this.apifyClient.actor(this.actorName); + const actorClient = this.apifyClient.actor(this.actorFullName); const actorInfo = await actorClient.get(); if (!actorInfo) { throw new Error( - `[${this.actorName}] not found. It is not published or we are missing token to access it privately or its name is misspelled`, + `[${this.actorFullName}] not found. It is not published or we are missing token to access it privately or its name is misspelled`, ); } const defaultBuildTag = actorInfo.defaultRunOptions.build; - console.error(`Default build tag for ${this.actorName} is ${defaultBuildTag}`); + console.error(`Default build tag for ${this.actorFullName} is ${defaultBuildTag}`); // We could technically allow this but in most cases this is accidentally set wrongly and there is a workaround if (defaultBuildTag.match(/\d+\.\d+\.\d+/)) { throw new Error( - `[${this.actorName}] Default build is a build number, not a tag. While this could work, ` + + `[${this.actorFullName}] Default build is a build number, not a tag. While this could work, ` + `we want to have a default as tag so this is often an accidental misconfiguration from the dev`, ); } // I reported that buildNumber should probably not be optional if (!actorInfo.taggedBuilds?.[defaultBuildTag]?.buildNumber) { throw new Error( - `[${this.actorName}] No build found for tag "${defaultBuildTag}". ` + + `[${this.actorFullName}] No build found for tag "${defaultBuildTag}". ` + `The first build must be triggered manually on the platform before CI can take over.`, ); } const defaultBuildNumber = actorInfo.taggedBuilds![defaultBuildTag].buildNumber!; const defaultVersionNumber = defaultBuildNumber.match(/(\d+\.\d+)\.\d+/)![1]; - console.error(`Default version for ${this.actorName} is ${defaultVersionNumber}`); + console.error(`Default version for ${this.actorFullName} is ${defaultVersionNumber}`); return { defaultBuildNumber, defaultVersionNumber, defaultBuildTag }; }; @@ -63,11 +63,11 @@ export class ApifyBuilder { gitRepoUrl, useDockerCache, }: BuildPrActorOptions): Promise => { - const actorClient = this.apifyClient.actor(this.actorName); + const actorClient = this.apifyClient.actor(this.actorFullName); const actorInfo = await actorClient.get(); if (!actorInfo) { throw new Error( - `No actor named '${this.actorName}' was found on the platform. If this` + + `No actor named '${this.actorFullName}' was found on the platform. If this` + ' is unexpected, make sure the actor you are targeting is spelled the' + ' same as the folder in the repository.', ); @@ -97,17 +97,17 @@ export class ApifyBuilder { // We also get back actId so the testing actor can both match by actor ID and name const { id, actId, buildNumber } = await actorClient.build(versionNumber, { useCache: useDockerCache }); - console.error(`[${this.actorName}]: ${id} (${buildNumber})`); - return { buildId: id, actorId: actId, buildNumber, actorName: this.actorName }; + console.error(`[${this.actorFullName}]: ${id} (${buildNumber})`); + return { buildId: id, actorRawId: actId, buildNumber, actorFullName: this.actorFullName }; }; startActorBuildFromSourceFiles = async (sourceFiles: ActorVersionSourceFile[]): Promise => { const ZIP_VERSION = '0.98'; - const actorClient = this.apifyClient.actor(this.actorName); + const actorClient = this.apifyClient.actor(this.actorFullName); const actorInfo = await actorClient.get(); if (!actorInfo) { throw new Error( - `No actor named '${this.actorName}' was found on the platform. If this` + + `No actor named '${this.actorFullName}' was found on the platform. If this` + ' is unexpected, make sure the actor you are targeting is spelled the' + ' same as the folder in the repository.', ); @@ -130,49 +130,39 @@ export class ApifyBuilder { } const { id, actId, buildNumber } = await actorClient.build(ZIP_VERSION, { useCache: false }); - console.error(`[${this.actorName}]: ${id} (${buildNumber})`); - return { buildId: id, actorId: actId, buildNumber, actorName: this.actorName }; + console.error(`[${this.actorFullName}]: ${id} (${buildNumber})`); + return { buildId: id, actorRawId: actId, buildNumber, actorFullName: this.actorFullName }; }; - waitForBuildToFinish = async (buildId: string, actorName: string): Promise => { + waitForBuildToFinish = async (buildId: string): Promise => { const build = await this.apifyClient.build(buildId).waitForFinish(); const versionNumber = build.buildNumber; if (build.status === 'FAILED' || build.status === 'TIMED-OUT') { - console.error(`[${this.actorName}]: ${versionNumber}`); + console.error(`[${this.actorFullName}]: ${versionNumber}`); try { const log = await this.apifyClient.build(buildId).log().get(); const logTail = log?.split('\n').slice(-40).join('\n'); console.error(`\n--- BUILD LOG (last 40 lines) ---\n${logTail}\n---`); } catch (err) { - console.error(`[${this.actorName}]: Failed to fetch build log: ${err}`); + console.error(`[${this.actorFullName}]: Failed to fetch build log: ${err}`); } throw new Error( - `[BUILD][${actorName}]: Build ${buildId} (${versionNumber}) failed. ` + + `[BUILD][${this.actorFullName}]: Build ${buildId} (${versionNumber}) failed. ` + `Not continuing with other builds and tests.`, ); } - console.error(`[${this.actorName}]: ${versionNumber}`); + console.error(`[${this.actorFullName}]: ${versionNumber}`); return build; }; - /** - * Create ApifyBuilder with actor owner's token - */ - static fromActorName = (actorName: string): ApifyBuilder => { - const username = actorName.split('/')[0]; - // GitHib secrets only allow word characters (alphanum + underscore) - const usernameInGitHubSecretsFormat = username.replaceAll(/\W/g, '_').toUpperCase(); - const usernameEnvVar = `APIFY_TOKEN_${usernameInGitHubSecretsFormat}`; - const token = process.env[usernameEnvVar]; + static fromActorConfig = (actorConfig: ActorConfig): ApifyBuilder => { + const { actorFullName, tokenEnvVar } = actorConfig; + const token = process.env[tokenEnvVar]; if (!token) { - throw new Error( - `Cannot find Apify API token for username: ${username}. ` + - `Have you set secret env var to this GitHub repo with key: ${usernameEnvVar}?`, - ); + throw new Error(`Env var ${tokenEnvVar} is not set (needed for actor "${actorFullName}").`); } const apifyClient = new ApifyClient({ token }); - const builder = new ApifyBuilder(apifyClient, actorName); - return builder; + return new ApifyBuilder(apifyClient, actorFullName); }; /** @@ -204,7 +194,7 @@ export class ApifyBuilder { const DEFAULT_DAYS_BACK_PROD_VERSIONS = 30; const DEFAULT_DAYS_BACK_DEVEL = 7; - const actorInfo = (await this.apifyClient.actor(this.actorName).get())!; + const actorInfo = (await this.apifyClient.actor(this.actorFullName).get())!; // 'devel' used to be hardcoded for testing version 0.99, once we get rid of this tag everywhere, we can remove this code const taggedDevelBuildNumber: string | undefined = actorInfo.taggedBuilds!.devel?.buildNumber; @@ -216,12 +206,10 @@ export class ApifyBuilder { tag, })); - const { items } = await this.apifyClient.actor(this.actorName).builds().list(); + const { items } = await this.apifyClient.actor(this.actorFullName).builds().list(); // Deleting default build throws an error, so we skip it - const { defaultBuildNumber, defaultBuildTag } = await ApifyBuilder.fromActorName( - this.actorName, - ).getDefaultVersionAndTag(); + const { defaultBuildNumber, defaultBuildTag } = await this.getDefaultVersionAndTag(); const daysAgoUnixProd = Date.now() - DEFAULT_DAYS_BACK_PROD_VERSIONS * 24 * 60 * 60 * 1000; const daysAgoUnixDevel = Date.now() - DEFAULT_DAYS_BACK_DEVEL * 24 * 60 * 60 * 1000; @@ -231,7 +219,7 @@ export class ApifyBuilder { const buildsToDelete = (items as CorrectBuildColletionItem[]).filter((build) => { if (build.buildNumber === defaultBuildNumber) { console.error( - `[DELETE OLD BUILDS][${this.actorName}]: Skipping default build ${defaultBuildNumber} (${defaultBuildTag}). ` + + `[DELETE OLD BUILDS][${this.actorFullName}]: Skipping default build ${defaultBuildNumber} (${defaultBuildTag}). ` + `We never delete default builds`, ); return false; @@ -242,7 +230,7 @@ export class ApifyBuilder { ); if (protectedTagFound) { console.error( - `[DELETE OLD BUILDS][${this.actorName}]: Skipping protected build ${protectedTagFound.buildNumber} (${protectedTagFound.tag}).`, + `[DELETE OLD BUILDS][${this.actorFullName}]: Skipping protected build ${protectedTagFound.buildNumber} (${protectedTagFound.tag}).`, ); return false; } @@ -251,7 +239,7 @@ export class ApifyBuilder { const shouldDeleteDevelBuild = build.startedAt.getTime() < daysAgoUnixDevel; if (shouldDeleteDevelBuild) { console.error( - `[DELETE OLD BUILDS][${this.actorName}]: Removing olf devel build ${taggedDevelBuildNumber}.`, + `[DELETE OLD BUILDS][${this.actorFullName}]: Removing olf devel build ${taggedDevelBuildNumber}.`, ); } return shouldDeleteDevelBuild; @@ -260,7 +248,7 @@ export class ApifyBuilder { }); console.error( - `[DELETE OLD BUILDS][${this.actorName}]: Deleting ${buildsToDelete.length} old builds that are non-default and ` + + `[DELETE OLD BUILDS][${this.actorFullName}]: Deleting ${buildsToDelete.length} old builds that are non-default and ` + `older than 30 days from total ${items.length}`, ); for (const build of buildsToDelete) { @@ -269,20 +257,24 @@ export class ApifyBuilder { } } -export const waitAndSummarizeBuilds = async (startedBuilds: BuildData[], label: string): Promise => { +export const waitAndSummarizeBuilds = async ( + startedBuilds: BuildData[], + buildersMap: Map, + label: string, +): Promise => { console.error('========================================='); console.error(`FINISHED ${label}:`); await Promise.all( startedBuilds.map(async (buildData) => { - const builder = ApifyBuilder.fromActorName(buildData.actorName); - await builder.waitForBuildToFinish(buildData.buildId, buildData.actorName); + const builder = buildersMap.get(buildData.actorFullName)!; + await builder.waitForBuildToFinish(buildData.buildId); }), ); console.error('========================================='); console.error('SUMMARY:'); - for (const buildData of startedBuilds.sort((a, b) => a.actorName.localeCompare(b.actorName))) { - console.error(`[${buildData.actorName}]: ${buildData.buildNumber}`); + for (const buildData of startedBuilds.sort((a, b) => a.actorFullName.localeCompare(b.actorFullName))) { + console.error(`[${buildData.actorFullName}]: ${buildData.buildNumber}`); } console.error('========================================='); @@ -308,13 +300,13 @@ export const runBuilds = async ({ }: RunBuildsOptions) => { const buildConfigs: BuildPrActorOptions[] = []; - for (const { actorName, folder } of actorConfigs) { + for (const actorConfig of actorConfigs) { let versionNumber: string; let buildTag: string | undefined; if (isLatest) { const { defaultVersionNumber, defaultBuildTag } = - await ApifyBuilder.fromActorName(actorName).getDefaultVersionAndTag(); + await ApifyBuilder.fromActorConfig(actorConfig).getDefaultVersionAndTag(); versionNumber = defaultVersionNumber; buildTag = defaultBuildTag; } else { @@ -323,30 +315,34 @@ export const runBuilds = async ({ // Depending on if these are miniactors or standaloneActors let gitRepoUrl = `${repoUrl}#${branch}`; - if (folder) { - gitRepoUrl = `${gitRepoUrl}:${folder}`; + if (actorConfig.folder) { + gitRepoUrl = `${gitRepoUrl}:${actorConfig.folder}`; } - buildConfigs.push({ actorName, gitRepoUrl, versionNumber, buildTag, useDockerCache }); + buildConfigs.push({ actorConfig, gitRepoUrl, versionNumber, buildTag, useDockerCache }); } if (dryRun) { return buildConfigs; } + + const buildersByActorFullName = new Map( + actorConfigs.map((actorConfig) => [actorConfig.actorFullName, ApifyBuilder.fromActorConfig(actorConfig)]), + ); console.error('========================================='); console.error('STARTED BUILDS:'); const startedBuilds = await Promise.all( buildConfigs.map(async (buildConfig) => { - const builder = ApifyBuilder.fromActorName(buildConfig.actorName); + const builder = buildersByActorFullName.get(buildConfig.actorConfig.actorFullName)!; const buildData = await builder.startActorBuild(buildConfig); return buildData; }), ); - return waitAndSummarizeBuilds(startedBuilds, 'BUILDS'); + return waitAndSummarizeBuilds(startedBuilds, buildersByActorFullName, 'BUILDS'); }; export const deleteOldBuilds = async (actorConfigs: ActorConfig[]) => { - for (const { actorName } of actorConfigs) { - await ApifyBuilder.fromActorName(actorName).deleteOldBuilds(); + for (const actorConfig of actorConfigs) { + await ApifyBuilder.fromActorConfig(actorConfig).deleteOldBuilds(); } }; diff --git a/bin/diff-changes.ts b/bin/diff-changes.ts index 7af55e0..47e73d8 100644 --- a/bin/diff-changes.ts +++ b/bin/diff-changes.ts @@ -1,4 +1,6 @@ import { isCosmeticOnlyJsonSchemaChange } from './diff-json-schema.js'; +import { type DockerIgnoreMatcher, loadDockerIgnore } from './dockerignore.js'; +import { findContainingScope, hoistPath, isPathWithinScope } from './path-utils.js'; import type { ActorConfig, Commit } from './types.js'; interface ShouldBuildAndTestOptions { @@ -8,89 +10,160 @@ interface ShouldBuildAndTestOptions { commits: Commit[]; } -export const maybeParseActorFolder = ( - lowercaseFilePath: string, -): { isActorFolder: true; actorName: string } | { isActorFolder: false } => { - const match = lowercaseFilePath.match(/^(?:standalone-)?actors\/([^/]+)\/.+/); - if (match) { - // Some usernames weirdly use underscores, e.g. google_maps_email_extractor_standby-contact-details-scraper so we only need replace the last one - return { isActorFolder: true, actorName: match[1].replace(/_(?=[^_]*$)/, '/') }; - } - return { isActorFolder: false }; -}; +const IGNORED_TOP_LEVEL_FILES = [ + '.vscode/', + '.gitignore', + '.husky/', + '.eslintrc', + 'eslint.config.mjs', + '.prettierrc', + '.editorconfig', +]; + +// Expects an already-hoisted path (relative to the matched context entry, see findContainingScope). +const isIgnoredTopLevelFile = (hoistedLowercaseFilePath: string): boolean => + IGNORED_TOP_LEVEL_FILES.some((pattern) => hoistedLowercaseFilePath.startsWith(pattern)); + +type FileChangeForActor = + | { impact: 'ignored' } + | { impact: 'outside-context' } + | { impact: 'cosmetic'; semanticallyVerified: boolean } + | { impact: 'functional' }; /** - * Also works for folders + * Classify a single file change for a single actor. + * + * Steps (in order): + * 1. CHANGELOG.md, by filename, anywhere → cosmetic. There is a single repo-wide shared changelog, + * not one per actor, so it applies to every actor regardless of context/folder. + * 2. Context matching (actorConfig.contextPaths) → outside-context if no match + * 3. Hardcoded ignore list, checked against the path hoisted relative to the matched context entry → ignored + * 4. .dockerignore filtering (patterns relative to dockerContextDir), skipped for the actor's own `.actor/` + * dir → ignored if matched + * 5. README.md by filename → cosmetic if inside the actor's own folder, otherwise ignored + * 6. .json inside the actor's own `.actor/` dir with only cosmetic schema diffs → cosmetic (semantically verified) + * 7. Everything else → functional */ -const isIgnoredTopLevelFile = (lowercaseFilePath: string) => { - // On top level, we should only have dev-only readme and .actor/ is just for apify push CLI (real Actor configs are in /actors) - const IGNORED_TOP_LEVEL_FILES = [ - '.vscode/', - '.gitignore', - 'readme.md', - '.husky/', - '.eslintrc', - 'eslint.config.mjs', - '.prettierrc', - '.editorconfig', - '.actor/', - ]; - // Strip out deprecated /code and /shared folders, treat them as top-level code - const sanitizedLowercaseFilePath = lowercaseFilePath.replace(/^code\//, '').replace(/^shared\//, ''); - - return IGNORED_TOP_LEVEL_FILES.some((ignoredFile) => sanitizedLowercaseFilePath.startsWith(ignoredFile)); -}; - -type FileChange = - | { impact: 'ignored' } - // Only things that influence how the Actor looks - e.g. README and CHANGELOG files, schema titles, descriptions, reordering, etc. We only need to rebuild on release - | { impact: 'cosmetic'; semanticallyVerified: boolean; includes: 'all-actors' | ActorConfig } - // Influences how the Actor works - we need to run tests - | { - impact: 'functional'; - includes: 'all-actors' | ActorConfig; - }; - -const classifyFileChange = (originalFilePath: string, actorConfigs: ActorConfig[], commits: Commit[]): FileChange => { - // Lowercase for case-insensitive matching; keep original for git show (case-sensitive on Linux) +const classifyFileChange = ( + originalFilePath: string, + actorConfig: ActorConfig, + commits: Commit[], + dockerIgnoreMatcher: DockerIgnoreMatcher, +): FileChangeForActor => { const lowercaseFilePath = originalFilePath.toLowerCase(); - if (isIgnoredTopLevelFile(lowercaseFilePath)) { + + // TODO: hardcodes that there's a single repo-wide changelog belonging to every actor. Should instead + // be derived from parsing actor.json (readme, changelog, schema paths), see + // https://github.com/apify/apify-test-tools/issues/106 + if (lowercaseFilePath.endsWith('changelog.md')) { + return { impact: 'cosmetic', semanticallyVerified: false }; + } + + const lowercaseContextPaths = actorConfig.contextPaths.map((contextPath) => contextPath.toLowerCase()); + + const matchedContext = findContainingScope(lowercaseFilePath, lowercaseContextPaths); + if (matchedContext === undefined) { + return { impact: 'outside-context' }; + } + + const hoistedFilePath = hoistPath(lowercaseFilePath, matchedContext); + if (isIgnoredTopLevelFile(hoistedFilePath)) { return { impact: 'ignored' }; } - if (lowercaseFilePath.endsWith('changelog.md')) { - return { impact: 'cosmetic', semanticallyVerified: false, includes: 'all-actors' }; + const lowercaseFolder = actorConfig.folder.toLowerCase(); + const actorDotDir = lowercaseFolder ? `${lowercaseFolder}/.actor` : '.actor'; + const isUnderActorDotDir = isPathWithinScope(lowercaseFilePath, actorDotDir); + + // .actor/ can legitimately be listed in .dockerignore (the Apify platform evaluates it before + // the Docker build, so excluding it from the build context is a valid caching optimization) — + // that shouldn't cause changes to .actor/ itself to be ignored here. + if (!isUnderActorDotDir && dockerIgnoreMatcher(originalFilePath)) { + return { impact: 'ignored' }; } - const actorFolderInfo = maybeParseActorFolder(lowercaseFilePath); - if (actorFolderInfo.isActorFolder) { - const actorConfigChanged = actorConfigs.find( - ({ actorName }) => actorName.toLowerCase() === actorFolderInfo.actorName, - ); - // This is some super weird case that happened once in the past but I don't remember the context anymore - if (actorConfigChanged === undefined) { - console.error( - 'SHOULD NEVER HAPPEN: changes was found in an actor folder which no longer exists in the current commit, skipping this file', - { - actorName: actorFolderInfo.actorName, - lowercaseFilePath, - }, - ); - return { impact: 'ignored' }; - } - if (lowercaseFilePath.endsWith('readme.md')) { - return { impact: 'cosmetic', semanticallyVerified: false, includes: actorConfigChanged }; + const isInActorFolder = isPathWithinScope(lowercaseFilePath, lowercaseFolder); + + if (lowercaseFilePath.endsWith('readme.md')) { + return isInActorFolder ? { impact: 'cosmetic', semanticallyVerified: false } : { impact: 'ignored' }; + } + + if (lowercaseFilePath.endsWith('.json') && isUnderActorDotDir) { + const isCosmetic = isCosmeticOnlyJsonSchemaChange(commits, originalFilePath); + if (isCosmetic) { + return { impact: 'cosmetic', semanticallyVerified: true }; } - // originalFilePath must be used here (not lowercaseFilePath) — git show is case-sensitive on Linux - if (lowercaseFilePath.endsWith('.json') && isCosmeticOnlyJsonSchemaChange(commits, originalFilePath)) { - return { impact: 'cosmetic', semanticallyVerified: true, includes: actorConfigChanged }; + } + + return { impact: 'functional' }; +}; + +/** + * Check if a file falls inside another actor's folder. + * Root actors (folder === "") never exclude files from siblings. + */ +const isExcludedBySibling = (lowercaseFilePath: string, actor: ActorConfig, allActors: ActorConfig[]): boolean => { + return allActors.some( + (other) => + other.folder !== actor.folder && + other.folder !== '' && + isPathWithinScope(lowercaseFilePath, other.folder.toLowerCase()), + ); +}; + +type ActorChangeEntry = { + actorConfig: ActorConfig; + files: string[]; +}; + +type ChangeGroup = { actors: string[]; files: string[] }; + +/** + * Maps each changed file to the set of actor names it triggered a change for. + */ +const buildFileToActorsMap = (actorsChangedMap: Map): Map> => { + const fileToActors = new Map>(); + for (const { actorConfig, files } of actorsChangedMap.values()) { + for (const file of files) { + const actors = fileToActors.get(file) ?? new Set(); + actors.add(actorConfig.actorFullName); + fileToActors.set(file, actors); } + } + return fileToActors; +}; - return { impact: 'functional', includes: actorConfigChanged }; +/** + * Groups files by their identical actor-set (files triggering a change for the exact same + * actors are grouped together), then orders the groups by descending actor-set size + * (most-shared groups first), breaking ties alphabetically by actor names. + */ +const groupFilesByActorSet = (fileToActors: Map>): ChangeGroup[] => { + const groupsByKey = new Map(); + for (const [file, actorsSet] of fileToActors) { + const actors = Array.from(actorsSet).sort(); + const key = actors.join(','); + const group = groupsByKey.get(key) ?? { actors, files: [] }; + group.files.push(file); + groupsByKey.set(key, group); } - // For any other files, we assume they can interact with the code - return { impact: 'functional', includes: 'all-actors' }; + return Array.from(groupsByKey.values()).sort((groupA, groupB) => { + if (groupB.actors.length !== groupA.actors.length) { + return groupB.actors.length - groupA.actors.length; + } + return groupA.actors.join(',').localeCompare(groupB.actors.join(',')); + }); +}; + +const logChangeGroups = (groups: ChangeGroup[]): void => { + for (const { actors, files } of groups) { + if (actors.length > 1) { + console.error(`[DIFF]: Shared changes for actors ${actors.join(', ')}: ${files.join(', ')}`); + } else { + console.error(`[DIFF]: Changes specific to actor ${actors[0]}: ${files.join(', ')}`); + } + } }; export const getChangedActors = ({ @@ -99,72 +172,40 @@ export const getChangedActors = ({ isLatest = false, commits, }: ShouldBuildAndTestOptions): ActorConfig[] => { - // folder -> ActorConfig - const actorsChangedMap = new Map(); - - const actorConfigsWithoutStandalone = actorConfigs.filter(({ isStandalone }) => !isStandalone); + const actorsChangedMap = new Map(); - for (const originalFilePath of filepathsChanged) { - const fileChange = classifyFileChange(originalFilePath, actorConfigs, commits); - if (fileChange.impact === 'ignored') { - continue; - } + for (const actorConfig of actorConfigs) { + const dockerIgnoreMatcher = loadDockerIgnore(actorConfig.dockerContextDir); - if (fileChange.impact === 'cosmetic' && !isLatest) { - continue; - } + for (const originalFilePath of filepathsChanged) { + const lowercaseFilePath = originalFilePath.toLowerCase(); - if (fileChange.includes !== 'all-actors') { - actorsChangedMap.set(fileChange.includes.folder, fileChange.includes); - } else if (fileChange.includes === 'all-actors') { - // Standalone Actors are handled always via specific actors change, not all-actors - for (const actorConfig of actorConfigsWithoutStandalone) { - actorsChangedMap.set(actorConfig.folder, actorConfig); + if (isExcludedBySibling(lowercaseFilePath, actorConfig, actorConfigs)) { + continue; } - } - } - const actorsChanged = Array.from(actorsChangedMap.values()); + const change = classifyFileChange(originalFilePath, actorConfig, commits, dockerIgnoreMatcher); - // All below here is just for logging - const formatFiles = (files: string[]) => (files.length > 0 ? files.join(', ') : ''); + if (change.impact === 'ignored' || change.impact === 'outside-context') continue; + if (change.impact === 'cosmetic' && !isLatest) continue; - const ignoredFilesChanged = filepathsChanged.filter( - (file) => classifyFileChange(file, actorConfigs, commits).impact === 'ignored', - ); - console.error(`[DIFF]: Ignored files (don't trigger test or build): ${formatFiles(ignoredFilesChanged)}`); - - const cosmeticChanges = filepathsChanged - .map((file) => ({ file, change: classifyFileChange(file, actorConfigs, commits) })) - .filter(({ change }) => change.impact === 'cosmetic') as { - file: string; - change: Extract; - }[]; - const semanticallyVerifiedFiles = cosmeticChanges - .filter(({ change }) => change.semanticallyVerified) - .map(({ file }) => file); - const inherentlyCosmeticFiles = cosmeticChanges - .filter(({ change }) => !change.semanticallyVerified) - .map(({ file }) => file); - console.error( - `[DIFF]: Cosmetic-only JSON schema changes (semantically verified, only trigger release build): ${formatFiles(semanticallyVerifiedFiles)}`, - ); - console.error( - `[DIFF]: Inherently cosmetic files (README, CHANGELOG — only trigger release build): ${formatFiles(inherentlyCosmeticFiles)}`, - ); + const entry = actorsChangedMap.get(actorConfig.folder) ?? { actorConfig, files: [] }; + entry.files.push(originalFilePath); + actorsChangedMap.set(actorConfig.folder, entry); + } + } - const functionalFilesChanged = filepathsChanged.filter( - (file) => classifyFileChange(file, actorConfigs, commits).impact === 'functional', - ); - console.error(`[DIFF]: Functional files (trigger test & release build): ${formatFiles(functionalFilesChanged)}`); + const actorsChanged = Array.from(actorsChangedMap.values()).map((entry) => entry.actorConfig); + + // Log changes grouped by actor set, so changes shared across actors are logged once + // instead of being repeated per actor. + const fileToActors = buildFileToActorsMap(actorsChangedMap); + const groups = groupFilesByActorSet(fileToActors); + logChangeGroups(groups); if (actorsChanged.length > 0) { - const miniactors = actorsChanged.filter((config) => !config.isStandalone).map((config) => config.actorName); - const standaloneActors = actorsChanged - .filter((config) => config.isStandalone) - .map((config) => config.actorName); - console.error(`[DIFF]: MiniActors to be built and tested: ${miniactors.join(', ')}`); - console.error(`[DIFF]: Standalone Actors to be built and tested: ${standaloneActors.join(', ')}`); + const actors = actorsChanged.map((config) => config.actorFullName); + console.error(`[DIFF]: Actors to be built and tested: ${actors.join(', ')}`); } else { console.error(`[DIFF]: No relevant files changed, skipping builds and tests`); } diff --git a/bin/dockerignore.ts b/bin/dockerignore.ts new file mode 100644 index 0000000..d12b310 --- /dev/null +++ b/bin/dockerignore.ts @@ -0,0 +1,50 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +import ignore from 'ignore'; + +import { hoistPath, isPathWithinScope } from './path-utils.js'; + +export type DockerIgnoreMatcher = (repoRelativePath: string) => boolean; + +// Docker normalizes each pattern before matching, so a leading "./" (as in the common "./node_modules" +// style) is a no-op for Docker. The `ignore` package has no such normalization — it treats "./" as +// literal pattern text that can never match a real path, so a .dockerignore written in that style +// would otherwise silently match nothing. Strip it here (after any negation prefix) so the pattern +// behaves the way Docker itself would apply it. +const normalizeDockerignorePattern = (line: string): string => line.replace(/^(!?)(?:\.\/)+/, '$1'); + +/** + * Reads `.dockerignore` from `absoluteRootDir` and returns a matcher for paths relative to + * `hoistFrom`. `hoistFrom` defaults to '', meaning callers already pass paths relative to + * `absoluteRootDir` directly — isPathWithinScope/hoistPath both treat '' as "matches everything" / + * identity, so the scope-check and hoist collapse to a no-op in that case, not via a branch. + * + * Returns a no-op matcher (always returns false) when the file is absent. + */ +export const buildDockerIgnoreMatcher = (absoluteRootDir: string, hoistFrom = ''): DockerIgnoreMatcher => { + let content: string; + try { + content = fs.readFileSync(path.join(absoluteRootDir, '.dockerignore'), 'utf-8'); + } catch { + return () => false; + } + + const matcher = ignore().add(content.split('\n').map(normalizeDockerignorePattern).join('\n')); + + return (filePath: string): boolean => { + if (!isPathWithinScope(filePath.toLowerCase(), hoistFrom.toLowerCase())) { + return false; + } + + return matcher.ignores(hoistPath(filePath, hoistFrom)); + }; +}; + +/** + * Load .dockerignore from the root of an actor's dockerContextDir and return a matcher + * that accepts repo-root-relative file paths. Patterns are resolved relative to + * dockerContextDir, matching Docker's own behavior. + */ +export const loadDockerIgnore = (dockerContextDir: string): DockerIgnoreMatcher => + buildDockerIgnoreMatcher(path.resolve(dockerContextDir), dockerContextDir); diff --git a/bin/main.ts b/bin/main.ts index 44d80bd..ee5fced 100644 --- a/bin/main.ts +++ b/bin/main.ts @@ -14,7 +14,7 @@ import { getPushData } from './github.js'; import { notifyToSlack } from './slack.js'; import { reportTestResults } from './test-report.js'; import type { Config } from './types.js'; -import { getRepoActors, setCwd, spawnCommandInGhWorkspace } from './utils.js'; +import { readConfigFile, setCwd, spawnCommandInGhWorkspace } from './utils.js'; /** * Middlewares to be run before every command execution @@ -44,7 +44,7 @@ const resolveChangedActors = async ( { targetBranch, sourceBranch, baseCommit }: Config, { isLatest }: { isLatest: boolean }, ) => { - const actorConfigs = await getRepoActors(); + const actorConfigs = await readConfigFile(); // This is an optimization for the common case where a branch only has cosmetic changes but had to merge in // functional changes from master (being up-to-date is a CI requirement). Master is already validated, and @@ -108,7 +108,7 @@ await yargs() '', (_) => _, async () => { - const actorConfigs = await getRepoActors(); + const actorConfigs = await readConfigFile(); console.log(JSON.stringify(actorConfigs)); }, ) @@ -173,7 +173,7 @@ await yargs() args.pushEventPath, ); const isLatest = true; - const actorConfigs = await getRepoActors(); + const actorConfigs = await readConfigFile(); const actorsChanged = getChangedActors({ filepathsChanged: changedFiles, actorConfigs, @@ -215,11 +215,11 @@ await yargs() }) .option('dry-run', { type: 'boolean', default: false }), async ({ actors, dryRun }) => { - const allActorConfigs = await getRepoActors(); + const allActorConfigs = await readConfigFile(); const actorConfigs = actors ? actors.split(',').map((name) => { const trimmed = name.trim(); - const config = allActorConfigs.find((c) => c.actorName === trimmed); + const config = allActorConfigs.find((c) => c.actorFullName === trimmed); if (!config) throw new Error(`Actor "${trimmed}" not found in repo`); return config; }) @@ -233,7 +233,7 @@ await yargs() '', (_) => _, async () => { - const actorConfigs = await getRepoActors(); + const actorConfigs = await readConfigFile(); await deleteOldBuilds(actorConfigs); }, ) diff --git a/bin/path-utils.ts b/bin/path-utils.ts new file mode 100644 index 0000000..a0bf961 --- /dev/null +++ b/bin/path-utils.ts @@ -0,0 +1,13 @@ +// filePath and scopePath are both repo-root-relative POSIX paths, already normalized (no trailing slashes). +// scopePath === '' means "matches everything" — the caller's own scope (a context path, an actor's folder, +// a sibling's folder, a docker context dir), not necessarily the repo root. +export const isPathWithinScope = (filePath: string, scopePath: string): boolean => + scopePath === '' || filePath === scopePath || filePath.startsWith(`${scopePath}/`); + +// Returns the single scopePaths entry filePath falls under, if any. +export const findContainingScope = (filePath: string, scopePaths: string[]): string | undefined => + scopePaths.find((scopePath) => isPathWithinScope(filePath, scopePath)); + +// Returns filePath relative to scopePath (assumes isPathWithinScope(filePath, scopePath) is already true). +export const hoistPath = (filePath: string, scopePath: string): string => + scopePath === '' ? filePath : filePath.slice(scopePath.length + 1); diff --git a/bin/test-report.ts b/bin/test-report.ts index 257f356..98e66c8 100644 --- a/bin/test-report.ts +++ b/bin/test-report.ts @@ -36,7 +36,7 @@ export const reportTestResults = async ({ } } - const failedAssertions: { message: string; runLink: string; actorName: string }[] = []; + const failedAssertions: { message: string; runLink: string; actorId: string }[] = []; console.error(); console.error(`PASSED: ${passed.length}, FAILED: ${failed.length}`); @@ -62,7 +62,7 @@ export const reportTestResults = async ({ ...failureMessages.map((message) => ({ message: message.split('\n')?.[0], runLink: meta.runLink, - actorName: meta.actorName, + actorId: meta.actorId, })), ); } @@ -89,10 +89,10 @@ export const reportTestResults = async ({ const jobLink = jobUrl ? ` Check <${jobUrl}|the job>.` : ''; let slackMessage = `\`${workflowName ?? '-'}\``; slackMessage += `: has ${failedAssertions.length} failed assertions. Failing test suites: ${failed.length}/${total}.${jobLink}`; - slackMessage += `\n\n${failedAssertions[0].message} --- <${failedAssertions[0].runLink}|${failedAssertions[0].actorName}>`; + slackMessage += `\n\n${failedAssertions[0].message} --- <${failedAssertions[0].runLink}|${failedAssertions[0].actorId}>`; const blocks = failedAssertions .slice(1) - .map(({ message, runLink, actorName }) => `• ${message} --- <${runLink}|${actorName}>`); + .map(({ message, runLink, actorId }) => `• ${message} --- <${runLink}|${actorId}>`); console.error('SLACK:', slackMessage); console.error('\tblocks:', blocks.join('\n\t\t')); @@ -122,7 +122,7 @@ interface JsonAssertionResult { meta: { runId: string; runLink: string; - actorName: string; + actorId: string; }; duration?: Milliseconds | null; failureMessages: string[] | null; diff --git a/bin/types.ts b/bin/types.ts index 9f60717..7baa85c 100644 --- a/bin/types.ts +++ b/bin/types.ts @@ -85,16 +85,28 @@ export interface GithubCommit { modified: string[]; } +export interface ActorConfigFileEntry { + folder: string; + actorFullName: string; + tokenEnvVar: string; + overrideActorContext?: string[]; +} + +export interface ActorConfigFile { + actors: ActorConfigFileEntry[]; +} + export interface BuildData { buildId: string; - actorId: string; - actorName: string; - // folder: string | undefined; + actorRawId: string; + actorFullName: string; buildNumber: string; } export interface ActorConfig { - actorName: string; + actorFullName: string; folder: string; - isStandalone: boolean; + tokenEnvVar: string; + dockerContextDir: string; + contextPaths: string[]; } diff --git a/bin/utils.ts b/bin/utils.ts index 841b6aa..ea400ad 100644 --- a/bin/utils.ts +++ b/bin/utils.ts @@ -1,14 +1,13 @@ import { spawnSync } from 'node:child_process'; -import fsSync from 'node:fs'; import fs from 'node:fs/promises'; -import os from 'node:os'; import path from 'node:path'; import type { ActorVersionSourceFile } from 'apify-client'; import { SOURCE_FILE_FORMATS } from '@apify/consts'; -import type { ActorConfig } from './types.js'; +import { isPathWithinScope } from './path-utils.js'; +import type { ActorConfig, ActorConfigFile } from './types.js'; // Returns true when `childPath` is not inside `parentPath`. // Used to detect monorepo actors whose dockerContextDir escapes the actor directory. @@ -59,65 +58,6 @@ export const getGitignoredPaths = (relativePaths: string[]): Set => { return new Set(result.stdout.toString().split('\n').filter(Boolean)); }; -// Docker normalizes each pattern before matching, so a leading "./" (as in the common "./node_modules" -// style) is a no-op for Docker. git's ignore engine has no such normalization — it treats "./" as -// literal pattern text that can never match a real path, so a .dockerignore written in that style -// would otherwise silently match nothing under git. Strip it here (after any negation prefix) so -// git sees the same effective pattern Docker would. -const normalizeDockerignorePattern = (line: string): string => line.replace(/^(!?)(?:\.\/)+/, '$1'); - -/** - * Given paths relative to `rootDir`, returns the subset that a `.dockerignore` file sitting at - * `rootDir` would exclude — mirroring getGitignoredPaths, but for Docker's own ignore file. `git - * check-ignore` has no built-in notion of .dockerignore, but pointing `core.excludesFile` at a - * normalized copy of it for a single invocation makes git apply its patterns exactly like a - * .gitignore, without re-implementing gitignore-style pattern matching ourselves. A missing - * .dockerignore is a no-op (empty set), so there's no need to check for its existence first. - * - * Crucially this passes `--no-index`: by default git never reports an already-tracked file as - * ignored (that's how real .gitignore semantics work — tracked files aren't affected by ignore - * rules), but Docker excludes a matching path from the build context unconditionally, regardless - * of git tracking. `--no-index` makes git apply the patterns uniformly, matching Docker's behavior. - */ -export const getDockerignoredPaths = (rootDir: string, relativePaths: string[]): Set => { - if (relativePaths.length === 0) return new Set(); - - let dockerignoreContent: string; - try { - dockerignoreContent = fsSync.readFileSync(path.join(rootDir, '.dockerignore'), 'utf8'); - } catch { - return new Set(); - } - - const normalizedContent = dockerignoreContent.split('\n').map(normalizeDockerignorePattern).join('\n'); - - const tempDir = fsSync.mkdtempSync(path.join(os.tmpdir(), 'apify-dockerignore-')); - try { - const normalizedPath = path.join(tempDir, '.dockerignore'); - fsSync.writeFileSync(normalizedPath, normalizedContent); - - const result = spawnSync( - 'git', - ['-c', `core.excludesFile=${normalizedPath}`, 'check-ignore', '--no-index', '--stdin'], - { - cwd: rootDir, - input: relativePaths.join('\n'), - maxBuffer: 100 * 1024 * 1024, - }, - ); - - // Exit code 1 means none of the given paths are ignored - not an error. Anything else - // (e.g. 128 for "not a git repository") is a real failure. - if (result.status !== 0 && result.status !== 1) { - throw new Error(`[Command failed]: git check-ignore (dockerignore)\n${result.stderr.toString()}`); - } - - return new Set(result.stdout.toString().split('\n').filter(Boolean)); - } finally { - fsSync.rmSync(tempDir, { recursive: true, force: true }); - } -}; - const isBinary = (buffer: Buffer): boolean => buffer.includes(0); export const toActorVersionSourceFile = async (absPath: string, rootDir: string): Promise => { @@ -154,50 +94,137 @@ export const getEnvVar = (varName: string, defaultValue?: string): string => { return value; }; -/** - * Reads and parses all directories in `actors` directory - * This works locally if checkoutRepoLocally is called first - */ -export const getRepoActors = async (): Promise => { - let actorDirs: string[]; +export const CONFIG_FILE_NAME = 'apify-test-tools.config.json'; + +// Strips a trailing slash so config-declared paths ("actors/shopify/" vs "actors/shopify") compare equal. +const stripTrailingSlash = (pathValue: string): string => pathValue.replace(/\/+$/, ''); + +const findOverlappingContextPaths = (contextPaths: string[]): [string, string] | undefined => { + for (let i = 0; i < contextPaths.length; i++) { + for (let j = i + 1; j < contextPaths.length; j++) { + if ( + isPathWithinScope(contextPaths[i], contextPaths[j]) || + isPathWithinScope(contextPaths[j], contextPaths[i]) + ) { + return [contextPaths[i], contextPaths[j]]; + } + } + } + return undefined; +}; + +export const readConfigFile = async (): Promise => { + let raw: string; try { - actorDirs = (await fs.readdir(`./actors`)).map((dir) => `actors/${dir}`); + raw = await fs.readFile(CONFIG_FILE_NAME, 'utf-8'); } catch { - console.warn(`No /actors directory found in repo`); - actorDirs = []; + throw new Error( + `Config file "${CONFIG_FILE_NAME}" not found in the current directory. ` + + `Please create one with the required actor entries.`, + ); } - let standaloneActorDirs: string[]; + + let config: ActorConfigFile; try { - standaloneActorDirs = (await fs.readdir(`./standalone-actors`)).map((dir) => `standalone-actors/${dir}`); + config = JSON.parse(raw); } catch { - console.warn(`No /standalone-actors directory found in repo`); - standaloneActorDirs = []; + throw new Error(`Config file "${CONFIG_FILE_NAME}" contains invalid JSON.`); + } + + if (!Array.isArray(config.actors)) { + throw new Error(`Config file "${CONFIG_FILE_NAME}" must have an "actors" array at the top level.`); } + + const seenFolders = new Set(); const actorConfigs: ActorConfig[] = []; - for (const actorDir of [...actorDirs, ...standaloneActorDirs]) { - const match = actorDir.match(/^([^/]+)\/(.+)_([^_]+)$/); - if (!match) { - throw new Error(`Invalid actor directory name. Got "${actorDir}", expected "actor.owner-name_actor-name"`); + + for (const [index, entry] of config.actors.entries()) { + if (typeof entry.folder !== 'string') { + throw new Error( + `Invalid "folder" for actor entry at index ${index} in "${CONFIG_FILE_NAME}". ` + + `Must be a string (use "." for a single-actor repo).`, + ); } - const [, folderType, owner, actorName] = match; + + const folder = entry.folder === '.' ? '' : stripTrailingSlash(entry.folder); + + if (seenFolders.has(folder)) { + throw new Error( + `Duplicate folder "${entry.folder}" in "${CONFIG_FILE_NAME}". Each actor must have a unique folder.`, + ); + } + seenFolders.add(folder); + + const nameParts = entry.actorFullName?.split('/'); + if (!nameParts || nameParts.length !== 2 || !nameParts[0] || !nameParts[1]) { + throw new Error( + `Invalid "actorFullName" for folder "${entry.folder}" in "${CONFIG_FILE_NAME}". ` + + `Must be in "owner/name" format (e.g. "apify/web-scraper").`, + ); + } + + if (entry.overrideActorContext !== undefined) { + if ( + !Array.isArray(entry.overrideActorContext) || + !entry.overrideActorContext.every((p) => typeof p === 'string') + ) { + throw new Error( + `Invalid "overrideActorContext" for folder "${entry.folder}" in "${CONFIG_FILE_NAME}". ` + + `Must be an array of strings.`, + ); + } + } + + const actorJsonPath = folder ? `${folder}/.actor/actor.json` : '.actor/actor.json'; + + let actorJson: { dockerContextDir?: string }; + try { + actorJson = JSON.parse(await fs.readFile(actorJsonPath, 'utf-8')); + } catch { + throw new Error( + `Cannot read "${actorJsonPath}". Every actor entry in "${CONFIG_FILE_NAME}" ` + + `must have a corresponding .actor/actor.json file.`, + ); + } + + const actorDotDir = folder ? `${folder}/.actor` : '.actor'; + const rawDockerContextDir = actorJson.dockerContextDir ?? '..'; + const resolved = path.resolve(process.cwd(), actorDotDir, rawDockerContextDir); + const dockerContextDir = path.relative(process.cwd(), resolved); + + if (dockerContextDir.startsWith('..')) { + throw new Error( + `"dockerContextDir" for folder "${entry.folder}" resolves outside the repository root. ` + + `Resolved path: "${dockerContextDir}".`, + ); + } + + const normalizedDockerContextDir = dockerContextDir === '.' ? '' : dockerContextDir; + const contextPaths = (entry.overrideActorContext ?? [normalizedDockerContextDir]).map(stripTrailingSlash); + + // The actor's own folder is always part of its context. When an explicit "overrideActorContext" + // doesn't already cover it, add it automatically instead of failing the workflow. + if (!contextPaths.some((contextPath) => isPathWithinScope(folder, contextPath))) { + contextPaths.push(folder); + } + + const overlap = findOverlappingContextPaths(contextPaths); + if (overlap) { + throw new Error( + `Invalid context paths for folder "${entry.folder}" in "${CONFIG_FILE_NAME}": ` + + `"${overlap[0]}" and "${overlap[1]}" overlap. Context paths must not be prefixes of one another.`, + ); + } + actorConfigs.push({ - actorName: `${owner}/${actorName}`, - folder: actorDir, - isStandalone: folderType === 'standalone-actors', + actorFullName: entry.actorFullName, + folder, + tokenEnvVar: entry.tokenEnvVar, + dockerContextDir: normalizedDockerContextDir, + contextPaths, }); } - console.error( - `Actors in repo: ${actorConfigs - .filter(({ isStandalone }) => !isStandalone) - .map(({ actorName }) => actorName) - .join(', ')}`, - ); - console.error( - `Standalone actors in repo: ${actorConfigs - .filter(({ isStandalone }) => !!isStandalone) - .map(({ actorName }) => actorName) - .join(', ')}`, - ); + return actorConfigs; }; diff --git a/lib/lib.ts b/lib/lib.ts index 96bdad3..5c83f31 100644 --- a/lib/lib.ts +++ b/lib/lib.ts @@ -24,8 +24,8 @@ try { } const config = actorBuilds.reduce>((map, cfg) => { - map.set(cfg.actorName, cfg); - map.set(cfg.actorId, cfg); + map.set(cfg.actorFullName, cfg); + map.set(cfg.actorRawId, cfg); return map; }, new Map()); @@ -51,8 +51,11 @@ const DEFAULT_TEST_ACTOR_OPTIONS: ActorTestOptions = { timeout: DEFAULT_TEST_RUN_DURATION_MS, }; +/** + * @param actorId - The actor's raw platform ID or its full name (`owner/name`, e.g. `"apify/web-scraper"`). + */ export const testActor = ( - actorName: string, + actorId: string, testName: string, fn: TestFunction<{ run: ReturnType> }>, testOptions?: ActorTestOptions, @@ -61,14 +64,13 @@ export const testActor = ( ...DEFAULT_TEST_ACTOR_OPTIONS, ...testOptions, }; - - const name = `${actorName}: ${testName}`; - const shouldRun = !!RUN_ALL_PLATFORM_TESTS || config.has(actorName); + const name = `${actorId}: ${testName}`; + const shouldRun = !!RUN_ALL_PLATFORM_TESTS || config.has(actorId); vitestTest.runIf(shouldRun)(name, options, async (context: TYPE) => { const { expect, ...rest } = context; await fn({ expect: extendExpect(expect), - run: createStartRunFn(actorName, context), + run: createStartRunFn(actorId, context), ...rest, }); }); @@ -80,9 +82,12 @@ export const testActor = ( * * Using task is just current shortcoming of standby feature but ideally we would use Actor directly */ +/** + * @param actorId - The actor's raw platform ID or its full name (`owner/name`, e.g. `"apify/web-scraper"`). + */ // eslint-disable-next-line @typescript-eslint/no-explicit-any export const testStandbyActor = ( - actorName: string, + actorId: string, testName: string, fn: TestFunction<{ callStandby: ReturnType> }>, testOptions?: ActorTestOptions, @@ -91,11 +96,11 @@ export const testStandbyActor = ( ...DEFAULT_TEST_ACTOR_OPTIONS, ...testOptions, }; - const name = `${actorName}: ${testName}`; - const shouldRun = !!RUN_ALL_PLATFORM_TESTS || config.has(actorName); + const name = `${actorId}: ${testName}`; + const shouldRun = !!RUN_ALL_PLATFORM_TESTS || config.has(actorId); vitestTest.runIf(shouldRun)(name, options, async (context: T) => { - const standbyTask = await createStandbyTask(actorName, config.get(actorName)?.buildNumber); + const standbyTask = await createStandbyTask(actorId, config.get(actorId)?.buildNumber); const { expect, ...rest } = context; // NOTE: we wrap `fn` in try/finally so cleanup (deleting the task) always runs afterwards @@ -194,20 +199,20 @@ interface StandbyTask { * * @throws if actor doesn't exist or it doesn't support standby mode. */ -const createStandbyTask = async (actorNameOrId: string, buildNumber?: string): Promise => { - const actor = apifyClient.actor(actorNameOrId); +const createStandbyTask = async (actorId: string, buildNumber?: string): Promise => { + const actor = apifyClient.actor(actorId); const actorInfo = (await actor.get()) as Actor & { standbyUrl?: string }; if (!actorInfo) { - throw new Error(`Actor "${actorNameOrId}" not found`); + throw new Error(`Actor "${actorId}" not found`); } if (!actorInfo.standbyUrl) { - throw new Error(`Actor "${actorNameOrId}" doesn't support standby mode`); + throw new Error(`Actor "${actorId}" doesn't support standby mode`); } if (!actorInfo.actorStandby) { - throw new Error(`Actor "${actorNameOrId} doesn't contain actorStandby options`); + throw new Error(`Actor "${actorId} doesn't contain actorStandby options`); } const { isEnabled, ...defaultActorStandby } = actorInfo.actorStandby; delete defaultActorStandby.disableStandbyFieldsOverride; @@ -226,9 +231,9 @@ const createStandbyTask = async (actorNameOrId: string, buildNumber?: string): P const randomValue = Math.random().toString(10).slice(2).padEnd(randomValueLength, '0'); const name = `test-${randomValue.slice(0, randomValueLength)}`; const newTask = (await apifyClient.tasks().create({ - actId: actorNameOrId, + actId: actorId, actorStandby: actorStandbyOptions, - description: `Task for testing standby version ${build} of actor "${actorNameOrId}"`, + description: `Task for testing standby version ${build} of actor "${actorId}"`, title, name, })) as Task & { standbyUrl?: string }; @@ -248,9 +253,9 @@ const createStandbyTask = async (actorNameOrId: string, buildNumber?: string): P } }; -const createStartRunFn = (actorNameOrId: string, testContext: TestContext) => { +const createStartRunFn = (actorId: string, testContext: TestContext) => { const { annotate, task } = testContext; - const actorConfig = config.get(actorNameOrId); + const actorConfig = config.get(actorId); const build = actorConfig?.buildNumber; const buildId = actorConfig?.buildId; return async (runOptions: RunOptions) => { @@ -264,10 +269,10 @@ const createStartRunFn = (actorNameOrId: string, testContext: TestContext) => return new RunTestResult(apifyClient, run); } - const actor = apifyClient.actor(actorNameOrId); + const actor = apifyClient.actor(actorId); const actorInput = { - ...(prefilledInput && (await getActorPrefilledInput(apifyClient, actorNameOrId, buildId))), + ...(prefilledInput && (await getActorPrefilledInput(apifyClient, actorId, buildId))), ...input, }; const run = await actor.call(actorInput, { build, log: null, ...options }); @@ -278,7 +283,7 @@ const createStartRunFn = (actorNameOrId: string, testContext: TestContext) => task.meta = { runId: run.id, runLink, - actorName: actorNameOrId, + actorId: actorConfig?.actorFullName ?? actorId, }; // waiting for dataset and statistics to sync, the Apify platform is only eventually consistent. diff --git a/lib/types.ts b/lib/types.ts index 6b301f3..d231bd5 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -3,9 +3,9 @@ import type { Assertion, TestOptions } from 'vitest'; export type ActorBuild = { buildId: string; - actorId: string; + actorRawId: string; buildNumber: string; - actorName: string; + actorFullName: string; }; export type RunOptions = { diff --git a/lib/utils.ts b/lib/utils.ts index 0034cb2..014299c 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -6,11 +6,11 @@ import type { ApifyClient } from 'apify-client'; */ export const getActorPrefilledInput = async ( apifyClient: ApifyClient, - actorNameOrId: string, + actorId: string, buildId: string | undefined, ) => { if (!buildId) { - const actorInfo = await apifyClient.actor(actorNameOrId).get(); + const actorInfo = await apifyClient.actor(actorId).get(); const defaultBuildTag = actorInfo?.defaultRunOptions.build; @@ -19,7 +19,7 @@ export const getActorPrefilledInput = async ( buildId = taggedBuild?.buildId; if (!buildId) { - console.error(`Coudn't find default build for actor ${actorNameOrId}. Prefilled values will not be used.`); + console.error(`Coudn't find default build for actor ${actorId}. Prefilled values will not be used.`); return {}; } } @@ -34,7 +34,7 @@ export const getActorPrefilledInput = async ( if (!inputSchema) { console.error( - `Coudn't find input schema definition for actor ${actorNameOrId}, build ${buildId}.`, + `Coudn't find input schema definition for actor ${actorId}, build ${buildId}.`, 'Prefilled values will not be used', ); return {}; diff --git a/package-lock.json b/package-lock.json index 2d53c3f..06df51d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "@apify/consts": "^2.43.0", "@slack/web-api": "^7.9.2", "apify-client": "^2.22.2", + "ignore": "^7.0.5", "yargs": "^18.0.0" }, "bin": { @@ -765,6 +766,16 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/@eslint/eslintrc/node_modules/ignore": { + "version": "5.3.2", + "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.3.2.tgz", + "integrity": "sha512-hsBTNUqQTDwkWtcdYI2i06Y/nUBEsNEDJKjWdigLvegy8kDuJAS8uRlpkkcQpyEXL0Z/pjDy5HBmMjRCJ2gq+g==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 4" + } + }, "node_modules/@eslint/eslintrc/node_modules/minimatch": { "version": "3.1.5", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", @@ -3505,6 +3516,16 @@ "url": "https://opencollective.com/eslint" } }, + "node_modules/eslint/node_modules/ignore": { + "version": "5.3.2", + "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.3.2.tgz", + "integrity": "sha512-hsBTNUqQTDwkWtcdYI2i06Y/nUBEsNEDJKjWdigLvegy8kDuJAS8uRlpkkcQpyEXL0Z/pjDy5HBmMjRCJ2gq+g==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 4" + } + }, "node_modules/eslint/node_modules/minimatch": { "version": "3.1.5", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", @@ -4252,10 +4273,9 @@ } }, "node_modules/ignore": { - "version": "5.3.2", - "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.3.2.tgz", - "integrity": "sha512-hsBTNUqQTDwkWtcdYI2i06Y/nUBEsNEDJKjWdigLvegy8kDuJAS8uRlpkkcQpyEXL0Z/pjDy5HBmMjRCJ2gq+g==", - "dev": true, + "version": "7.0.5", + "resolved": "https://registry.npmjs.org/ignore/-/ignore-7.0.5.tgz", + "integrity": "sha512-Hs59xBNfUIunMFgWAbGX5cq6893IbWg4KnrjbYwX3tx0ztorVgTDA6B2sxf8ejHJ4wz8BqGUMYlnzNBer5NvGg==", "license": "MIT", "engines": { "node": ">= 4" @@ -7021,16 +7041,6 @@ "url": "https://opencollective.com/eslint" } }, - "node_modules/typescript-eslint/node_modules/ignore": { - "version": "7.0.5", - "resolved": "https://registry.npmjs.org/ignore/-/ignore-7.0.5.tgz", - "integrity": "sha512-Hs59xBNfUIunMFgWAbGX5cq6893IbWg4KnrjbYwX3tx0ztorVgTDA6B2sxf8ejHJ4wz8BqGUMYlnzNBer5NvGg==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 4" - } - }, "node_modules/typescript-eslint/node_modules/minimatch": { "version": "10.2.4", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", diff --git a/package.json b/package.json index fa27c3a..9f7e58d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "apify-test-tools", - "version": "0.8.7", + "version": "0.9.0", "type": "module", "description": "TBD", "repository": { @@ -19,6 +19,7 @@ "@apify/consts": "^2.43.0", "@slack/web-api": "^7.9.2", "apify-client": "^2.22.2", + "ignore": "^7.0.5", "yargs": "^18.0.0" }, "devDependencies": { @@ -29,14 +30,14 @@ "eslint": "^9.29.0", "eslint-config-prettier": "^10.1.5", "globals": "^17.0.0", + "husky": "^9.0.11", + "knip": "^5.65.0", + "lint-staged": "^15.2.2", "prettier": "^3.5.3", "tsx": "^4.20.3", "typescript": "^5.9.3", "typescript-eslint": "^8.34.1", - "vitest": "^3.2.4", - "knip": "^5.65.0", - "husky": "^9.0.11", - "lint-staged": "^15.2.2" + "vitest": "^3.2.4" }, "peerDependencies": { "vitest": ">=3.2.4" diff --git a/test/unit/bin/build-from-local.test.ts b/test/unit/bin/build-from-local.test.ts index 31aa0ac..ade282d 100644 --- a/test/unit/bin/build-from-local.test.ts +++ b/test/unit/bin/build-from-local.test.ts @@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { collectNonIgnoredFiles, + collectSourceFiles, flattenMonorepoContext, rewriteActorJsonPaths, // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -62,86 +63,76 @@ describe('build-from-local helpers', () => { }); describe('flattenMonorepoContext', () => { - it('filters the .actor/ overlay through the same secret-pattern check as the rest of the context', async () => { + it("overlays the actor's own .actor/ files at the flattened root", async () => { + // Simulates flattening from a monorepo with an actors//.actor structure. const repoRoot = await mkTempDir('apify-test-tools-repo-'); tempDirs.push(repoRoot); - initGitRepo(repoRoot); const absActorDir = path.join(repoRoot, 'actors', 'owner_actor'); - await fs.mkdir(path.join(absActorDir, '.actor'), { recursive: true }); - await fs.writeFile( - path.join(absActorDir, '.actor', 'actor.json'), - JSON.stringify({ actorSpecification: 1, name: 'actor' }), - ); - // A stray secret file inside .actor/ must never survive into the build. - await fs.writeFile(path.join(absActorDir, '.actor', '.env'), 'SECRET=leaked'); - await fs.writeFile(path.join(absActorDir, '.actor', 'INPUT_SCHEMA.json'), '{}'); + const actorJsonPath = path.join(absActorDir, '.actor', 'actor.json'); + const inputSchemaPath = path.join(absActorDir, '.actor', 'INPUT_SCHEMA.json'); + const mainSrcPath = path.join(absActorDir, 'src', 'main.ts'); + const packageJsonPath = path.join(repoRoot, 'package.json'); - const keptContextFile = path.join(repoRoot, 'package.json'); - await fs.writeFile(keptContextFile, '{}'); + await fs.mkdir(path.join(absActorDir, '.actor'), { recursive: true }); + await fs.mkdir(path.join(absActorDir, 'src'), { recursive: true }); + await fs.writeFile(actorJsonPath, JSON.stringify({ actorSpecification: 1, name: 'actor' })); + await fs.writeFile(inputSchemaPath, '{}'); + await fs.writeFile(mainSrcPath, 'console.log(1)'); + await fs.writeFile(packageJsonPath, '{}'); - // Nothing is gitignored here — isolates the assertion to the secret-pattern filter. - vi.spyOn(Utils, 'getGitignoredPaths').mockReturnValue(new Set()); + const keptContextFiles = [packageJsonPath, mainSrcPath, actorJsonPath, inputSchemaPath]; - const actorJson = JSON.parse( - await fs.readFile(path.join(absActorDir, '.actor', 'actor.json'), 'utf8'), - ) as Record; + const actorJson = JSON.parse(await fs.readFile(actorJsonPath, 'utf8')) as Record; const { tempDir: flattenedDir, filePaths } = await flattenMonorepoContext( 'test/actor', absActorDir, repoRoot, actorJson, - [keptContextFile], - repoRoot, + keptContextFiles, ); tempDirs.push(flattenedDir); - await expect(fs.access(path.join(flattenedDir, '.actor', '.env'))).rejects.toThrow(); - await expect(fs.access(path.join(flattenedDir, '.actor', 'INPUT_SCHEMA.json'))).resolves.toBeUndefined(); + const flattenedMainSrcPath = path.join(flattenedDir, 'actors', 'owner_actor', 'src', 'main.ts'); await expect(fs.access(path.join(flattenedDir, '.actor', 'actor.json'))).resolves.toBeUndefined(); + await expect(fs.access(path.join(flattenedDir, '.actor', 'INPUT_SCHEMA.json'))).resolves.toBeUndefined(); await expect(fs.access(path.join(flattenedDir, 'package.json'))).resolves.toBeUndefined(); - - // The returned filePaths must match what actually landed on disk — no .env, everything else present. - expect(new Set(filePaths)).toStrictEqual( - new Set([ - path.join(flattenedDir, 'package.json'), - path.join(flattenedDir, '.actor', 'INPUT_SCHEMA.json'), - path.join(flattenedDir, '.actor', 'actor.json'), - ]), - ); + await expect(fs.access(flattenedMainSrcPath)).resolves.toBeUndefined(); + expect(filePaths).toContain(path.join(flattenedDir, '.actor', 'actor.json')); + expect(filePaths).toContain(path.join(flattenedDir, '.actor', 'INPUT_SCHEMA.json')); + expect(filePaths).toContain(flattenedMainSrcPath); }); it("keeps another actor's .actor/ directory intact at its original nested path", async () => { + // Simulates flattening from a monorepo with an actors//.actor structure — two + // sibling actors share the same context, but only one is being flattened here. const repoRoot = await mkTempDir('apify-test-tools-repo-'); tempDirs.push(repoRoot); - initGitRepo(repoRoot); const absActorDir = path.join(repoRoot, 'actors', 'owner_actor'); - await fs.mkdir(path.join(absActorDir, '.actor'), { recursive: true }); - await fs.writeFile( - path.join(absActorDir, '.actor', 'actor.json'), - JSON.stringify({ actorSpecification: 1, name: 'actor' }), - ); - + const actorJsonPath = path.join(absActorDir, '.actor', 'actor.json'); const otherActorDir = path.join(repoRoot, 'actors', 'owner_other-actor'); - await fs.mkdir(path.join(otherActorDir, '.actor'), { recursive: true }); const otherActorSchemaFile = path.join(otherActorDir, '.actor', 'input_schema.json'); + + await fs.mkdir(path.join(absActorDir, '.actor'), { recursive: true }); + await fs.writeFile(actorJsonPath, JSON.stringify({ actorSpecification: 1, name: 'actor' })); + await fs.mkdir(path.join(otherActorDir, '.actor'), { recursive: true }); await fs.writeFile(otherActorSchemaFile, JSON.stringify({ schema: 'other' })); - vi.spyOn(Utils, 'getGitignoredPaths').mockReturnValue(new Set()); + // Stands in for collectNonIgnoredFiles's already-filtered output: the current actor's own + // actor.json (always kept by collectNonIgnoredFiles's .actor/ bypass in the real flow — + // see the collectSourceFiles test below) plus the other actor's file. + const keptContextFiles = [actorJsonPath, otherActorSchemaFile]; - const actorJson = JSON.parse( - await fs.readFile(path.join(absActorDir, '.actor', 'actor.json'), 'utf8'), - ) as Record; + const actorJson = JSON.parse(await fs.readFile(actorJsonPath, 'utf8')) as Record; const { tempDir: flattenedDir, filePaths } = await flattenMonorepoContext( 'test/actor', absActorDir, repoRoot, actorJson, - [otherActorSchemaFile], - repoRoot, + keptContextFiles, ); tempDirs.push(flattenedDir); @@ -151,6 +142,33 @@ describe('build-from-local helpers', () => { }); }); + describe('collectSourceFiles', () => { + it("always collects the actor's own .actor/actor.json for a monorepo actor, since flattenMonorepoContext depends on it", async () => { + const repoRoot = await mkTempDir('apify-test-tools-collect-source-'); + tempDirs.push(repoRoot); + initGitRepo(repoRoot); + + const originalCwd = process.cwd(); + // We simulate the working directory being the repo root, since collectSourceFiles uses relative paths to the repo root. + Utils.setCwd({ workspace: repoRoot }); + try { + const cwd = process.cwd(); + const actorDir = path.join(cwd, 'actors', 'owner_actor'); + await fs.mkdir(path.join(actorDir, '.actor'), { recursive: true }); + await fs.writeFile( + path.join(actorDir, '.actor', 'actor.json'), + JSON.stringify({ actorSpecification: 1, name: 'actor', dockerContextDir: '../../..' }), + ); + await fs.writeFile(path.join(cwd, 'package.json'), '{}'); + + const sourceFiles = await collectSourceFiles('owner/actor', actorDir); + expect(sourceFiles.map((file) => file.name)).toContain('.actor/actor.json'); + } finally { + process.chdir(originalCwd); + } + }); + }); + describe('rewriteActorJsonPaths', () => { it('rewrites path fields that escape .actor/ relative to the new flattened location', async () => { const repoRoot = await mkTempDir('apify-test-tools-rewrite-'); @@ -182,83 +200,6 @@ describe('build-from-local helpers', () => { }); }); -describe('getDockerignoredPaths', () => { - const tempDirs: string[] = []; - - afterEach(async () => { - vi.mocked(spawnSync).mockClear(); - await Promise.all(tempDirs.splice(0).map(async (dir) => fs.rm(dir, { recursive: true, force: true }))); - }); - - it('returns an empty set without calling git when given no paths', () => { - const result = Utils.getDockerignoredPaths('/does/not/matter', []); - - expect(result).toStrictEqual(new Set()); - expect(spawnSync).not.toHaveBeenCalled(); - }); - - it('returns an empty set when rootDir has no .dockerignore', async () => { - const rootDir = await mkTempDir('apify-test-tools-dockerignore-'); - tempDirs.push(rootDir); - initGitRepo(rootDir); - - expect(Utils.getDockerignoredPaths(rootDir, ['main.js'])).toStrictEqual(new Set()); - }); - - it('returns the paths that match .dockerignore patterns rooted at rootDir', async () => { - const rootDir = await mkTempDir('apify-test-tools-dockerignore-'); - tempDirs.push(rootDir); - initGitRepo(rootDir); - - await fs.writeFile(path.join(rootDir, '.dockerignore'), 'test/\n*.log\n'); - - const result = Utils.getDockerignoredPaths(rootDir, ['main.js', 'test/fixture.json', 'debug.log']); - - expect(result).toStrictEqual(new Set(['test/fixture.json', 'debug.log'])); - }); - - it('matches "./"-prefixed patterns, which Docker treats as a no-op but git treats as literal text', async () => { - const rootDir = await mkTempDir('apify-test-tools-dockerignore-'); - tempDirs.push(rootDir); - initGitRepo(rootDir); - - // The common real-world .dockerignore style: every pattern prefixed with "./", - // including a negation re-including one of the otherwise-matched files. - await fs.writeFile(path.join(rootDir, '.dockerignore'), './node_modules\n./*.log\n!./keep.log\n'); - - const result = Utils.getDockerignoredPaths(rootDir, [ - 'node_modules/foo.js', - 'debug.log', - 'keep.log', - 'main.js', - ]); - - expect(result).toStrictEqual(new Set(['node_modules/foo.js', 'debug.log'])); - }); - - it('matches already-tracked files too, since Docker excludes them regardless of git tracking', async () => { - const rootDir = await mkTempDir('apify-test-tools-dockerignore-'); - tempDirs.push(rootDir); - initGitRepo(rootDir); - spawnSync('git', ['config', 'user.email', 'test@example.com'], { cwd: rootDir }); - spawnSync('git', ['config', 'user.name', 'Test'], { cwd: rootDir }); - - // Husky hooks are committed to the repo, so this path is tracked by git — by default - // `git check-ignore` never reports a tracked file as ignored, which would otherwise mask - // this exact pattern from matching, unlike a real Docker build which excludes it anyway. - await fs.mkdir(path.join(rootDir, '.husky')); - await fs.writeFile(path.join(rootDir, '.husky', 'pre-commit'), '#!/bin/sh\n'); - spawnSync('git', ['add', '.husky/pre-commit'], { cwd: rootDir }); - spawnSync('git', ['commit', '-q', '-m', 'add husky hook'], { cwd: rootDir }); - - await fs.writeFile(path.join(rootDir, '.dockerignore'), './.husky\n'); - - const result = Utils.getDockerignoredPaths(rootDir, ['.husky/pre-commit', 'main.js']); - - expect(result).toStrictEqual(new Set(['.husky/pre-commit'])); - }); -}); - describe('getGitignoredPaths', () => { beforeEach(() => { vi.mocked(spawnSync).mockReset(); diff --git a/test/unit/bin/diff-changes.test.ts b/test/unit/bin/diff-changes.test.ts index 727c366..cb09142 100644 --- a/test/unit/bin/diff-changes.test.ts +++ b/test/unit/bin/diff-changes.test.ts @@ -1,47 +1,35 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { getChangedActors, maybeParseActorFolder } from '../../../bin/diff-changes.js'; +import { getChangedActors } from '../../../bin/diff-changes.js'; import * as DiffJsonSchema from '../../../bin/diff-json-schema.js'; +import * as Dockerignore from '../../../bin/dockerignore.js'; import type { ActorConfig } from '../../../bin/types.js'; -const miniActor: ActorConfig = { actorName: 'foo/bar', folder: 'actors/foo_bar', isStandalone: false }; +const miniActor: ActorConfig = { + actorFullName: 'foo/bar', + folder: 'actors/foo_bar', + tokenEnvVar: 'APIFY_TOKEN_FOO', + dockerContextDir: '', + contextPaths: [''], +}; const standaloneActor: ActorConfig = { - actorName: 'standalone', + actorFullName: 'owner/standalone', folder: 'standalone-actors/standalone', - isStandalone: true, + tokenEnvVar: 'APIFY_TOKEN_OWNER', + dockerContextDir: 'standalone-actors/standalone', + contextPaths: ['standalone-actors/standalone'], }; const actorConfigs = [miniActor, standaloneActor]; +const amazonActor: ActorConfig = { + actorFullName: 'junglee/amazon-crawler', + folder: 'actors/junglee_Amazon-crawler', + tokenEnvVar: 'APIFY_TOKEN_JUNGLEE', + dockerContextDir: '', + contextPaths: ['actors/junglee_Amazon-crawler', 'code', 'shared'], +}; const commits = [{ sha: 'Commit1', author: '', date: '', message: '' }]; -describe('maybeParseActorFolder', () => { - it('returns actorName for actors/ path', () => { - expect(maybeParseActorFolder('actors/foo_bar/actor.json')).toEqual({ - isActorFolder: true, - actorName: 'foo/bar', - }); - }); - - it('returns actorName for standalone-actors/ path', () => { - expect(maybeParseActorFolder('standalone-actors/my_actor/main.ts')).toEqual({ - isActorFolder: true, - actorName: 'my/actor', - }); - }); - - it('returns false for top-level file', () => { - expect(maybeParseActorFolder('package.json')).toEqual({ isActorFolder: false }); - }); - - it('returns false for path with no file inside actor folder', () => { - expect(maybeParseActorFolder('actors/foo_bar')).toEqual({ isActorFolder: false }); - }); - - it('returns false for unrelated folder', () => { - expect(maybeParseActorFolder('src/utils.ts')).toEqual({ isActorFolder: false }); - }); -}); - describe('getChangedActors', () => { beforeEach(() => { vi.spyOn(DiffJsonSchema, 'isCosmeticOnlyJsonSchemaChange').mockReturnValue(false); @@ -92,7 +80,7 @@ describe('getChangedActors', () => { it('returns actor when isLatest and JSON file has only cosmetic changes', () => { vi.spyOn(DiffJsonSchema, 'isCosmeticOnlyJsonSchemaChange').mockReturnValue(true); const result = getChangedActors({ - filepathsChanged: ['actors/foo_bar/actor.json'], + filepathsChanged: ['actors/foo_bar/.actor/actor.json'], actorConfigs, commits, isLatest: true, @@ -103,7 +91,7 @@ describe('getChangedActors', () => { it('does not return actor when not isLatest and JSON file has only cosmetic changes', () => { vi.spyOn(DiffJsonSchema, 'isCosmeticOnlyJsonSchemaChange').mockReturnValue(true); const result = getChangedActors({ - filepathsChanged: ['actors/foo_bar/actor.json'], + filepathsChanged: ['actors/foo_bar/.actor/actor.json'], actorConfigs, commits, isLatest: false, @@ -114,14 +102,37 @@ describe('getChangedActors', () => { it('returns actor when JSON file has functional changes', () => { vi.spyOn(DiffJsonSchema, 'isCosmeticOnlyJsonSchemaChange').mockReturnValue(false); const result = getChangedActors({ - filepathsChanged: ['actors/foo_bar/actor.json'], + filepathsChanged: ['actors/foo_bar/.actor/actor.json'], + actorConfigs, + commits, + }); + expect(result).toEqual([miniActor]); + }); + + it('JSON file in actor folder but outside .actor/ is functional, not checked for cosmetic', () => { + vi.spyOn(DiffJsonSchema, 'isCosmeticOnlyJsonSchemaChange').mockReturnValue(true); + const result = getChangedActors({ + filepathsChanged: ['actors/foo_bar/package.json'], actorConfigs, commits, + isLatest: false, + }); + expect(result).toEqual([miniActor]); + expect(DiffJsonSchema.isCosmeticOnlyJsonSchemaChange).not.toHaveBeenCalled(); + }); + + it('JSON file under .actor/ inside actor folder is checked for cosmetic changes', () => { + vi.spyOn(DiffJsonSchema, 'isCosmeticOnlyJsonSchemaChange').mockReturnValue(true); + const result = getChangedActors({ + filepathsChanged: ['actors/foo_bar/.actor/input_schema.json'], + actorConfigs, + commits, + isLatest: true, }); expect(result).toEqual([miniActor]); }); - it('returns all non-standalone actors when a non-actor-folder functional file changes', () => { + it('does not trigger narrow-context actor when shared file changes', () => { const result = getChangedActors({ filepathsChanged: ['shared/utils.ts'], actorConfigs, @@ -131,18 +142,28 @@ describe('getChangedActors', () => { expect(result).not.toContainEqual(standaloneActor); }); - it('does not include standalone actor in all-actors expansion from changelog', () => { + it('root-level changelog outside any actor folder is cosmetic for every actor when isLatest', () => { const result = getChangedActors({ filepathsChanged: ['CHANGELOG.md'], actorConfigs, commits, isLatest: true, }); - expect(result).toContainEqual(miniActor); - expect(result).not.toContainEqual(standaloneActor); + expect(result).toEqual(expect.arrayContaining([miniActor, standaloneActor])); + expect(result).toHaveLength(2); }); - it('includes standalone actor when its own folder changes', () => { + it('root-level changelog is not cosmetic-triggered when not isLatest', () => { + const result = getChangedActors({ + filepathsChanged: ['CHANGELOG.md'], + actorConfigs, + commits, + isLatest: false, + }); + expect(result).toEqual([]); + }); + + it('triggers narrow-context actor when its own folder changes', () => { const result = getChangedActors({ filepathsChanged: ['standalone-actors/standalone/src/main.ts'], actorConfigs, @@ -161,7 +182,7 @@ describe('getChangedActors', () => { expect(result).toContainEqual(miniActor); }); - it('handles mixed changes: returns both mini and standalone actors', () => { + it('handles mixed changes: returns both broad and narrow-context actors', () => { const result = getChangedActors({ filepathsChanged: ['actors/foo_bar/src/main.ts', 'standalone-actors/standalone/Dockerfile'], actorConfigs, @@ -171,6 +192,47 @@ describe('getChangedActors', () => { expect(result).toContainEqual(standaloneActor); }); + it('matches folder where folder name differs from actor name', () => { + const ownerlessActor: ActorConfig = { + actorFullName: 'myteam/shopify-scraper', + folder: 'actors/shopify', + tokenEnvVar: 'APIFY_TOKEN_MYTEAM', + dockerContextDir: '', + contextPaths: [''], + }; + const result = getChangedActors({ + filepathsChanged: ['actors/shopify/src/main.ts'], + actorConfigs: [ownerlessActor], + commits, + }); + expect(result).toEqual([ownerlessActor]); + }); + + it('in single-actor repo, .actor/ changes trigger builds', () => { + const rootActor: ActorConfig = { + actorFullName: 'myteam/my-actor', + folder: '', + tokenEnvVar: 'BUILDER_APIFY_TOKEN', + dockerContextDir: '', + contextPaths: [''], + }; + const result = getChangedActors({ + filepathsChanged: ['.actor/actor.json'], + actorConfigs: [rootActor], + commits, + }); + expect(result).toEqual([rootActor]); + }); + + it('in multi-actor repo, .actor/ changes only trigger broad-context actors', () => { + const result = getChangedActors({ + filepathsChanged: ['.actor/actor.json'], + actorConfigs, + commits, + }); + expect(result).toEqual([miniActor]); + }); + it('file paths are matched case-insensitively', () => { const result = getChangedActors({ filepathsChanged: ['Actors/FOO_BAR/Main.ts'], @@ -179,4 +241,298 @@ describe('getChangedActors', () => { }); expect(result).toEqual([miniActor]); }); + + it('triggers actor with contextPaths override when file matches an override path', () => { + const overrideActor: ActorConfig = { + actorFullName: 'team/override-actor', + folder: 'actors/override', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: 'actors/override', + contextPaths: ['actors/override', 'packages'], + }; + const result = getChangedActors({ + filepathsChanged: ['packages/shared/utils.ts'], + actorConfigs: [overrideActor], + commits, + }); + expect(result).toEqual([overrideActor]); + }); + + it('does not trigger actor with contextPaths override when file is outside all override paths', () => { + const overrideActor: ActorConfig = { + actorFullName: 'team/override-actor', + folder: 'actors/override', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: 'actors/override', + contextPaths: ['actors/override', 'packages'], + }; + const result = getChangedActors({ + filepathsChanged: ['other-dir/file.ts'], + actorConfigs: [overrideActor], + commits, + }); + expect(result).toEqual([]); + }); + + it('broad-context actor skips files in sibling actor folders', () => { + const actorA: ActorConfig = { + actorFullName: 'team/actor-a', + folder: 'actors/a', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: '', + contextPaths: [''], + }; + const actorB: ActorConfig = { + actorFullName: 'team/actor-b', + folder: 'actors/b', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: '', + contextPaths: [''], + }; + const result = getChangedActors({ + filepathsChanged: ['actors/b/src/main.ts'], + actorConfigs: [actorA, actorB], + commits, + }); + expect(result).toEqual([actorB]); + }); + + it('root actor (folder="") is excluded from sibling actor folder files', () => { + const rootActor: ActorConfig = { + actorFullName: 'team/root', + folder: '', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: '', + contextPaths: [''], + }; + const childActor: ActorConfig = { + actorFullName: 'team/child', + folder: 'actors/child', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: 'actors/child', + contextPaths: ['actors/child'], + }; + const result = getChangedActors({ + filepathsChanged: ['actors/child/src/main.ts'], + actorConfigs: [rootActor, childActor], + commits, + }); + expect(result).not.toContainEqual(rootActor); + expect(result).toContainEqual(childActor); + }); + + it('root actor (folder="") sees files outside any actor folder', () => { + const rootActor: ActorConfig = { + actorFullName: 'team/root', + folder: '', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: '', + contextPaths: [''], + }; + const childActor: ActorConfig = { + actorFullName: 'team/child', + folder: 'actors/child', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: 'actors/child', + contextPaths: ['actors/child'], + }; + const result = getChangedActors({ + filepathsChanged: ['lib/shared-utils.ts'], + actorConfigs: [rootActor, childActor], + commits, + }); + expect(result).toContainEqual(rootActor); + expect(result).not.toContainEqual(childActor); + }); + + it('file matched by .dockerignore is treated as ignored', () => { + vi.spyOn(Dockerignore, 'loadDockerIgnore').mockReturnValue( + (filePath) => filePath === 'actors/foo_bar/node_modules/foo.js', + ); + const result = getChangedActors({ + filepathsChanged: ['actors/foo_bar/node_modules/foo.js'], + actorConfigs: [miniActor], + commits, + }); + expect(result).toEqual([]); + }); + + it('file not matched by .dockerignore is classified normally', () => { + vi.spyOn(Dockerignore, 'loadDockerIgnore').mockReturnValue((filePath) => filePath.includes('node_modules')); + const result = getChangedActors({ + filepathsChanged: ['actors/foo_bar/src/main.ts'], + actorConfigs: [miniActor], + commits, + }); + expect(result).toEqual([miniActor]); + }); + + it('JSON file in context but outside actor folder is functional (not checked for cosmetic)', () => { + vi.spyOn(DiffJsonSchema, 'isCosmeticOnlyJsonSchemaChange').mockReturnValue(true); + const result = getChangedActors({ + filepathsChanged: ['lib/config.json'], + actorConfigs: [miniActor], + commits, + }); + expect(result).toEqual([miniActor]); + }); + + it('README outside actor folder but inside context is ignored (not cosmetic)', () => { + const result = getChangedActors({ + filepathsChanged: ['docs/README.md'], + actorConfigs: [miniActor], + commits, + isLatest: true, + }); + expect(result).toEqual([]); + }); + + it('README inside actor folder is cosmetic', () => { + const result = getChangedActors({ + filepathsChanged: ['actors/foo_bar/README.md'], + actorConfigs: [miniActor], + commits, + isLatest: true, + }); + expect(result).toEqual([miniActor]); + }); + + it('hoists a standalone actor own top-level dev file relative to its context before checking the ignore list', () => { + const result = getChangedActors({ + filepathsChanged: ['standalone-actors/standalone/.eslintrc'], + actorConfigs, + commits, + }); + expect(result).toEqual([]); + }); + + it('does not special-case code/ and shared/ prefixes anymore — must be declared via overrideActorContext', () => { + const result = getChangedActors({ + filepathsChanged: ['code/.eslintrc'], + actorConfigs: [miniActor], + commits, + }); + expect(result).toEqual([miniActor]); + }); + + it('ignores code/.eslintrc when "code" is declared via overrideActorContext', () => { + const result = getChangedActors({ + filepathsChanged: ['code/.eslintrc'], + actorConfigs: [amazonActor], + commits, + }); + expect(result).toEqual([]); + }); + + it('shared/Dockerfile declared via overrideActorContext is functional', () => { + const result = getChangedActors({ + filepathsChanged: ['shared/Dockerfile'], + actorConfigs: [amazonActor], + commits, + }); + expect(result).toEqual([amazonActor]); + }); + + it('code/README.md declared via overrideActorContext is ignored (outside actor folder)', () => { + const result = getChangedActors({ + filepathsChanged: ['code/README.md'], + actorConfigs: [amazonActor], + commits, + isLatest: true, + }); + expect(result).toEqual([]); + }); +}); + +describe('getChangedActors logging', () => { + beforeEach(() => { + vi.spyOn(DiffJsonSchema, 'isCosmeticOnlyJsonSchemaChange').mockReturnValue(false); + vi.spyOn(console, 'error').mockImplementation(() => undefined); + }); + + it('logs a single "specific" group for a single actor with one functional file', () => { + getChangedActors({ + filepathsChanged: ['actors/foo_bar/src/main.ts'], + actorConfigs: [miniActor], + commits, + }); + + expect(console.error).toHaveBeenCalledWith( + '[DIFF]: Changes specific to actor foo/bar: actors/foo_bar/src/main.ts', + ); + expect(console.error).toHaveBeenCalledWith('[DIFF]: Actors to be built and tested: foo/bar'); + }); + + it('logs a single "shared" group when two actors are triggered by the exact same file', () => { + const actorA: ActorConfig = { + actorFullName: 'team/actor-a', + folder: 'actors/a', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: '', + contextPaths: ['', 'shared'], + }; + const actorB: ActorConfig = { + actorFullName: 'team/actor-b', + folder: 'actors/b', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: '', + contextPaths: ['', 'shared'], + }; + + getChangedActors({ + filepathsChanged: ['shared/shared.ts'], + actorConfigs: [actorA, actorB], + commits, + }); + + expect(console.error).toHaveBeenCalledWith( + '[DIFF]: Shared changes for actors team/actor-a, team/actor-b: shared/shared.ts', + ); + expect(console.error).not.toHaveBeenCalledWith(expect.stringContaining('Changes specific to actor')); + expect(console.error).toHaveBeenCalledWith('[DIFF]: Actors to be built and tested: team/actor-a, team/actor-b'); + }); + + it('logs shared and specific groups in descending-size order for partial overlap across actors', () => { + const actorA: ActorConfig = { + actorFullName: 'team/actor-a', + folder: 'actors/a', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: '', + contextPaths: [''], + }; + const actorB: ActorConfig = { + actorFullName: 'team/actor-b', + folder: 'actors/b', + tokenEnvVar: 'APIFY_TOKEN_TEAM', + dockerContextDir: '', + contextPaths: [''], + }; + + getChangedActors({ + filepathsChanged: ['shared.ts', 'actors/a/a-only.ts', 'actors/b/b-only.ts'], + actorConfigs: [actorA, actorB], + commits, + }); + + const errorCalls = (console.error as unknown as ReturnType).mock.calls.map((call) => call[0]); + + expect(errorCalls).toEqual([ + '[DIFF]: Shared changes for actors team/actor-a, team/actor-b: shared.ts', + '[DIFF]: Changes specific to actor team/actor-a: actors/a/a-only.ts', + '[DIFF]: Changes specific to actor team/actor-b: actors/b/b-only.ts', + '[DIFF]: Actors to be built and tested: team/actor-a, team/actor-b', + ]); + }); + + it('logs no group lines when zero actors changed', () => { + getChangedActors({ + filepathsChanged: ['.gitignore', 'README.md'], + actorConfigs, + commits, + }); + + expect(console.error).not.toHaveBeenCalledWith(expect.stringContaining('Shared changes')); + expect(console.error).not.toHaveBeenCalledWith(expect.stringContaining('Changes specific to')); + expect(console.error).toHaveBeenCalledWith('[DIFF]: No relevant files changed, skipping builds and tests'); + }); }); diff --git a/test/unit/bin/dockerignore.test.ts b/test/unit/bin/dockerignore.test.ts new file mode 100644 index 0000000..f1ff6a6 --- /dev/null +++ b/test/unit/bin/dockerignore.test.ts @@ -0,0 +1,123 @@ +import path from 'node:path'; + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { buildDockerIgnoreMatcher, loadDockerIgnore } from '../../../bin/dockerignore.js'; + +const { fsMock } = vi.hoisted(() => ({ + fsMock: { + readFileSync: vi.fn(), + }, +})); + +vi.mock('node:fs', () => ({ default: fsMock })); + +afterEach(() => vi.restoreAllMocks()); + +describe('loadDockerIgnore', () => { + it('returns no-op matcher when .dockerignore is absent', () => { + fsMock.readFileSync.mockImplementation(() => { + throw new Error('ENOENT'); + }); + const matcher = loadDockerIgnore('actors/my-actor'); + expect(matcher('actors/my-actor/src/main.ts')).toBe(false); + expect(matcher('actors/my-actor/node_modules/foo.js')).toBe(false); + }); + + it('matches files listed in .dockerignore', () => { + fsMock.readFileSync.mockReturnValue('node_modules\n*.log\n'); + const matcher = loadDockerIgnore(''); + expect(matcher('node_modules/foo/bar.js')).toBe(true); + expect(matcher('debug.log')).toBe(true); + }); + + it('does not match files not in .dockerignore', () => { + fsMock.readFileSync.mockReturnValue('node_modules\n'); + const matcher = loadDockerIgnore(''); + expect(matcher('src/main.ts')).toBe(false); + }); + + it.each(['dist', 'dist/'])('handles directory pattern "%s"', (pattern) => { + fsMock.readFileSync.mockReturnValue(`${pattern}\n`); + const matcher = loadDockerIgnore(''); + expect(matcher('dist/bundle.js')).toBe(true); + expect(matcher('src/dist-utils.ts')).toBe(false); + }); + + it('handles negation patterns', () => { + fsMock.readFileSync.mockReturnValue('*.log\n!important.log\n'); + const matcher = loadDockerIgnore(''); + expect(matcher('debug.log')).toBe(true); + expect(matcher('important.log')).toBe(false); + }); + + it('strips dockerContextDir prefix before matching', () => { + fsMock.readFileSync.mockReturnValue('node_modules\n'); + const matcher = loadDockerIgnore('actors/shopify'); + expect(matcher('actors/shopify/node_modules/foo.js')).toBe(true); + expect(matcher('actors/shopify/src/main.ts')).toBe(false); + }); + + it('returns false for files outside dockerContextDir', () => { + fsMock.readFileSync.mockReturnValue('*\n'); + const matcher = loadDockerIgnore('actors/shopify'); + expect(matcher('other-actor/src/main.ts')).toBe(false); + }); + + it('reads .dockerignore from the resolved, absolute dockerContextDir root', () => { + fsMock.readFileSync.mockReturnValue(''); + loadDockerIgnore('actors/shopify'); + expect(fsMock.readFileSync).toHaveBeenCalledWith( + path.join(path.resolve('actors/shopify'), '.dockerignore'), + 'utf-8', + ); + }); + + it('reads .dockerignore from the resolved repo root when dockerContextDir is empty', () => { + fsMock.readFileSync.mockReturnValue(''); + loadDockerIgnore(''); + expect(fsMock.readFileSync).toHaveBeenCalledWith(path.join(path.resolve(''), '.dockerignore'), 'utf-8'); + }); + + it('handles comments and blank lines', () => { + fsMock.readFileSync.mockReturnValue('# this is a comment\n\nnode_modules\n'); + const matcher = loadDockerIgnore(''); + expect(matcher('node_modules/foo.js')).toBe(true); + expect(matcher('src/main.ts')).toBe(false); + }); + + it('normalizes a leading "./" in patterns, which Docker treats as a no-op', () => { + // The common real-world .dockerignore style: every pattern prefixed with "./", + // including a negation re-including one of the otherwise-matched files. + fsMock.readFileSync.mockReturnValue('./node_modules\n./*.log\n!./keep.log\n'); + const matcher = loadDockerIgnore(''); + expect(matcher('node_modules/foo.js')).toBe(true); + expect(matcher('debug.log')).toBe(true); + expect(matcher('keep.log')).toBe(false); + expect(matcher('main.js')).toBe(false); + }); +}); + +describe('buildDockerIgnoreMatcher', () => { + it('treats paths as already relative to absoluteRootDir when hoistFrom is left at its default', () => { + fsMock.readFileSync.mockReturnValue('test/\n*.log\n'); + const matcher = buildDockerIgnoreMatcher('/repo/actors/shopify'); + expect(matcher('test/fixture.json')).toBe(true); + expect(matcher('debug.log')).toBe(true); + expect(matcher('main.js')).toBe(false); + }); + + it('reads .dockerignore from the given absoluteRootDir', () => { + fsMock.readFileSync.mockReturnValue(''); + buildDockerIgnoreMatcher('/repo/actors/shopify'); + expect(fsMock.readFileSync).toHaveBeenCalledWith('/repo/actors/shopify/.dockerignore', 'utf-8'); + }); + + it('returns no-op matcher when .dockerignore is absent', () => { + fsMock.readFileSync.mockImplementation(() => { + throw new Error('ENOENT'); + }); + const matcher = buildDockerIgnoreMatcher('/repo/actors/shopify'); + expect(matcher('main.js')).toBe(false); + }); +}); diff --git a/test/unit/bin/path-utils.test.ts b/test/unit/bin/path-utils.test.ts new file mode 100644 index 0000000..2f46e96 --- /dev/null +++ b/test/unit/bin/path-utils.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest'; + +import { findContainingScope, hoistPath, isPathWithinScope } from '../../../bin/path-utils.js'; + +describe('path-utils', () => { + it('isPathWithinScope matches a scope entry that names an exact file, not just a directory prefix', () => { + expect(isPathWithinScope('shared/utils.ts', 'shared/utils.ts')).toBe(true); + }); + + it('findContainingScope picks the exact-file entry out of a list of scope paths', () => { + expect(findContainingScope('shared/utils.ts', ['actors/foo', 'shared/utils.ts'])).toBe('shared/utils.ts'); + }); + + it('hoistPath resolves an exact-file scope match to an empty string, not an out-of-bounds slice', () => { + expect(hoistPath('shared/utils.ts', 'shared/utils.ts')).toBe(''); + }); +}); diff --git a/test/unit/bin/utils.test.ts b/test/unit/bin/utils.test.ts new file mode 100644 index 0000000..5fa2149 --- /dev/null +++ b/test/unit/bin/utils.test.ts @@ -0,0 +1,343 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { CONFIG_FILE_NAME, readConfigFile } from '../../../bin/utils.js'; + +const { fsMock } = vi.hoisted(() => ({ + fsMock: { + readFile: vi.fn(), + }, +})); + +vi.mock('node:fs/promises', () => ({ default: fsMock })); + +afterEach(() => vi.restoreAllMocks()); + +const validConfig = (actors: object[]) => JSON.stringify({ actors }); +const actorJson = (fields: Record = {}) => JSON.stringify(fields); + +const expectFileRead = (filePath: string) => { + expect(fsMock.readFile).toHaveBeenCalledWith(filePath, expect.anything()); +}; + +const mockFiles = (files: Record) => { + fsMock.readFile.mockImplementation(async (filePath: string) => { + if (filePath in files) return Promise.resolve(files[filePath]); + return Promise.reject(new Error(`ENOENT: ${filePath}`)); + }); +}; + +describe('readConfigFile', () => { + it('returns correct ActorConfig[] for a valid config', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify', + actorFullName: 'myteam/shopify-scraper', + tokenEnvVar: 'APIFY_TOKEN_MYTEAM', + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({ dockerContextDir: '../../..' }), + }); + + const result = await readConfigFile(); + expectFileRead('actors/shopify/.actor/actor.json'); + expect(result).toEqual([ + { + actorFullName: 'myteam/shopify-scraper', + folder: 'actors/shopify', + tokenEnvVar: 'APIFY_TOKEN_MYTEAM', + dockerContextDir: '', + contextPaths: [''], + }, + ]); + }); + + it('normalizes folder "." to ""', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: '.', actorFullName: 'apify/my-actor', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + ]), + '.actor/actor.json': actorJson({}), + }); + + const result = await readConfigFile(); + expect(result[0].folder).toBe(''); + expectFileRead('.actor/actor.json'); + }); + + it('defaults dockerContextDir to actor folder when absent from actor.json', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: 'actors/web-scraper', actorFullName: 'apify/web-scraper', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + ]), + 'actors/web-scraper/.actor/actor.json': actorJson({}), + }); + + const result = await readConfigFile(); + expect(result[0].dockerContextDir).toBe('actors/web-scraper'); + expect(result[0].contextPaths).toEqual(['actors/web-scraper']); + }); + + it('resolves dockerContextDir relative to .actor/ folder', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: 'actors/shopify', actorFullName: 'myteam/shopify', tokenEnvVar: 'APIFY_TOKEN' }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({ dockerContextDir: '../../..' }), + }); + + const result = await readConfigFile(); + expect(result[0].dockerContextDir).toBe(''); + }); + + it('resolves contextPaths from overrideActorContext', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify', + actorFullName: 'myteam/shopify', + tokenEnvVar: 'APIFY_TOKEN', + overrideActorContext: ['actors/shopify', 'packages'], + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({ dockerContextDir: '../../..' }), + }); + + const result = await readConfigFile(); + expect(result[0].contextPaths).toEqual(['actors/shopify', 'packages']); + }); + + it('handles multiple actors', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: 'actors/web-scraper', actorFullName: 'apify/web-scraper', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + { + folder: 'actors/email-sender', + actorFullName: 'other-team/email-sender', + tokenEnvVar: 'APIFY_TOKEN_OTHER_TEAM', + }, + ]), + 'actors/web-scraper/.actor/actor.json': actorJson({}), + 'actors/email-sender/.actor/actor.json': actorJson({}), + }); + + const result = await readConfigFile(); + expect(result).toHaveLength(2); + expect(result[0].actorFullName).toBe('apify/web-scraper'); + expect(result[1].actorFullName).toBe('other-team/email-sender'); + }); + + it('throws when config file is missing', async () => { + fsMock.readFile.mockRejectedValue(new Error('ENOENT')); + await expect(readConfigFile()).rejects.toThrow('not found'); + }); + + it('throws when config file contains invalid JSON', async () => { + fsMock.readFile.mockResolvedValue('{bad json'); + await expect(readConfigFile()).rejects.toThrow('invalid JSON'); + }); + + it('throws when actors array is missing', async () => { + fsMock.readFile.mockResolvedValue(JSON.stringify({ notActors: [] })); + await expect(readConfigFile()).rejects.toThrow('"actors" array'); + }); + + it('throws on duplicate folders', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: 'actors/shopify', actorFullName: 'apify/shopify', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + { folder: 'actors/shopify', actorFullName: 'other/shopify', tokenEnvVar: 'APIFY_TOKEN_OTHER' }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow('Duplicate folder'); + }); + + it('throws on duplicate folders after normalization ("." and "")', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: '.', actorFullName: 'apify/actor-a', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + { folder: '', actorFullName: 'other/actor-b', tokenEnvVar: 'APIFY_TOKEN_OTHER' }, + ]), + '.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow('Duplicate folder'); + }); + + it('throws when actor.json is missing', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: 'actors/shopify', actorFullName: 'apify/shopify', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + ]), + }); + + await expect(readConfigFile()).rejects.toThrow('Cannot read'); + }); + + it('throws when folder is missing', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([{ actorFullName: 'apify/shopify', tokenEnvVar: 'APIFY_TOKEN_APIFY' }]), + }); + + await expect(readConfigFile()).rejects.toThrow(/Invalid "folder"/); + }); + + it('throws when folder is not a string', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: 123, actorFullName: 'apify/shopify', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + ]), + }); + + await expect(readConfigFile()).rejects.toThrow(/Invalid "folder"/); + }); + + it('throws when actorFullName is missing', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([{ folder: 'actors/shopify', tokenEnvVar: 'APIFY_TOKEN_APIFY' }]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow('Invalid "actorFullName"'); + }); + + it('throws when actorFullName has no slash', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: 'actors/shopify', actorFullName: 'shopify-scraper', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow('Invalid "actorFullName"'); + }); + + it('throws when actorFullName has empty parts', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { folder: 'actors/shopify', actorFullName: '/shopify', tokenEnvVar: 'APIFY_TOKEN_APIFY' }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow('Invalid "actorFullName"'); + }); + + it('throws when overrideActorContext is not an array', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify', + actorFullName: 'myteam/shopify', + tokenEnvVar: 'APIFY_TOKEN', + overrideActorContext: 'packages', + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow('Invalid "overrideActorContext"'); + }); + + it('throws when overrideActorContext contains non-strings', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify', + actorFullName: 'myteam/shopify', + tokenEnvVar: 'APIFY_TOKEN', + overrideActorContext: [123], + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow('Invalid "overrideActorContext"'); + }); + + it('throws when overrideActorContext entries overlap (one is a prefix of another)', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify', + actorFullName: 'myteam/shopify', + tokenEnvVar: 'APIFY_TOKEN', + overrideActorContext: ['actors/shopify', 'actors'], + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow(/overlap/); + }); + + it('throws when overrideActorContext contains the repo root alongside another entry', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify', + actorFullName: 'myteam/shopify', + tokenEnvVar: 'APIFY_TOKEN', + overrideActorContext: ['', 'actors/shopify'], + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + await expect(readConfigFile()).rejects.toThrow(/overlap/); + }); + + it('adds the actor own folder automatically when overrideActorContext does not cover it', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify', + actorFullName: 'myteam/shopify', + tokenEnvVar: 'APIFY_TOKEN', + overrideActorContext: ['code', 'shared'], + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + const result = await readConfigFile(); + expect(result[0].contextPaths).toEqual(['code', 'shared', 'actors/shopify']); + }); + + it('strips trailing slashes from folder and overrideActorContext entries', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify/', + actorFullName: 'myteam/shopify', + tokenEnvVar: 'APIFY_TOKEN', + overrideActorContext: ['actors/shopify/', 'packages/'], + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + const result = await readConfigFile(); + expect(result[0].folder).toBe('actors/shopify'); + expect(result[0].contextPaths).toEqual(['actors/shopify', 'packages']); + }); + + it('allows overrideActorContext with disjoint sibling paths that all reach the actor folder via one entry', async () => { + mockFiles({ + [CONFIG_FILE_NAME]: validConfig([ + { + folder: 'actors/shopify', + actorFullName: 'myteam/shopify', + tokenEnvVar: 'APIFY_TOKEN', + overrideActorContext: ['actors/shopify', 'code', 'shared'], + }, + ]), + 'actors/shopify/.actor/actor.json': actorJson({}), + }); + + const result = await readConfigFile(); + expect(result[0].contextPaths).toEqual(['actors/shopify', 'code', 'shared']); + }); +}); diff --git a/test/unit/should-built-and-test.test.ts b/test/unit/should-built-and-test.test.ts index 3dfb131..f68ab17 100644 --- a/test/unit/should-built-and-test.test.ts +++ b/test/unit/should-built-and-test.test.ts @@ -9,19 +9,25 @@ describe('Should build and test parser', () => { // From https://github.com/apify-store/testing-repo-for-github-actions const ACTOR_CONFIGS: ActorConfig[] = [ { - actorName: 'lukaskrivka/testing-github-integration-1', + actorFullName: 'lukaskrivka/testing-github-integration-1', folder: 'actors/lukaskrivka_testing-github-integration-1', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_LUKASKRIVKA', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'lukaskrivka/testing-github-integration-2', + actorFullName: 'lukaskrivka/testing-github-integration-2', folder: 'actors/lukaskrivka_testing-github-integration-2', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_LUKASKRIVKA', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'lukaskrivka/test-standalone', + actorFullName: 'lukaskrivka/test-standalone', folder: 'standalone-actors/lukaskrivka_test-standalone', - isStandalone: true, + tokenEnvVar: 'APIFY_TOKEN_LUKASKRIVKA', + dockerContextDir: 'standalone-actors/lukaskrivka_test-standalone', + contextPaths: ['standalone-actors/lukaskrivka_test-standalone'], }, ]; @@ -51,7 +57,7 @@ describe('Should build and test parser', () => { }); test('Ignores other ignored files and folders', () => { - const FILES = ['.vscode/', '.gitignore', '.husky/', '.eslintrc', '.editorconfig', '.actor/']; + const FILES = ['.vscode/', '.gitignore', '.husky/', '.eslintrc', '.editorconfig']; const actorsChanged = getChangedActors({ actorConfigs: ACTOR_CONFIGS, @@ -63,9 +69,42 @@ describe('Should build and test parser', () => { expect(actorsChanged).toEqual([]); }); - test('Only builds latest for all Actors', () => { + test('.actor/ changes trigger builds for broad-context actors', () => { + const FILES = ['.actor/actor.json']; + + const actorsChanged = getChangedActors({ + actorConfigs: ACTOR_CONFIGS, + isLatest: false, + filepathsChanged: FILES, + commits, + }); + + expect(actorsChanged).toEqual(ACTOR_CONFIGS.slice(0, 2)); + }); + + test('Root-level changelog is cosmetic for every actor, only on latest', () => { const FILES = ['shared/CHANGELOG.md', 'CHANGELOG.md']; + const actorsChangedNotLatest = getChangedActors({ + actorConfigs: ACTOR_CONFIGS, + isLatest: false, + filepathsChanged: FILES, + commits, + }); + expect(actorsChangedNotLatest).toEqual([]); + + const actorsChangedLatest = getChangedActors({ + actorConfigs: ACTOR_CONFIGS, + isLatest: true, + filepathsChanged: FILES, + commits, + }); + expect(actorsChangedLatest).toEqual(ACTOR_CONFIGS); + }); + + test('A changelog nested inside one actor own folder is excluded for sibling actors, only triggers that actor', () => { + const FILES = ['actors/lukaskrivka_testing-github-integration-1/CHANGELOG.md']; + const actorsChanged = getChangedActors({ actorConfigs: ACTOR_CONFIGS, isLatest: true, @@ -73,10 +112,10 @@ describe('Should build and test parser', () => { commits, }); - expect(actorsChanged).toEqual(ACTOR_CONFIGS.filter(({ isStandalone }) => !isStandalone)); + expect(actorsChanged).toEqual([ACTOR_CONFIGS[0]]); }); - test('Code updated, tests miniactors', () => { + test('Code updated, tests broad-context actors', () => { const FILES = ['code/src/main.ts', 'package.json']; const actorsChanged = getChangedActors({ @@ -86,7 +125,7 @@ describe('Should build and test parser', () => { commits, }); - expect(actorsChanged).toEqual(ACTOR_CONFIGS.filter(({ isStandalone }) => !isStandalone)); + expect(actorsChanged).toEqual(ACTOR_CONFIGS.slice(0, 2)); }); test('Specific Actor functionality configs updated', () => { @@ -119,7 +158,7 @@ describe('Should build and test parser', () => { expect(actorsChanged).toEqual(ACTOR_CONFIGS.slice(0, 2)); }); - test('Miniactor, Code and standalone actor updated,', () => { + test('Actor folder, shared code and narrow-context actor updated', () => { const FILES = [ 'actors/lukaskrivka_testing-github-integration-1/.actor/actor.json', 'code/src/main.ts', @@ -216,7 +255,7 @@ describe('Should build and test parser', () => { expect(actorsChanged).toEqual([ACTOR_CONFIGS[1]]); }); - test('Standalone actor with cosmetic-only JSON change in PR context skips tests', () => { + test('Narrow-context actor with cosmetic-only JSON change in PR context skips tests', () => { const FILES = ['standalone-actors/lukaskrivka_test-standalone/.actor/actor.json']; isCosmeticOnlyJsonSchemaSpy.mockReturnValue(true); @@ -249,49 +288,67 @@ describe('Should build and test parser', () => { const ACTOR_CONFIGS_GOOGLE_MAPS: ActorConfig[] = [ { // Edge case of capitals in actor name :) - actorName: 'compass/Google-Maps-Reviews-Scraper', + actorFullName: 'compass/Google-Maps-Reviews-Scraper', folder: 'actors/compass_Google-Maps-Reviews-Scraper', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_COMPASS', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'compass/crawler-google-places', + actorFullName: 'compass/crawler-google-places', folder: 'actors/compass_crawler-google-places', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_COMPASS', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'compass/easy-google-maps', + actorFullName: 'compass/easy-google-maps', folder: 'actors/compass_easy-google-maps', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_COMPASS', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'compass/google-maps-extractor', + actorFullName: 'compass/google-maps-extractor', folder: 'actors/compass_google-maps-extractor', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_COMPASS', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'compass/google-places-api', + actorFullName: 'compass/google-places-api', folder: 'actors/compass_google-places-api', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_COMPASS', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'lukaskrivka/google-maps-with-contact-details', + actorFullName: 'lukaskrivka/google-maps-with-contact-details', folder: 'actors/lukaskrivka_google-maps-with-contact-details', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_LUKASKRIVKA', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'natasha.lekh/gas-prices-scraper', + actorFullName: 'natasha.lekh/gas-prices-scraper', folder: 'actors/natasha.lekh_gas-prices-scraper', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_NATASHA_LEKH', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'natasha.lekh/vegan-places-finder', + actorFullName: 'natasha.lekh/vegan-places-finder', folder: 'actors/natasha.lekh_vegan-places-finder', - isStandalone: false, + tokenEnvVar: 'APIFY_TOKEN_NATASHA_LEKH', + dockerContextDir: '', + contextPaths: [''], }, { - actorName: 'lukaskrivka/google-maps-scraper-orchestrator', + actorFullName: 'lukaskrivka/google-maps-scraper-orchestrator', folder: 'standalone-actors/lukaskrivka_google-maps-scraper-orchestrator', - isStandalone: true, + tokenEnvVar: 'APIFY_TOKEN_LUKASKRIVKA', + dockerContextDir: 'standalone-actors/lukaskrivka_google-maps-scraper-orchestrator', + contextPaths: ['standalone-actors/lukaskrivka_google-maps-scraper-orchestrator'], }, ]; @@ -302,6 +359,6 @@ describe('Should build and test parser', () => { commits, }); - expect(actorsChanged).toEqual(ACTOR_CONFIGS_GOOGLE_MAPS.filter(({ isStandalone }) => !isStandalone)); + expect(actorsChanged).toEqual(ACTOR_CONFIGS_GOOGLE_MAPS.slice(0, 8)); }); });