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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 198 additions & 0 deletions src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import {
getGitTagPrefix,
loadConfigurationFromString,
validateConfiguration,
setActiveWorkspace,
getActiveWorkspace,
getVersioningPolicy,
WORKSPACES_MIN_VERSION,
} from '../config';
import { CraftProjectConfigSchema } from '../schemas/project_config';
import { logger } from '../logger';
Expand Down Expand Up @@ -122,6 +126,36 @@ describe('noMerge config', () => {
test('fails with invalid noMerge type', () => {
expect(() => validateConfiguration({ noMerge: 'yes' })).toThrow(/noMerge/);
});

test('parses configuration with workspaces', () => {
const data = {
minVersion: '2.27.0',
github: { owner: 'getsentry', repo: 'toolkit' },
workspaces: {
cli: {
releaseBranchPrefix: 'release/cli',
github: { projectPath: 'cli' },
targets: [{ name: 'github', tagPrefix: 'cli@' }],
},
mcp: {
targets: [{ name: 'github', tagPrefix: 'mcp@' }],
},
},
};

expect(validateConfiguration(data)).toEqual(data);
});

test('allows a workspace github override without owner/repo', () => {
const data = {
workspaces: {
cli: { github: { projectPath: 'cli' } },
},
};

// Workspace github is partial; owner/repo are inherited, not required here.
expect(() => validateConfiguration(data)).not.toThrow();
});
});

describe('getGitTagPrefix', () => {
Expand Down Expand Up @@ -190,3 +224,167 @@ describe('getGitTagPrefix', () => {
expect(warnSpy).toHaveBeenCalledTimes(1);
});
});

describe('workspaces', () => {
afterEach(() => {
setActiveWorkspace(undefined);
vi.restoreAllMocks();
});

const WS_CONFIG = [
`minVersion: ${WORKSPACES_MIN_VERSION}`,
'github:',
' owner: getsentry',
' repo: toolkit',
'changelog: CHANGELOG.md',
'workspaces:',
' cli:',
' releaseBranchPrefix: release/cli',
' github:',
' projectPath: cli',
' targets:',
' - name: github',
' tagPrefix: "cli@"',
' mcp:',
' releaseBranchPrefix: release/mcp',
' versioning:',
' policy: calver',
' targets:',
' - name: github',
' tagPrefix: "mcp@"',
].join('\n');

test('backward compatible: no workspaces, no selection resolves normally', () => {
setActiveWorkspace(undefined);
loadConfigurationFromString(
['github:', ' owner: getsentry', ' repo: craft'].join('\n'),
);
expect(getActiveWorkspace()).toBeUndefined();
});

test('resolves the selected workspace: overrides win, base inherited', () => {
setActiveWorkspace('cli');
const config = loadConfigurationFromString(WS_CONFIG);

// Overridden by the workspace.
expect(config.releaseBranchPrefix).toBe('release/cli');
expect(getGitTagPrefix()).toBe('cli@');
// github is shallow-merged: owner/repo inherited, projectPath overridden.
expect(config.github).toEqual({
owner: 'getsentry',
repo: 'toolkit',
projectPath: 'cli',
});
// Inherited from the top level.
expect(config.changelog).toBe('CHANGELOG.md');
// `workspaces` is stripped from the resolved config.
expect(config.workspaces).toBeUndefined();
});

test('a different workspace resolves independently', () => {
setActiveWorkspace('mcp');
const config = loadConfigurationFromString(WS_CONFIG);
expect(config.releaseBranchPrefix).toBe('release/mcp');
expect(getGitTagPrefix()).toBe('mcp@');
expect(getVersioningPolicy()).toBe('calver');
// mcp did not override github.projectPath, so it inherits base github only.
expect(config.github).toEqual({ owner: 'getsentry', repo: 'toolkit' });
});

test('errors when workspaces are defined but none is selected', () => {
setActiveWorkspace(undefined);
expect(() => loadConfigurationFromString(WS_CONFIG)).toThrow(
/defines workspaces; select one/,
);
});

test('errors on an unknown workspace name', () => {
setActiveWorkspace('nope');
expect(() => loadConfigurationFromString(WS_CONFIG)).toThrow(
/Unknown workspace "nope"/,
);
});

test('errors when a workspace is selected but none are defined', () => {
setActiveWorkspace('cli');
expect(() =>
loadConfigurationFromString(
['github:', ' owner: getsentry', ' repo: craft'].join('\n'),
),
).toThrow(/no "workspaces" are defined/);
});

test('errors when minVersion is below the workspaces gate', () => {
setActiveWorkspace('cli');
const belowGate = WS_CONFIG.replace(
`minVersion: ${WORKSPACES_MIN_VERSION}`,
'minVersion: 2.14.0',
);
expect(() => loadConfigurationFromString(belowGate)).toThrow(
new RegExp(`requires minVersion >= ${WORKSPACES_MIN_VERSION}`),
);
});

test('setActiveWorkspace re-resolves against a new selection', () => {
setActiveWorkspace('cli');
loadConfigurationFromString(WS_CONFIG);
expect(getGitTagPrefix()).toBe('cli@');

setActiveWorkspace('mcp');
loadConfigurationFromString(WS_CONFIG);
expect(getGitTagPrefix()).toBe('mcp@');
});

test('resolved config exposes the workspace targets (publish builder contract)', () => {
// Regression for the parse-time interaction: the `publish` builder reads
// getConfiguration().targets to compute --target choices. With a workspace
// selected up front, this must resolve to that workspace's targets and must
// not throw the "select a workspace" error.
setActiveWorkspace('cli');
const config = loadConfigurationFromString(WS_CONFIG);
expect(config.targets).toEqual([{ name: 'github', tagPrefix: 'cli@' }]);
});

test('does not produce an incomplete github when base has none', () => {
// A workspace that sets only github.projectPath, with NO top-level github,
// must NOT yield a truthy-but-incomplete github object (missing owner/repo)
// — that would make getGlobalGitHubConfig skip its git-remote fallback.
setActiveWorkspace('cli');
const config = loadConfigurationFromString(
[
`minVersion: ${WORKSPACES_MIN_VERSION}`,
'workspaces:',
' cli:',
' github:',
' projectPath: cli',
' targets:',
' - name: github',
' tagPrefix: "cli@"',
].join('\n'),
);
// Incomplete github is dropped so git-remote detection can still run.
expect(config.github).toBeUndefined();
});

test('keeps github when workspace override completes owner/repo', () => {
setActiveWorkspace('cli');
const config = loadConfigurationFromString(
[
`minVersion: ${WORKSPACES_MIN_VERSION}`,
'workspaces:',
' cli:',
' github:',
' owner: getsentry',
' repo: toolkit',
' projectPath: cli',
' targets:',
' - name: github',
].join('\n'),
);
expect(config.github).toEqual({
owner: 'getsentry',
repo: 'toolkit',
projectPath: 'cli',
});
});
});
21 changes: 16 additions & 5 deletions src/commands/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,22 @@
export const description = '🛫 Publish artifacts';

