From c08719b9ec051c7e47931aadaa4c137d3952a7a2 Mon Sep 17 00:00:00 2001 From: Onur Solmaz <2453968+osolmaz@users.noreply.github.com> Date: Wed, 8 Jul 2026 20:17:50 +0800 Subject: [PATCH] feat: show progress during owner-wide planning --- README.md | 6 ++ slophammer.yml | 6 ++ src/app/apply.ts | 36 +++++-- src/app/planner.ts | 65 ++++++++++--- src/app/types.ts | 62 ++++++++++++ src/cli/args.ts | 29 +++++- src/cli/main.ts | 59 +++++++----- src/cli/progress.ts | 192 ++++++++++++++++++++++++++++++++++++++ src/github/client.ts | 71 ++++++++++++-- src/shared/concurrency.ts | 57 +++++++++++ stryker.conf.json | 1 + tests/apply.test.ts | 57 ++++++++++- tests/args.test.ts | 9 ++ tests/client.test.ts | 22 ++++- tests/concurrency.test.ts | 39 ++++++++ tests/planner.test.ts | 59 +++++++++++- tests/progress.test.ts | 104 +++++++++++++++++++++ 17 files changed, 810 insertions(+), 64 deletions(-) create mode 100644 src/cli/progress.ts create mode 100644 src/shared/concurrency.ts create mode 100644 tests/concurrency.test.ts create mode 100644 tests/progress.test.ts diff --git a/README.md b/README.md index f17b4d1..62e878d 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,12 @@ github-sane-defaults apply example-org/example-repo --yes The `--owner example-owner --repo example-repo` form is also accepted. The legacy `--org example-org --repo example-repo` form remains supported. +Long owner-wide runs show a single-line progress bar on `stderr` when running in +an interactive terminal. The status line covers repository detail loading, +planning, and the mutation phase of `apply`. The final plan and apply summaries +still print to `stdout`, so scripted output stays clean. Use `--no-progress` to +disable the status bar or `--progress` to force it. + ## Defaults Repository settings: diff --git a/slophammer.yml b/slophammer.yml index 05e0792..ad6f8b2 100644 --- a/slophammer.yml +++ b/slophammer.yml @@ -19,17 +19,23 @@ typescript: - src/cli - src/github/parse.ts - src/policy + - src/shared dependency_boundaries: - from: src/cli allow: - src/app - src/github + - src/shared - from: src/app allow: - src/github - src/policy + - src/shared - from: src/github allow: - src/policy + - src/shared - from: src/policy allow: [] + - from: src/shared + allow: [] diff --git a/src/app/apply.ts b/src/app/apply.ts index 894e726..9e9ca7c 100644 --- a/src/app/apply.ts +++ b/src/app/apply.ts @@ -1,30 +1,44 @@ import type { GitHubClient } from "../github/client.js"; import { desiredRulesetPayload, RULESET_NAME } from "../policy/defaults.js"; -import type { ApplySummary, RepoPlan, TargetSelection } from "./types.js"; +import type { ApplyOptions, ApplySummary, RepoPlan, TargetSelection } from "./types.js"; import { buildPlan } from "./planner.js"; import { mergeRulesetPayload } from "./planner.js"; export async function applyDefaults( client: GitHubClient, - selection: TargetSelection + selection: TargetSelection, + options: ApplyOptions = {} ): Promise { - const planned = await buildPlan(client, selection); + const planned = await buildPlan(client, selection, options); - await applyPlannedDefaults(client, selection.owner, planned); + await applyPlannedDefaults(client, selection.owner, planned, options); return { planned, - applied: planned.length + applied: countChangedRepos(planned) }; } export async function applyPlannedDefaults( client: GitHubClient, owner: string, - planned: RepoPlan[] + planned: RepoPlan[], + options: ApplyOptions = {} ): Promise { - for (const plan of planned) { + const changedPlans = planned.filter(repoPlanHasChanges); + let completed = 0; + + options.progress?.applyingRepos?.({ owner, total: changedPlans.length }); + + for (const plan of changedPlans) { await applyRepoPlan(client, owner, plan); + completed += 1; + options.progress?.appliedRepo?.({ + owner, + total: changedPlans.length, + completed, + current: plan.fullName + }); } } @@ -60,3 +74,11 @@ async function applyRepoPlan(client: GitHubClient, owner: string, plan: RepoPlan mergeRulesetPayload(existingRuleset, desired) ); } + +function countChangedRepos(planned: RepoPlan[]): number { + return planned.filter(repoPlanHasChanges).length; +} + +function repoPlanHasChanges(plan: RepoPlan): boolean { + return plan.settingChanges.length > 0 || plan.ruleset.action !== "none"; +} diff --git a/src/app/planner.ts b/src/app/planner.ts index 05025fc..072f3f1 100644 --- a/src/app/planner.ts +++ b/src/app/planner.ts @@ -3,29 +3,66 @@ import type { GitHubRuleset, RulesetSummary } from "../github/types.js"; import type { GitHubRepo } from "../github/types.js"; import { desiredRulesetPayload, repoSettingChanges, RULESET_NAME } from "../policy/defaults.js"; import type { ExistingRulesetRule, RefNameCondition, RulesetPayload } from "../policy/types.js"; -import type { RepoPlan, RulesetPlan, TargetSelection } from "./types.js"; +import { mapWithConcurrency } from "../shared/concurrency.js"; +import type { BuildPlanOptions, RepoPlan, RulesetPlan, TargetSelection } from "./types.js"; + +const DEFAULT_PLAN_CONCURRENCY = 8; export async function buildPlan( client: GitHubClient, - selection: TargetSelection + selection: TargetSelection, + options: BuildPlanOptions = {} ): Promise { - const repos = await selectRepos(client, selection); - const activeRepos = repos.filter((repo) => !repo.archived && !repo.disabled); - const plans: RepoPlan[] = []; + options.progress?.loadingRepos?.(selection.owner); - for (const repo of activeRepos) { - plans.push(await buildRepoPlan(client, selection.owner, repo)); - } - - return plans; + const repos = await selectRepos(client, selection, options); + const activeRepos = repos.filter((repo) => !repo.archived && !repo.disabled); + const skipped = repos.length - activeRepos.length; + const progressState = { completed: 0, changed: 0, clean: 0 }; + + options.progress?.selectedRepos?.({ + owner: selection.owner, + total: activeRepos.length, + skipped + }); + + return mapWithConcurrency( + activeRepos, + options.concurrency ?? DEFAULT_PLAN_CONCURRENCY, + async (repo) => { + const plan = await buildRepoPlan(client, selection.owner, repo); + progressState.completed += 1; + + if (repoPlanHasChanges(plan)) { + progressState.changed += 1; + } else { + progressState.clean += 1; + } + + options.progress?.plannedRepo?.({ + owner: selection.owner, + total: activeRepos.length, + completed: progressState.completed, + changed: progressState.changed, + clean: progressState.clean, + skipped, + current: repo.full_name + }); + + return plan; + } + ); } async function selectRepos( client: GitHubClient, - selection: TargetSelection + selection: TargetSelection, + options: BuildPlanOptions ): Promise { if (selection.all) { - return client.listOwnerRepos(selection.owner); + return options.progress === undefined + ? client.listOwnerRepos(selection.owner) + : client.listOwnerRepos(selection.owner, { progress: options.progress }); } return Promise.all(selection.repos.map((repo) => client.getRepo(selection.owner, repo))); @@ -101,6 +138,10 @@ function managedRulesetSatisfiesDesired(existing: GitHubRuleset, desired: Rulese return existing.name === desired.name && rulesetSatisfiesDesired(existing, desired); } +function repoPlanHasChanges(plan: RepoPlan): boolean { + return plan.settingChanges.length > 0 || plan.ruleset.action !== "none"; +} + export function rulesetSatisfiesDesired(existing: GitHubRuleset, desired: RulesetPayload): boolean { return ( existing.enforcement === desired.enforcement && diff --git a/src/app/types.ts b/src/app/types.ts index 96957ca..4317bd4 100644 --- a/src/app/types.ts +++ b/src/app/types.ts @@ -6,6 +6,68 @@ export type TargetSelection = { all: boolean; }; +export type PlanProgressSelectedRepos = { + owner: string; + total: number; + skipped: number; +}; + +export type PlanProgressLoadingRepoDetails = { + owner: string; + total: number; +}; + +export type PlanProgressLoadedRepoDetails = { + owner: string; + total: number; + completed: number; + current: string; +}; + +export type PlanProgressPlannedRepo = { + owner: string; + total: number; + completed: number; + changed: number; + clean: number; + skipped: number; + current: string; +}; + +export type PlanProgress = { + loadingRepos?(owner: string): void; + loadingRepoDetails?(state: PlanProgressLoadingRepoDetails): void; + loadedRepoDetails?(state: PlanProgressLoadedRepoDetails): void; + selectedRepos?(state: PlanProgressSelectedRepos): void; + plannedRepo?(state: PlanProgressPlannedRepo): void; +}; + +export type BuildPlanOptions = { + concurrency?: number; + progress?: PlanProgress; +}; + +export type ApplyProgressSelectedRepos = { + owner: string; + total: number; +}; + +export type ApplyProgressAppliedRepo = { + owner: string; + total: number; + completed: number; + current: string; +}; + +export type ApplyProgress = { + applyingRepos?(state: ApplyProgressSelectedRepos): void; + appliedRepo?(state: ApplyProgressAppliedRepo): void; +}; + +export type ApplyOptions = { + progress?: ApplyProgress & PlanProgress; +}; + export type RulesetPlan = { action: "create" | "update" | "none"; coveredBy?: string; diff --git a/src/cli/args.ts b/src/cli/args.ts index f99eb4a..c05793f 100644 --- a/src/cli/args.ts +++ b/src/cli/args.ts @@ -1,9 +1,11 @@ import type { TargetSelection } from "../app/types.js"; +import type { ProgressMode } from "./progress.js"; export type CliCommand = "plan" | "apply"; export type CliOptions = TargetSelection & { command: CliCommand; + progress?: ProgressMode; token?: string; yes: boolean; }; @@ -31,6 +33,10 @@ export function parseArgs(args: string[]): CliOptions { options.token = parsed.token; } + if (parsed.progress !== "auto") { + options.progress = parsed.progress; + } + return options; } @@ -52,6 +58,7 @@ function validateFlags(parsed: ParsedFlags): asserts parsed is ParsedFlags & { o type ParsedFlags = { owner?: string; + progress: ProgressMode; token?: string; repos: string[]; targets: string[]; @@ -60,7 +67,13 @@ type ParsedFlags = { }; function parseFlags(args: string[]): ParsedFlags { - const flags: ParsedFlags = { repos: [], targets: [], all: false, yes: false }; + const flags: ParsedFlags = { + repos: [], + targets: [], + all: false, + progress: "auto", + yes: false + }; for (let index = 0; index < args.length; index += 1) { const arg = args[index]; @@ -92,6 +105,16 @@ function parseFlag(args: string[], index: number, flags: ParsedFlags): number { return index; } + if (arg === "--progress") { + flags.progress = "always"; + return index; + } + + if (arg === "--no-progress") { + flags.progress = "never"; + return index; + } + if (!arg.startsWith("--")) { flags.targets.push(arg); return index; @@ -192,6 +215,8 @@ export function usage(): string { " github-sane-defaults apply --owner --repo ", "", "Options:", - " -y, --yes Skip apply confirmation" + " -y, --yes Skip apply confirmation", + " --progress Show progress even when stderr is not a TTY", + " --no-progress Disable progress output" ].join("\n"); } diff --git a/src/cli/main.ts b/src/cli/main.ts index 96fad42..7986ed8 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -12,37 +12,46 @@ import { shouldUseColor } from "./format.js"; import { parseArgs } from "./args.js"; +import { createProgressReporter } from "./progress.js"; async function main(): Promise { const options = parseArgs(process.argv.slice(2)); const client = new RestGitHubClient(resolveToken(options.token)); const formatOptions = { color: shouldUseColor() }; - - if (options.command === "plan") { - console.log(formatPlan(await buildPlan(client, options), formatOptions)); - return; - } - - const planned = await buildPlan(client, options); - - console.log(formatPlan(planned, formatOptions)); - - if (!planHasChanges(planned)) { - console.error("No changes to apply."); - return; + const progress = createProgressReporter(options.progress); + + try { + if (options.command === "plan") { + const planned = await buildPlan(client, options, { progress }); + progress.stop(); + console.log(formatPlan(planned, formatOptions)); + return; + } + + const planned = await buildPlan(client, options, { progress }); + progress.stop(); + + console.log(formatPlan(planned, formatOptions)); + + if (!planHasChanges(planned)) { + console.error("No changes to apply."); + return; + } + + if (!options.yes && !(await confirmApply())) { + console.error("Apply cancelled."); + process.exitCode = 1; + return; + } + + await applyPlannedDefaults(client, options.owner, planned, { progress }); + + const summary: ApplySummary = { planned, applied: countChangedRepos(planned) }; + console.log(""); + console.log(formatApplySummary(summary, formatOptions)); + } finally { + progress.stop(); } - - if (!options.yes && !(await confirmApply())) { - console.error("Apply cancelled."); - process.exitCode = 1; - return; - } - - await applyPlannedDefaults(client, options.owner, planned); - - const summary: ApplySummary = { planned, applied: countChangedRepos(planned) }; - console.log(""); - console.log(formatApplySummary(summary, formatOptions)); } main().catch((error: unknown) => { diff --git a/src/cli/progress.ts b/src/cli/progress.ts new file mode 100644 index 0000000..73c073d --- /dev/null +++ b/src/cli/progress.ts @@ -0,0 +1,192 @@ +import type { + ApplyProgress, + ApplyProgressAppliedRepo, + ApplyProgressSelectedRepos, + PlanProgress, + PlanProgressLoadedRepoDetails, + PlanProgressLoadingRepoDetails, + PlanProgressPlannedRepo, + PlanProgressSelectedRepos +} from "../app/types.js"; + +export type ProgressMode = "auto" | "always" | "never"; + +export type ProgressStream = { + columns?: number; + isTTY?: boolean; + write(chunk: string): boolean; +}; + +export type ProgressEnvironment = { + CI?: string; +}; + +export type ProgressReporter = ApplyProgress & + PlanProgress & { + stop(): void; + }; + +export function createProgressReporter( + mode: ProgressMode = "auto", + stream: ProgressStream = process.stderr, + environment: ProgressEnvironment = process.env +): ProgressReporter { + if (!shouldShowProgress(mode, stream, environment)) { + return new NoopProgressReporter(); + } + + return new StatusBarProgressReporter(stream); +} + +function shouldShowProgress( + mode: ProgressMode, + stream: ProgressStream, + environment: ProgressEnvironment +): boolean { + if (mode === "always") { + return true; + } + + if (mode === "never") { + return false; + } + + return stream.isTTY === true && (environment.CI === undefined || environment.CI.length === 0); +} + +class NoopProgressReporter implements ProgressReporter { + public stop(): void { + return undefined; + } +} + +class StatusBarProgressReporter implements ProgressReporter { + private rendered = false; + + public constructor(private readonly stream: ProgressStream) {} + + public loadingRepos(owner: string): void { + this.render(`Loading repositories for ${owner}...`); + } + + public loadingRepoDetails(state: PlanProgressLoadingRepoDetails): void { + this.render(formatRepoDetailsStatus({ ...state, completed: 0, current: "" })); + } + + public loadedRepoDetails(state: PlanProgressLoadedRepoDetails): void { + this.render(formatRepoDetailsStatus(state)); + } + + public selectedRepos(state: PlanProgressSelectedRepos): void { + this.render( + formatPlanningStatus({ + owner: state.owner, + total: state.total, + completed: 0, + changed: 0, + clean: 0, + skipped: state.skipped, + current: "" + }) + ); + } + + public plannedRepo(state: PlanProgressPlannedRepo): void { + this.render(formatPlanningStatus(state)); + } + + public applyingRepos(state: ApplyProgressSelectedRepos): void { + this.render(formatApplyingStatus({ ...state, completed: 0, current: "" })); + } + + public appliedRepo(state: ApplyProgressAppliedRepo): void { + this.render(formatApplyingStatus(state)); + } + + public stop(): void { + if (!this.rendered) { + return; + } + + this.stream.write("\r\x1b[K"); + this.rendered = false; + } + + private render(message: string): void { + this.stream.write(`\r${truncate(message, this.stream.columns)}\x1b[K`); + this.rendered = true; + } +} + +function formatPlanningStatus(state: PlanProgressPlannedRepo): string { + return formatBarStatus({ + label: `Planning ${state.owner}`, + completed: state.completed, + total: state.total, + details: [ + `changed ${String(state.changed)}`, + `clean ${String(state.clean)}`, + `skipped ${String(state.skipped)}` + ], + current: state.current + }); +} + +function formatRepoDetailsStatus(state: PlanProgressLoadedRepoDetails): string { + return formatBarStatus({ + label: `Loading repository details for ${state.owner}`, + completed: state.completed, + total: state.total, + details: [], + current: state.current + }); +} + +function formatApplyingStatus(state: ApplyProgressAppliedRepo): string { + return formatBarStatus({ + label: `Applying ${state.owner}`, + completed: state.completed, + total: state.total, + details: [], + current: state.current + }); +} + +type BarStatus = { + label: string; + completed: number; + total: number; + details: string[]; + current: string; +}; + +function formatBarStatus(status: BarStatus): string { + const current = status.current.length > 0 ? [`current: ${status.current}`] : []; + + return [ + status.label, + progressBar(status.completed, status.total), + `${String(status.completed)}/${String(status.total)}`, + ...status.details, + ...current + ].join(" "); +} + +function progressBar(completed: number, total: number): string { + const width = 20; + const rawFilled = total === 0 ? 0 : Math.floor((completed / total) * width); + const filled = Math.min(width, Math.max(0, rawFilled)); + return `[${"#".repeat(filled)}${"-".repeat(width - filled)}]`; +} + +function truncate(message: string, columns: number | undefined): string { + if (columns === undefined || columns <= 0 || message.length <= columns) { + return message; + } + + if (columns <= 1) { + return ""; + } + + return message.slice(0, columns - 1); +} diff --git a/src/github/client.ts b/src/github/client.ts index 9576f9a..0338d3d 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -2,6 +2,7 @@ import { execFileSync } from "node:child_process"; import { DESIRED_REPO_SETTINGS } from "../policy/defaults.js"; import type { RulesetPayload } from "../policy/types.js"; +import { mapWithConcurrency } from "../shared/concurrency.js"; import { parseOwnerType, parseRepo, @@ -17,6 +18,29 @@ type GitHubResponse = { headers: Headers; }; +const DEFAULT_REPO_DETAIL_CONCURRENCY = 12; + +export type RepoDetailsProgressLoading = { + owner: string; + total: number; +}; + +export type RepoDetailsProgressLoaded = { + owner: string; + total: number; + completed: number; + current: string; +}; + +export type RepoDetailsProgress = { + loadingRepoDetails?(state: RepoDetailsProgressLoading): void; + loadedRepoDetails?(state: RepoDetailsProgressLoaded): void; +}; + +export type ListOwnerReposOptions = { + progress?: RepoDetailsProgress; +}; + export class GitHubApiError extends Error { public readonly context: GitHubErrorContext; @@ -30,7 +54,7 @@ export class GitHubApiError extends Error { export type GitHubClient = { getRepo(owner: string, repo: string): Promise; - listOwnerRepos(owner: string): Promise; + listOwnerRepos(owner: string, options?: ListOwnerReposOptions): Promise; updateRepoDefaults(owner: string, repo: string): Promise; listRepoRulesets(owner: string, repo: string): Promise; getRepoRuleset(owner: string, repo: string, id: number): Promise; @@ -54,26 +78,35 @@ export class RestGitHubClient implements GitHubClient { return parseRepo(await this.request("GET", `/repos/${owner}/${repo}`)); } - public async listOwnerRepos(owner: string): Promise { + public async listOwnerRepos( + owner: string, + options: ListOwnerReposOptions = {} + ): Promise { const ownerType = parseOwnerType(await this.request("GET", `/users/${owner}`)); if (ownerType === "Organization") { - return this.listOrganizationRepos(owner); + return this.listOrganizationRepos(owner, options); } - return this.listUserOwnedRepos(owner); + return this.listUserOwnedRepos(owner, options); } - private async listOrganizationRepos(org: string): Promise { + private async listOrganizationRepos( + org: string, + options: ListOwnerReposOptions + ): Promise { const repoNames = await this.paginate( `/orgs/${org}/repos?type=all&per_page=100`, parseRepoNames ); - return Promise.all(repoNames.map((repo) => this.getRepo(org, repo))); + return this.loadRepoDetails(org, repoNames, options.progress); } - private async listUserOwnedRepos(owner: string): Promise { + private async listUserOwnedRepos( + owner: string, + options: ListOwnerReposOptions + ): Promise { const normalizedOwner = owner.toLowerCase(); const repoItems = await this.paginate( "/user/repos?visibility=all&affiliation=owner,collaborator&per_page=100", @@ -83,7 +116,29 @@ export class RestGitHubClient implements GitHubClient { .filter((repo) => repo.owner_login.toLowerCase() === normalizedOwner) .map((repo) => repo.name); - return Promise.all(ownedRepoNames.map((repo) => this.getRepo(owner, repo))); + return this.loadRepoDetails(owner, ownedRepoNames, options.progress); + } + + private async loadRepoDetails( + owner: string, + repoNames: string[], + progress: RepoDetailsProgress | undefined + ): Promise { + let completed = 0; + + progress?.loadingRepoDetails?.({ owner, total: repoNames.length }); + + return mapWithConcurrency(repoNames, DEFAULT_REPO_DETAIL_CONCURRENCY, async (repo) => { + const detail = await this.getRepo(owner, repo); + completed += 1; + progress?.loadedRepoDetails?.({ + owner, + total: repoNames.length, + completed, + current: detail.full_name + }); + return detail; + }); } public async updateRepoDefaults(owner: string, repo: string): Promise { diff --git a/src/shared/concurrency.ts b/src/shared/concurrency.ts new file mode 100644 index 0000000..fd2ce9f --- /dev/null +++ b/src/shared/concurrency.ts @@ -0,0 +1,57 @@ +export async function mapWithConcurrency( + items: readonly T[], + concurrency: number, + worker: (item: T, index: number) => Promise +): Promise { + const limit = Math.max(1, Math.floor(concurrency)); + const results: ({ value: U } | undefined)[] = Array.from( + { length: items.length }, + () => undefined + ); + let firstError: Error | undefined; + let nextIndex = 0; + + function recordFailure(error: unknown): void { + firstError ??= error instanceof Error ? error : new Error(String(error)); + } + + async function runWorker(): Promise { + while (firstError === undefined) { + const index = nextIndex; + nextIndex += 1; + + if (index >= items.length) { + return; + } + + const item = items[index]; + + if (item === undefined) { + recordFailure(new Error(`Missing concurrency item at index ${String(index)}.`)); + return; + } + + try { + results[index] = { value: await worker(item, index) }; + } catch (error) { + recordFailure(error); + return; + } + } + } + + const workerCount = Math.min(limit, items.length); + await Promise.all(Array.from({ length: workerCount }, () => runWorker())); + + if (firstError !== undefined) { + throw firstError; + } + + return results.map((result, index) => { + if (result === undefined) { + throw new Error(`Missing concurrency result at index ${String(index)}.`); + } + + return result.value; + }); +} diff --git a/stryker.conf.json b/stryker.conf.json index a7fd998..3c47735 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -13,6 +13,7 @@ "src/cli/**/*.ts", "src/github/parse.ts", "src/policy/**/*.ts", + "src/shared/**/*.ts", "!src/cli/main.ts", "!src/**/types.ts" ] diff --git a/tests/apply.test.ts b/tests/apply.test.ts index b6153dd..87303ba 100644 --- a/tests/apply.test.ts +++ b/tests/apply.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; -import { applyDefaults } from "../src/app/apply.js"; +import { applyDefaults, applyPlannedDefaults } from "../src/app/apply.js"; +import type { RepoPlan } from "../src/app/types.js"; import type { GitHubClient } from "../src/github/client.js"; import type { GitHubRepo, GitHubRuleset, RulesetSummary } from "../src/github/types.js"; import { @@ -94,6 +95,30 @@ describe("applyDefaults", () => { "createRepoRuleset:scratch" ]); }); + + it("reports apply progress for changed repositories only", async () => { + const calls: string[] = []; + const progress: string[] = []; + const client = fakeClient({ + calls, + repo: baseRepo(), + rulesets: [] + }); + + await applyPlannedDefaults(client, "dutifuldev", [cleanPlan(), settingsPlan(), rulesetPlan()], { + progress: { + applyingRepos: (state) => { + progress.push(`start:${String(state.total)}`); + }, + appliedRepo: (state) => { + progress.push(`${String(state.completed)}:${state.current}`); + } + } + }); + + expect(calls).toEqual(["updateRepoDefaults:settings", "createRepoRuleset:ruleset"]); + expect(progress).toEqual(["start:2", "1:dutifuldev/settings", "2:dutifuldev/ruleset"]); + }); }); type FakeClientOptions = { @@ -154,3 +179,33 @@ function baseRepo(): GitHubRepo { default_branch: "main" }; } + +function cleanPlan(): RepoPlan { + return { + name: "clean", + fullName: "dutifuldev/clean", + archived: false, + settingChanges: [], + ruleset: { action: "none" } + }; +} + +function settingsPlan(): RepoPlan { + return { + name: "settings", + fullName: "dutifuldev/settings", + archived: false, + settingChanges: [{ key: "allow_auto_merge", current: false, desired: true }], + ruleset: { action: "none" } + }; +} + +function rulesetPlan(): RepoPlan { + return { + name: "ruleset", + fullName: "dutifuldev/ruleset", + archived: false, + settingChanges: [], + ruleset: { action: "create" } + }; +} diff --git a/tests/args.test.ts b/tests/args.test.ts index e13e08a..4cffd5b 100644 --- a/tests/args.test.ts +++ b/tests/args.test.ts @@ -54,6 +54,15 @@ describe("parseArgs", () => { expect(parseArgs(["apply", "dutifuldev/scratch", "-y"])).toMatchObject({ yes: true }); }); + it("parses progress flags", () => { + expect(parseArgs(["plan", "dutifuldev/scratch", "--progress"])).toMatchObject({ + progress: "always" + }); + expect(parseArgs(["plan", "dutifuldev/scratch", "--no-progress"])).toMatchObject({ + progress: "never" + }); + }); + it("rejects commands without target repositories", () => { expect(() => parseArgs(["plan", "--org", "dutifuldev"])).toThrow( "Pass at least one --repo value or --all." diff --git a/tests/client.test.ts b/tests/client.test.ts index e1abe3d..2cff084 100644 --- a/tests/client.test.ts +++ b/tests/client.test.ts @@ -9,6 +9,7 @@ afterEach(() => { describe("RestGitHubClient.listOwnerRepos", () => { it("lists organization repositories through the organization endpoint", async () => { + const progress: string[] = []; const requests = stubFetch( new Map([ ["https://api.github.com/users/dutifuldev", { type: "Organization" }], @@ -33,10 +34,18 @@ describe("RestGitHubClient.listOwnerRepos", () => { ]) ); - await expect(new RestGitHubClient("token").listOwnerRepos("dutifuldev")).resolves.toEqual([ - repo("dutifuldev", "scratch"), - repo("dutifuldev", "tools") - ]); + await expect( + new RestGitHubClient("token").listOwnerRepos("dutifuldev", { + progress: { + loadingRepoDetails: (state) => { + progress.push(`start:${String(state.total)}`); + }, + loadedRepoDetails: (state) => { + progress.push(`${String(state.completed)}/${String(state.total)}:${state.current}`); + } + } + }) + ).resolves.toEqual([repo("dutifuldev", "scratch"), repo("dutifuldev", "tools")]); expect(requests).toEqual([ "https://api.github.com/users/dutifuldev", "https://api.github.com/orgs/dutifuldev/repos?type=all&per_page=100", @@ -44,6 +53,11 @@ describe("RestGitHubClient.listOwnerRepos", () => { "https://api.github.com/repos/dutifuldev/scratch", "https://api.github.com/repos/dutifuldev/tools" ]); + expect(progress[0]).toBe("start:2"); + expect(progress.at(-1)?.startsWith("2/2:")).toBe(true); + expect(new Set(progress.slice(1).map((item) => item.replace(/^\d+\/\d+:/, "")))).toEqual( + new Set(["dutifuldev/scratch", "dutifuldev/tools"]) + ); }); it("lists user-owned repositories through the authenticated user endpoint", async () => { diff --git a/tests/concurrency.test.ts b/tests/concurrency.test.ts new file mode 100644 index 0000000..dbdba14 --- /dev/null +++ b/tests/concurrency.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; + +import { mapWithConcurrency } from "../src/shared/concurrency.js"; + +describe("mapWithConcurrency", () => { + it("stops assigning new work after the first failure and awaits active workers", async () => { + const started: number[] = []; + let slowWorkerFinished = false; + const failure = new Error("request failed"); + + await expect( + mapWithConcurrency([0, 1, 2, 3], 2, async (item) => { + started.push(item); + + if (item === 0) { + await delay(5); + throw failure; + } + + if (item === 1) { + await delay(20); + slowWorkerFinished = true; + return item; + } + + throw new Error(`Unexpected work item: ${String(item)}`); + }) + ).rejects.toBe(failure); + + expect(started).toEqual([0, 1]); + expect(slowWorkerFinished).toBe(true); + }); +}); + +async function delay(milliseconds: number): Promise { + await new Promise((resolve) => { + setTimeout(resolve, milliseconds); + }); +} diff --git a/tests/planner.test.ts b/tests/planner.test.ts index 8ac9849..17f8c11 100644 --- a/tests/planner.test.ts +++ b/tests/planner.test.ts @@ -75,6 +75,44 @@ describe("buildPlan", () => { ).resolves.toMatchObject([{ ruleset: { action: "update" } }]); }); + it("plans repositories concurrently while preserving repository order", async () => { + const repos = [baseRepo("a"), baseRepo("b"), baseRepo("c")]; + let activeCalls = 0; + let maxActiveCalls = 0; + const completed: string[] = []; + const progressCompleted: number[] = []; + const client = fakeClient({ + repo: repos[0] ?? baseRepo("missing"), + repos, + rulesets: [], + onListRepoRulesets: async (repo) => { + activeCalls += 1; + maxActiveCalls = Math.max(maxActiveCalls, activeCalls); + await delay(repo === "a" ? 20 : 5); + completed.push(repo); + activeCalls -= 1; + } + }); + + const plans = await buildPlan( + client, + { owner: "dutifuldev", repos: [], all: true }, + { + concurrency: 2, + progress: { + plannedRepo: (state) => { + progressCompleted.push(state.completed); + } + } + } + ); + + expect(plans.map((plan) => plan.name)).toEqual(["a", "b", "c"]); + expect(completed[0]).not.toBe("a"); + expect(maxActiveCalls).toBe(2); + expect(progressCompleted).toEqual([1, 2, 3]); + }); + it("plans no ruleset change when payload already matches", () => { const desired = desiredRulesetPayload(); const existing: GitHubRuleset = { id: 1, ...desired }; @@ -109,7 +147,9 @@ describe("buildPlan", () => { }); type FakeClientOptions = { + onListRepoRulesets?: (repo: string) => Promise; repo: GitHubRepo; + repos?: GitHubRepo[]; rulesets: RulesetSummary[]; ruleset?: GitHubRuleset; }; @@ -117,9 +157,12 @@ type FakeClientOptions = { function fakeClient(options: FakeClientOptions): GitHubClient { return { getRepo: () => Promise.resolve(options.repo), - listOwnerRepos: () => Promise.resolve([options.repo]), + listOwnerRepos: () => Promise.resolve(options.repos ?? [options.repo]), updateRepoDefaults: () => Promise.resolve(), - listRepoRulesets: () => Promise.resolve(options.rulesets), + listRepoRulesets: async (_owner: string, repo: string) => { + await options.onListRepoRulesets?.(repo); + return options.rulesets; + }, getRepoRuleset: () => { if (options.ruleset === undefined) { return Promise.reject(new Error("ruleset not found")); @@ -132,11 +175,17 @@ function fakeClient(options: FakeClientOptions): GitHubClient { }; } -function baseRepo(): GitHubRepo { +async function delay(milliseconds: number): Promise { + await new Promise((resolve) => { + setTimeout(resolve, milliseconds); + }); +} + +function baseRepo(name = "scratch"): GitHubRepo { return { ...DESIRED_REPO_SETTINGS, - name: "scratch", - full_name: "dutifuldev/scratch", + name, + full_name: `dutifuldev/${name}`, archived: false, disabled: false, default_branch: "main" diff --git a/tests/progress.test.ts b/tests/progress.test.ts new file mode 100644 index 0000000..204276c --- /dev/null +++ b/tests/progress.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from "vitest"; + +import { + createProgressReporter, + type ProgressEnvironment, + type ProgressStream +} from "../src/cli/progress.js"; + +describe("createProgressReporter", () => { + it("does not write progress for non-tty auto mode", () => { + const stream = fakeStream({ isTTY: false }); + const progress = createProgressReporter("auto", stream, {}); + + progress.loadingRepos?.("osolmaz"); + progress.selectedRepos?.({ owner: "osolmaz", total: 1, skipped: 0 }); + progress.plannedRepo?.({ + owner: "osolmaz", + total: 1, + completed: 1, + changed: 1, + clean: 0, + skipped: 0, + current: "osolmaz/tools" + }); + progress.stop(); + + expect(stream.chunks).toEqual([]); + }); + + it("renders and clears a single-line status bar", () => { + const stream = fakeStream({ columns: 120, isTTY: true }); + const progress = createProgressReporter("auto", stream, {}); + + progress.loadingRepos?.("osolmaz"); + progress.loadingRepoDetails?.({ owner: "osolmaz", total: 2 }); + progress.loadedRepoDetails?.({ + owner: "osolmaz", + total: 2, + completed: 1, + current: "osolmaz/tools" + }); + progress.selectedRepos?.({ owner: "osolmaz", total: 2, skipped: 1 }); + progress.plannedRepo?.({ + owner: "osolmaz", + total: 2, + completed: 1, + changed: 1, + clean: 0, + skipped: 1, + current: "osolmaz/tools" + }); + progress.applyingRepos?.({ owner: "osolmaz", total: 1 }); + progress.appliedRepo?.({ + owner: "osolmaz", + total: 1, + completed: 1, + current: "osolmaz/tools" + }); + progress.stop(); + + expect(stream.chunks.join("")).toContain("Loading repositories for osolmaz"); + expect(stream.chunks.join("")).toContain("Loading repository details for osolmaz"); + expect(stream.chunks.join("")).toContain("Planning osolmaz"); + expect(stream.chunks.join("")).toContain("Applying osolmaz"); + expect(stream.chunks.join("")).toContain("[##########----------]"); + expect(stream.chunks.join("")).toContain("[####################]"); + expect(stream.chunks.join("")).toContain("1/2"); + expect(stream.chunks.at(-1)).toBe("\r\x1b[K"); + }); + + it("can force progress in non-tty output and disables progress in CI", () => { + const forcedStream = fakeStream({ isTTY: false }); + const ciStream = fakeStream({ isTTY: true }); + const ciEnvironment: ProgressEnvironment = { CI: "true" }; + + createProgressReporter("always", forcedStream, {}).loadingRepos?.("osolmaz"); + createProgressReporter("auto", ciStream, ciEnvironment).loadingRepos?.("osolmaz"); + + expect(forcedStream.chunks.join("")).toContain("Loading repositories"); + expect(ciStream.chunks).toEqual([]); + }); +}); + +type FakeStream = ProgressStream & { + chunks: string[]; +}; + +function fakeStream(options: { columns?: number; isTTY: boolean }): FakeStream { + const chunks: string[] = []; + const stream: FakeStream = { + isTTY: options.isTTY, + chunks, + write: (chunk: string) => { + chunks.push(chunk); + return true; + } + }; + + if (options.columns !== undefined) { + stream.columns = options.columns; + } + + return stream; +}