Skip to content

Feat: skip building actors if only tests changed #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kazadoiyul wants to merge 2 commits into
base: pretify-build-logs
Choose a base branch
from
Open

Feat: skip building actors if only tests changed #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d6632f
feat: ignore test files
kazadoiyul Oct 22, 2025
f370276
feat: get existing builds to run tests
kazadoiyul Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions bin/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ class ApifyBuilder {
await this.apifyClient.build(build.id).delete();
}
}

async getDefaultBuilt() {
Copy link
Collaborator

@oklinov oklinov Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this isn't needed...you can extend getDefaultVersionAndTag to return buildId:

    const defaultBuildId = actorInfo.taggedBuilds![defaultBuildTag].buildId!;

const client = this.apifyClient.actor(this.actorName)
const { defaultVersionNumber } = await this.getDefaultVersionAndTag()
const { id, actId, buildNumber } = await client.build(defaultVersionNumber)
return { buildId: id, actorId: actId, buildNumber, actorName: this.actorName };
}
}

type RunBuildsOptions = {
Expand All @@ -184,6 +191,15 @@ type RunBuildsOptions = {
dryRun: boolean
}

export const getAllDefaultBuilds = async (actorConfigs: ActorConfig[]) => {
const existingBuilds = await Promise.all(
actorConfigs.map(actor =>
ApifyBuilder.fromActorName(actor.actorName).getDefaultBuilt()
)
);
return existingBuilds;
}

export const runBuilds = async ({
repoUrl,
branch,
Expand Down
14 changes: 12 additions & 2 deletions bin/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
spawnCommandInGhWorkspace,
setCwd,
} from './utils.js';
import { runBuilds } from './build.js';
import {getAllDefaultBuilds, runBuilds} from './build.js';
Copy link
Collaborator

@oklinov oklinov Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import {getAllDefaultBuilds, runBuilds} from './build.js';
import { getAllDefaultBuilds, runBuilds } from './build.js';

import { getChangedFiles, getCommits } from './git.js';
import { getPushData } from './github.js';
import { notifyToSlack } from './slack.js';
Expand Down Expand Up @@ -107,7 +107,17 @@ await yargs()
branch: args.sourceBranch.replace('origin/', ''),
dryRun: args.dryRun,
});
console.log(JSON.stringify(builds));

if (args.dryRun) {
console.log(JSON.stringify([...builds]));
return;
}

// add a build entry even if no build was triggered to run the tests on all actors
Copy link
Collaborator

@oklinov oklinov Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will run always the tests for the actors that we're not building, no? My idea was to run them only if there is a changed in platform tests (in test/platfrom).

What I would do is to have getChangedActors return actorsToTest if there is such change. So, if we detect that there is a change in platform tests, we put all the actors that are not being built to actorsToTest.

I prepared some unit tests that hopefully explain the logic:

    test('only core tests changed', async () => {
        const FILES = [
            'test/platform/core/core.test.ts',
        ];

        const { actorsChanged, actorsToTest, codeChanged } = await getChangedActors({ actorConfigs: ACTOR_CONFIGS, isLatest: false, filepathsChanged: FILES });

        expect(actorsChanged).toEqual([]);
        expect(actorsToTest).toEqual(ACTOR_CONFIGS);
        expect(codeChanged).toBe(false);
    });

    test('core tests and src (excluding standalone actors) changed', async () => {
        const FILES = [
            'src/main.ts',
            'test/platform/core/core.test.ts',
        ];

        const { actorsChanged, actorsToTest, codeChanged } = await getChangedActors({ actorConfigs: ACTOR_CONFIGS, isLatest: false, filepathsChanged: FILES });

        expect(actorsChanged).toEqual(ACTOR_CONFIGS.filter((actor) => actor.isStandalone));
        expect(actorsToTest).toEqual(ACTOR_CONFIGS.filter((actor) => !actor.isStandalone));
        expect(codeChanged).toBe(false);
    });

    test('core tests and one input_schema changed', async () => {
        const FILES = [
            'test/platform/core/core.test.ts',
            'actors/lukaskrivka_testing-github-integration-1/.actor/INPUT_SCHEMA.json'
        ];

        const { actorsChanged, actorsToTest, codeChanged } = await getChangedActors({ actorConfigs: ACTOR_CONFIGS, isLatest: false, filepathsChanged: FILES });

        expect(actorsChanged).toEqual([ACTOR_CONFIGS[0]]);
        expect(actorsToTest).toEqual(ACTOR_CONFIGS.slice(1));
        expect(codeChanged).toBe(false);
    });

let me know if something's not clear

const remainingActors = actorConfigs.filter((a) => !actorsChanged.find((ac) => ac.actorName === a.actorName))
const existingBuilds = await getAllDefaultBuilds(remainingActors)

console.log(JSON.stringify([...builds, ...existingBuilds]));
})
.command(
'release',
Expand Down
7 changes: 6 additions & 1 deletion bin/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ const isIgnoredTopLevelFile = (lowercaseFilePath: string) => {
return IGNORED_TOP_LEVEL_FILES.some((ignoredFile) => sanitizedLowercaseFilePath.startsWith(ignoredFile));
};

const isIgnoredTestFile = (lowercaseFilePath: string) => {
Copy link
Collaborator

@oklinov oklinov Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should check if the lowercaseFilePath is a platform test, you can do it like this:

        return lowercaseFilePath.match(/^(code\/)?test\/platform\//);

I'd also rename it to isPlatformTestFile

const sanitizedLowercaseFilePath = lowercaseFilePath.replace(/^code\//, '').replace(/^shared\//, '');
return sanitizedLowercaseFilePath.startsWith('tests/');
}

const isLatestBuildOnlyFile = (lowercaseFilePath: string) => {
if (lowercaseFilePath.endsWith('changelog.md')) {
return true;
Expand Down Expand Up @@ -145,7 +150,7 @@ export const getChangedActors = (
const lowercaseFiles = filepathsChanged.map((file) => file.toLowerCase());

for (const lowercaseFilePath of lowercaseFiles) {
if (isIgnoredTopLevelFile(lowercaseFilePath)) {
if (isIgnoredTopLevelFile(lowercaseFilePath) || isIgnoredTestFile(lowercaseFilePath)) {
continue;
}
// First we check for specific actors that have configs in /actors or standalone actors in /standalone-actors
Expand Down