Skip to content

Skip publishing packages that aren't bumped #1126

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

Merged
ecraig12345 merged 3 commits into from
Dec 20, 2025
Merged

Skip publishing packages that aren't bumped #1126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
57d8962
Skip publishing packages that aren't bumped
ecraig12345 Dec 20, 2025
745ff00
remove bump exception
ecraig12345 Dec 20, 2025
a12984f
fix
ecraig12345 Dec 20, 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
1 change: 1 addition & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"javascript.preferences.quoteStyle": "single",
"typescript.preferences.quoteStyle": "single",
"typescript.tsdk": "node_modules/typescript/lib",
"jest.runMode": "on-demand",
"search.exclude": {
"**/node_modules": true,
"**/lib": true,
Expand Down
7 changes: 7 additions & 0 deletions change/beachball-3d12ebc5-ed7b-4929-a670-5fb04530a140.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Ensure packages that aren't being bumped (and aren't new) aren't published",
"packageName": "beachball",
"email": "elcraig@microsoft.com",
"dependentChangeType": "patch"
}
142 changes: 33 additions & 109 deletions src/__e2e__/publishE2E.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { describe, expect, it, afterEach, jest } from '@jest/globals';
import fs from 'fs';
import fsPromises from 'fs/promises';
import path from 'path';
import { addGitObserver, clearGitObservers } from 'workspace-tools';
import { generateChangeFiles, getChangeFiles } from '../__fixtures__/changeFiles';
Expand All @@ -9,14 +8,15 @@ import { initMockLogs } from '../__fixtures__/mockLogs';
import type { Repository } from '../__fixtures__/repository';
import { type PackageJsonFixture, RepositoryFactory } from '../__fixtures__/repositoryFactory';
import { publish } from '../commands/publish';
import type { RepoOptions } from '../types/BeachballOptions';
import type { ParsedOptions, RepoOptions } from '../types/BeachballOptions';
import { _mockNpmPublish, initNpmMock } from '../__fixtures__/mockNpm';
import type { PackageJson } from '../types/PackageInfo';
import { getParsedOptions } from '../options/getOptions';
import { getPackageInfos } from '../monorepo/getPackageInfos';
import { validate } from '../validation/validate';
import { readJson } from '../object/readJson';
import { createCommandContext } from '../monorepo/createCommandContext';
import { deepFreezeProperties } from '../__fixtures__/object';