export const builder: CommandBuilder = (yargs: Argv) => {
const definedTargets = getConfiguration().targets || [];
const possibleTargetNames = new Set(getAllTargetNames());
const allowedTargetNames = definedTargets
.filter(target => target.name && possibleTargetNames.has(target.name))
.map(BaseTarget.getId);
// Compute the allowed --target choices from the (workspace-resolved) config.
// The active workspace is selected before parsing (see index.ts), so this
// reflects the selected workspace's targets. If the config can't be resolved
// at parse time (e.g. missing/invalid file, or a workspaces config with no
// selection yet during shell completion), fall back to all known target
// names rather than aborting argument parsing.
let allowedTargetNames: string[];
try {
const definedTargets = getConfiguration().targets || [];
const possibleTargetNames = new Set(getAllTargetNames());
allowedTargetNames = definedTargets
.filter(target => target.name && possibleTargetNames.has(target.name))
.map(BaseTarget.getId);
} catch {
allowedTargetNames = getAllTargetNames();
}

return yargs
.positional('NEW-VERSION', {
Expand Down Expand Up @@ -438,7 +449,7 @@
// Pull --rebase failure can leave the repo in an active rebase state
try {
await git.raw(['rebase', '--abort']);
} catch (_abortError) {

Check warning on line 452 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_abortError' is defined but never used.
logger.trace('git rebase --abort failed (may be no rebase in progress)');
}
throw pullError;
Expand All @@ -455,7 +466,7 @@
);
try {
await git.merge(['--abort']);
} catch (_abortError) {

Check warning on line 469 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_abortError' is defined but never used.
// merge --abort can fail if no merge in progress (e.g. pull failed)
logger.trace('git merge --abort failed (may be no merge in progress)');
}
Expand All @@ -471,19 +482,19 @@
try {
const status = await git.status();
conflictedFiles = status.conflicted;
} catch (_statusError) {

Check warning on line 485 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_statusError' is defined but never used.
logger.trace('git status failed while collecting conflict info');
}
if (conflictedFiles.length > 0) {
try {
conflictDiff = await git.diff(conflictedFiles);
} catch (_diffError) {

Check warning on line 491 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_diffError' is defined but never used.
logger.trace('git diff failed while collecting conflict diff');
}
}
try {
await git.merge(['--abort']);
} catch (_abortError) {

Check warning on line 497 in src/commands/publish.ts

View workflow job for this annotation

GitHub Actions / Lint fixes

[@typescript-eslint/no-unused-vars] '_abortError' is defined but never used.
logger.trace('git merge --abort failed after resolve strategy');
}
throw new MergeConflictError(
Expand Down
Loading
Loading