// Spawning actual npm to run commands against a fake registry is extremely slow, so mock it for
// this test (packagePublish covers the more complete npm registry scenario).
Expand Down Expand Up @@ -51,6 +51,19 @@ describe('publish command (e2e)', () => {
return { options: parsedOptions.options, parsedOptions };
}

/**
* For more realistic testing, call `validate()` like the CLI command does, then call `publish()`.
* This helps catch any new issues with double bumps or context mutation.
*/
async function publishWrapper(parsedOptions: ParsedOptions) {
// This does an initial bump
const { context } = validate(parsedOptions, { checkDependencies: true });
// Ensure the later bump process does not modify the context
deepFreezeProperties(context.bumpInfo);
deepFreezeProperties(context.originalPackageInfos);
await publish(parsedOptions.options, context);
}

afterEach(() => {
clearGitObservers();

Expand All @@ -75,11 +88,7 @@ describe('publish command (e2e)', () => {
args[0] === 'fetch' && fetchCount++;
});

// For this test, run validate first to simulate what the CLI does.
// This would catch double bump issues if the validate step's bump call mutated the original PackageInfos.
const { context } = validate(parsedOptions, { checkDependencies: true });

await publish(options, context);
await publishWrapper(parsedOptions);

const publishedFoo = npmMock.getPublishedVersions('foo');
expect(publishedFoo).toEqual({
Expand Down Expand Up @@ -112,7 +121,7 @@ describe('publish command (e2e)', () => {

repo.checkout('--detach');

await publish(options, createCommandContext(parsedOptions));
await publishWrapper(parsedOptions);

expect(npmMock.getPublishedVersions('foo')).toEqual({
versions: ['1.1.0'],
Expand Down Expand Up @@ -145,7 +154,7 @@ describe('publish command (e2e)', () => {
}
});

await publish(options, createCommandContext(parsedOptions));
await publishWrapper(parsedOptions);

expect(npmMock.getPublishedVersions('foo')).toEqual({
versions: ['1.1.0'],
Expand Down Expand Up @@ -193,7 +202,7 @@ describe('publish command (e2e)', () => {
}
});

await publish(options, createCommandContext(parsedOptions));
await publishWrapper(parsedOptions);

expect(npmMock.getPublishedVersions('foo')).toEqual({
versions: ['1.1.0'],
Expand Down Expand Up @@ -221,7 +230,7 @@ describe('publish command (e2e)', () => {
generateChangeFiles(['foo'], options);
repo.push();

await publish(options, createCommandContext(parsedOptions));
await publishWrapper(parsedOptions);

expect(npmMock.getPublishedVersions('foo')).toEqual({
versions: ['1.0.0'],
Expand All @@ -236,36 +245,7 @@ describe('publish command (e2e)', () => {
expect(newPackageInfos.foo.version).toBe('1.0.0');
});

it('publishes only changed packages in a monorepo', async () => {
repositoryFactory = new RepositoryFactory('monorepo');
repo = repositoryFactory.cloneRepository();

const { options, parsedOptions } = getOptions({ fetch: false });

generateChangeFiles(['foo'], options);
repo.push();

// For this test, run validate first to simulate what the CLI does
validate(parsedOptions, { checkDependencies: true });

await publish(options, createCommandContext(parsedOptions));

expect(npmMock.getPublishedVersions('bar')).toBeUndefined();

expect(npmMock.getPublishedVersions('foo')).toEqual({
versions: ['1.1.0'],
'dist-tags': { latest: '1.1.0' },
});

repo.checkout(defaultBranchName);
repo.pull();
expect(repo.getCurrentTags()).toEqual(['foo_v1.1.0']);

const newPackageInfos = getPackageInfos(parsedOptions);
expect(newPackageInfos.foo.version).toBe('1.1.0');
});

it('publishes dependent packages in a monorepo', async () => {
it('publishes changed and dependent packages in a monorepo', async () => {
repositoryFactory = new RepositoryFactory('monorepo');
repo = repositoryFactory.cloneRepository();

Expand All @@ -280,7 +260,7 @@ describe('publish command (e2e)', () => {
// For this test, run validate first to simulate what the CLI does
validate(parsedOptions, { checkDependencies: true });

await publish(options, createCommandContext(parsedOptions));
await publishWrapper(parsedOptions);

expect(npmMock.getPublishedVersions('baz')).toEqual({ versions: ['1.4.0'], 'dist-tags': { latest: '1.4.0' } });

Expand Down Expand Up @@ -314,7 +294,7 @@ describe('publish command (e2e)', () => {
generateChangeFiles(['foo'], options);
repo.push();

await publish(options, createCommandContext(parsedOptions));
await publishWrapper(parsedOptions);

expect(npmMock.getPublishedVersions('foo')).toEqual({ versions: ['1.1.0'], 'dist-tags': { latest: '1.1.0' } });
expect(npmMock.getPublishedVersions('bar')).toEqual({ versions: ['1.3.4'], 'dist-tags': { latest: '1.3.4' } });
Expand All @@ -337,7 +317,7 @@ describe('publish command (e2e)', () => {
expect(getChangeFiles(options)).toHaveLength(2);
repo.push();

await publish(options, createCommandContext(parsedOptions));
await publishWrapper(parsedOptions);

expect(npmMock.getPublishedVersions('foo')).toBeUndefined();
expect(npmMock.getPublishedVersions('bar')).toEqual({
Expand Down Expand Up @@ -375,9 +355,13 @@ describe('publish command (e2e)', () => {
const { options, parsedOptions } = getOptions({
fetch: false,
hooks: {
prepublish: (packagePath: string) => {
prepublish: (packagePath, name, version) => {
const packageJsonPath = path.join(packagePath, 'package.json');
const packageJson = readJson<ExtraPackageJson>(packageJsonPath);
if (name === 'foo') {
expect(version).toBe('1.1.0');
expect(packageJson.version).toBe('1.1.0'); // bumped version
}
if (packageJson.customOnPublish) {
Object.assign(packageJson, packageJson.customOnPublish);
delete packageJson.customOnPublish;
Expand All @@ -397,7 +381,7 @@ describe('publish command (e2e)', () => {
generateChangeFiles(['foo'], options);
repo.push();

await publish(options, createCommandContext(parsedOptions));
await publishWrapper(parsedOptions);

// Query the information from package.json from the registry to see if it was successfully patched
const publishedFooJson = npmMock.getPublishedPackage('foo')!;
Expand All @@ -416,69 +400,6 @@ describe('publish command (e2e)', () => {
expect(notified).toBe(fooJsonPost.customAfterPublish?.notify);
});

it('specifies fetch depth when depth param is defined', async () => {
repositoryFactory = new RepositoryFactory('single');
repo = repositoryFactory.cloneRepository();

const { options, parsedOptions } = getOptions({
depth: 10,
});

generateChangeFiles(['foo'], options);
repo.push();

let fetchCommand = '';

addGitObserver(args => {
if (args[0] === 'fetch') {
fetchCommand = args.join(' ');
}
});

await publish(options, createCommandContext(parsedOptions));

expect(npmMock.getPublishedVersions('foo')).toEqual({
versions: ['1.1.0'],
'dist-tags': { latest: '1.1.0' },
});

expect(fetchCommand).toMatch('--depth=10');
});

it('calls precommit hook before committing changes', async () => {
repositoryFactory = new RepositoryFactory('monorepo');
repo = repositoryFactory.cloneRepository();

const { options, parsedOptions } = getOptions({
publish: false, // irrelevant to this test
fetch: false,
hooks: {
precommit: jest.fn(async (cwd: string) => {
expect(readJson<PackageJson>(path.join(cwd, 'packages/foo/package.json')).version).toBe('1.1.0');
const filePath = path.join(cwd, 'foo.txt');
await fsPromises.writeFile(filePath, 'foo');
}),
},
});

generateChangeFiles(['foo', 'bar'], options);
repo.push();

await publish(options, createCommandContext(parsedOptions));

// precommit was called (once for whole repo, not per package)
expect(options.hooks?.precommit).toHaveBeenCalledTimes(1);
// but changes from publish process were reverted locally
const txtPath = repo.pathTo('foo.txt');
expect(fs.existsSync(txtPath)).toBe(false);

repo.checkout(defaultBranchName);
repo.pull();

// changes from publish process were committed
expect(fs.existsSync(txtPath)).toBe(true);
});

it('respects concurrency limit when publishing multiple packages', async () => {
const packagesToPublish = ['pkg1', 'pkg2', 'pkg3', 'pkg4', 'pkg5'];
const packages: { [packageName: string]: PackageJsonFixture } = {};
Expand Down Expand Up @@ -511,6 +432,7 @@ describe('publish command (e2e)', () => {
return result;
});

// skip validate for this test since it's not relevant
await publish(options, createCommandContext(parsedOptions));
// Verify that at most `concurrency` number of packages were published concurrently
expect(maxConcurrency).toBe(concurrency);
Expand Down Expand Up @@ -556,6 +478,7 @@ describe('publish command (e2e)', () => {
return _mockNpmPublish(registryData, args, opts);
});

// skip validate for this test since it's not relevant
await expect(publish(options, createCommandContext(parsedOptions))).rejects.toThrow(
'Error publishing! Refer to the previous logs for recovery instructions.'
);
Expand Down Expand Up @@ -620,6 +543,7 @@ describe('publish command (e2e)', () => {

generateChangeFiles(packagesToPublish, options);

// skip validate for this test since it's not relevant
await publish(options, createCommandContext(parsedOptions));
// Verify that at most `concurrency` number of postpublish hooks were running concurrently
expect(maxConcurrency).toBe(concurrency);
Expand Down
Loading
⚡