Skip to content
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.

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

Fix issues flagged by lint rules #1017

Merged
merged 1 commit into from
Nov 25, 2024
Merged
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
7 changes: 7 additions & 0 deletions change/beachball-e300f8e4-e881-44c4-b266-4150412232fe.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix issues found by lint rules",
"packageName": "beachball",
"email": "[email protected]",
"dependentChangeType": "patch"
}
17 changes: 9 additions & 8 deletions src/__e2e__/bump.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { getPackageInfos } from '../monorepo/getPackageInfos';
import { BeachballOptions, HooksOptions } from '../types/BeachballOptions';
import type { Repository } from '../__fixtures__/repository';
import { getDefaultOptions } from '../options/getDefaultOptions';
import type { PackageJson } from '../types/PackageInfo';

describe('version bumping', () => {
let repositoryFactory: RepositoryFactory | undefined;
Expand Down Expand Up @@ -277,7 +278,7 @@ describe('version bumping', () => {

it('should not bump out-of-scope package even if package has change', async () => {
repositoryFactory = new RepositoryFactory('monorepo');
const monorepo = repositoryFactory.fixture.folders!;
const monorepo = repositoryFactory.fixture.folders;
repo = repositoryFactory.cloneRepository();

const options = getOptions({
Expand All @@ -299,7 +300,7 @@ describe('version bumping', () => {

it('should not bump out-of-scope package and its dependencies even if dependency of the package has change', async () => {
repositoryFactory = new RepositoryFactory('monorepo');
const monorepo = repositoryFactory.fixture.folders!;
const monorepo = repositoryFactory.fixture.folders;
repo = repositoryFactory.cloneRepository();

const options = getOptions({
Expand Down Expand Up @@ -622,7 +623,7 @@ describe('version bumping', () => {
expect(version).toBe('1.1.0');

const jsonPath = path.join(packagePath, 'package.json');
expect(fs.readJSONSync(jsonPath).version).toBe('1.0.0');
expect((fs.readJSONSync(jsonPath) as PackageJson).version).toBe('1.0.0');
}),
},
});
Expand Down Expand Up @@ -652,7 +653,7 @@ describe('version bumping', () => {
expect(version).toBe('1.1.0');

const jsonPath = path.join(packagePath, 'package.json');
expect((await fs.readJSON(jsonPath)).version).toBe('1.0.0');
expect(((await fs.readJSON(jsonPath)) as PackageJson).version).toBe('1.0.0');
}),
},
});
Expand All @@ -677,7 +678,7 @@ describe('version bumping', () => {
path: repo.rootPath,
bumpDeps: false,
hooks: {
prebump: async (_packagePath, _name, _version): Promise<void> => {
prebump: (): Promise<void> => {
throw new Error('Foo');
},
},
Expand Down Expand Up @@ -708,7 +709,7 @@ describe('version bumping', () => {
expect(version).toBe('1.1.0');

const jsonPath = path.join(packagePath, 'package.json');
expect(fs.readJSONSync(jsonPath).version).toBe('1.1.0');
expect((fs.readJSONSync(jsonPath) as PackageJson).version).toBe('1.1.0');
}),
},
});
Expand Down Expand Up @@ -738,7 +739,7 @@ describe('version bumping', () => {
expect(version).toBe('1.1.0');

const jsonPath = path.join(packagePath, 'package.json');
expect((await fs.readJSON(jsonPath)).version).toBe('1.1.0');
expect(((await fs.readJSON(jsonPath)) as PackageJson).version).toBe('1.1.0');
}),
},
});
Expand All @@ -762,7 +763,7 @@ describe('version bumping', () => {
const options = getOptions({
bumpDeps: false,
hooks: {
postbump: async (_packagePath, _name, _version): Promise<void> => {
postbump: (): Promise<void> => {
throw new Error('Foo');
},
},
Expand Down
16 changes: 9 additions & 7 deletions src/__e2e__/change.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { BeachballOptions } from '../types/BeachballOptions';
import { defaultBranchName } from '../__fixtures__/gitDefaults';
import { MockStdout } from '../__fixtures__/mockStdout';
import { MockStdin } from '../__fixtures__/mockStdin';
import { ChangeFileInfo } from '../types/ChangeInfo';
import type { ChangeFileInfo, ChangeInfoMultiple } from '../types/ChangeInfo';
import { Repository } from '../__fixtures__/repository';
import { getDefaultOptions } from '../options/getDefaultOptions';

Expand All @@ -23,7 +23,7 @@ jest.mock(
((questions, options) => {
questions = Array.isArray(questions) ? questions : [questions];
questions = questions.map(q => ({ ...q, stdin, stdout }));
return (jest.requireActual('prompts') as typeof prompts)(questions, options);
return jest.requireActual<typeof prompts>('prompts')(questions, options);
}) as typeof prompts
);

Expand All @@ -35,7 +35,9 @@ jest.mock(
let mockBeachballOptions: Partial<BeachballOptions> | undefined;
jest.mock('../options/getDefaultOptions', () => ({
getDefaultOptions: () => ({
...(jest.requireActual('../options/getDefaultOptions') as any).getDefaultOptions(),
...jest
.requireActual<typeof import('../options/getDefaultOptions')>('../options/getDefaultOptions')
.getDefaultOptions(),
...mockBeachballOptions,
}),
}));
Expand Down Expand Up @@ -192,7 +194,7 @@ describe('change command', () => {
repo = repositoryFactory.cloneRepository();

const options = getOptions({
package: repositoryFactory.fixture.rootPackage!.name,
package: repositoryFactory.fixture.rootPackage.name,
commit: false,
});
const changePromise = change(options);
Expand Down Expand Up @@ -240,7 +242,7 @@ describe('change command', () => {

const changeFiles = getChangeFiles(options);
expect(changeFiles).toHaveLength(2);
const changeFileContents = changeFiles.map(changeFile => fs.readJSONSync(changeFile)) as ChangeFileInfo[];
const changeFileContents = changeFiles.map(changeFile => fs.readJSONSync(changeFile) as ChangeFileInfo);
expect(changeFileContents).toContainEqual(
expect.objectContaining({ comment: 'custom', packageName: 'pkg-1', type: 'minor' })
);
Expand Down Expand Up @@ -276,7 +278,7 @@ describe('change command', () => {

const changeFiles = getChangeFiles(options);
expect(changeFiles).toHaveLength(1);
const contents = fs.readJSONSync(changeFiles[0]);
const contents = fs.readJSONSync(changeFiles[0]) as ChangeInfoMultiple;
expect(contents.changes).toEqual([
expect.objectContaining({ comment: 'custom', packageName: 'pkg-1', type: 'minor' }),
expect.objectContaining({ comment: 'commit 2', packageName: 'pkg-2', type: 'patch' }),
Expand Down Expand Up @@ -323,7 +325,7 @@ describe('change command', () => {

const changeFiles = getChangeFiles(options);
expect(changeFiles).toHaveLength(1);
const contents = fs.readJSONSync(changeFiles[0]);
const contents = fs.readJSONSync(changeFiles[0]) as ChangeInfoMultiple;
expect(contents.changes).toEqual([
expect.objectContaining({ packageName: 'pkg-1', type: 'patch', comment: 'commit 2' }),
expect.objectContaining({ packageName: 'pkg-2', type: 'patch', comment: 'commit 2', custom: 'stuff' }),
Expand Down
46 changes: 26 additions & 20 deletions src/__e2e__/publishE2E.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { publish } from '../commands/publish';
import { getDefaultOptions } from '../options/getDefaultOptions';
import { BeachballOptions } from '../types/BeachballOptions';
import { _mockNpmPublish, initNpmMock } from '../__fixtures__/mockNpm';
import type { PackageJson } from '../types/PackageInfo';

// 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 @@ -109,7 +110,7 @@ describe('publish command (e2e)', () => {
// Adds a step that injects a race condition
let fetchCount = 0;

addGitObserver((args, output) => {
addGitObserver(args => {
if (args[0] === 'fetch') {
if (fetchCount === 0) {
const anotherRepo = repositoryFactory!.cloneRepository();
Expand Down Expand Up @@ -150,14 +151,14 @@ describe('publish command (e2e)', () => {
// Adds a step that injects a race condition
let fetchCount = 0;

addGitObserver((args, output) => {
addGitObserver(args => {
if (args[0] === 'fetch') {
if (fetchCount === 0) {
const anotherRepo = repositoryFactory!.cloneRepository();
// inject a checkin
const packageJsonFile = anotherRepo.pathTo('package.json');
const contents = fs.readJSONSync(packageJsonFile, 'utf-8');
delete contents.dependencies.baz;
const contents = fs.readJSONSync(packageJsonFile, 'utf-8') as PackageJson;
delete contents.dependencies?.baz;
anotherRepo.commitChange('package.json', JSON.stringify(contents, null, 2));
anotherRepo.push();
}
Expand All @@ -182,8 +183,8 @@ describe('publish command (e2e)', () => {
expect(fetchCount).toBe(2);

const packageJsonFile = repo.pathTo('package.json');
const contents = JSON.parse(fs.readFileSync(packageJsonFile, 'utf-8'));
expect(contents.dependencies.baz).toBeUndefined();
const contents = JSON.parse(fs.readFileSync(packageJsonFile, 'utf-8')) as PackageJson;
expect(contents.dependencies?.baz).toBeUndefined();
});

it('can perform a successful npm publish without bump', async () => {
Expand Down Expand Up @@ -241,8 +242,8 @@ describe('publish command (e2e)', () => {

// bump baz => dependent bump bar => dependent bump foo
generateChangeFiles(['baz'], options);
expect(repositoryFactory.fixture.folders!.packages.foo.dependencies!.bar).toBeTruthy();
expect(repositoryFactory.fixture.folders!.packages.bar.dependencies!.baz).toBeTruthy();
expect(repositoryFactory.fixture.folders.packages.foo.dependencies!.bar).toBeTruthy();
expect(repositoryFactory.fixture.folders.packages.bar.dependencies!.baz).toBeTruthy();
repo.push();

await publish(options);
Expand Down Expand Up @@ -329,11 +330,13 @@ describe('publish command (e2e)', () => {
repositoryFactory = new RepositoryFactory('monorepo');
repo = repositoryFactory.cloneRepository();

type ExtraPackageJson = PackageJson & { onPublish?: { main: string } };

const options = getOptions({
hooks: {
prepublish: (packagePath: string) => {
const packageJsonPath = path.join(packagePath, 'package.json');
const packageJson = fs.readJSONSync(packageJsonPath);
const packageJson = fs.readJSONSync(packageJsonPath) as ExtraPackageJson;
if (packageJson.onPublish) {
Object.assign(packageJson, packageJson.onPublish);
delete packageJson.onPublish;
Expand All @@ -352,28 +355,30 @@ describe('publish command (e2e)', () => {
const show = (await npmShow('foo'))!;
expect(show.name).toEqual('foo');
expect(show.main).toEqual('lib/index.js');
expect(show.hasOwnProperty('onPublish')).toBeFalsy();
expect(show).not.toHaveProperty('onPublish');

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

// All git results should still have previous information
expect(repo.getCurrentTags()).toEqual(['foo_v1.1.0']);
const fooPackageJson = fs.readJSONSync(repo.pathTo('packages/foo/package.json'));
const fooPackageJson = fs.readJSONSync(repo.pathTo('packages/foo/package.json')) as ExtraPackageJson;
expect(fooPackageJson.main).toBe('src/index.ts');
expect(fooPackageJson.onPublish.main).toBe('lib/index.js');
expect(fooPackageJson.onPublish?.main).toBe('lib/index.js');
});

it('should respect postpublish hooks', async () => {
repositoryFactory = new RepositoryFactory('monorepo');
repo = repositoryFactory.cloneRepository();
let notified;

type ExtraPackageJson = PackageJson & { afterPublish?: { notify: string } };

const options = getOptions({
hooks: {
postpublish: packagePath => {
const packageJsonPath = path.join(packagePath, 'package.json');
const packageJson = fs.readJSONSync(packageJsonPath);
const packageJson = fs.readJSONSync(packageJsonPath) as ExtraPackageJson;
if (packageJson.afterPublish) {
notified = packageJson.afterPublish.notify;
}
Expand All @@ -386,9 +391,9 @@ describe('publish command (e2e)', () => {

await publish(options);

const fooPackageJson = fs.readJSONSync(repo.pathTo('packages/foo/package.json'));
const fooPackageJson = fs.readJSONSync(repo.pathTo('packages/foo/package.json')) as ExtraPackageJson;
expect(fooPackageJson.main).toBe('src/index.ts');
expect(notified).toBe(fooPackageJson.afterPublish.notify);
expect(notified).toBe(fooPackageJson.afterPublish?.notify);
});

it('can perform a successful npm publish without fetch', async () => {
Expand All @@ -404,7 +409,7 @@ describe('publish command (e2e)', () => {

let fetchCount = 0;

addGitObserver((args, output) => {
addGitObserver(args => {
if (args[0] === 'fetch') {
fetchCount++;
}
Expand Down Expand Up @@ -436,7 +441,7 @@ describe('publish command (e2e)', () => {

let fetchCommand: string = '';

addGitObserver((args, output) => {
addGitObserver(args => {
if (args[0] === 'fetch') {
fetchCommand = args.join(' ');
}
Expand Down Expand Up @@ -592,7 +597,8 @@ describe('publish command (e2e)', () => {

it('should respect postpublish hook respecting the concurrency limit when publishing multiple packages concurrently', async () => {
const packagesToPublish = ['pkg1', 'pkg2', 'pkg3', 'pkg4', 'pkg5', 'pkg6', 'pkg7', 'pkg8', 'pkg9'];
const packages: { [packageName: string]: PackageJsonFixture } = {};
type ExtraPackageJsonFixture = PackageJsonFixture & { afterPublish?: { notify: string } };
const packages: { [packageName: string]: ExtraPackageJsonFixture } = {};
for (const name of packagesToPublish) {
packages[name] = {
version: '1.0.0',
Expand Down Expand Up @@ -622,7 +628,7 @@ describe('publish command (e2e)', () => {
await simulateWait(100);
const packageName = path.basename(packagePath);
const packageJsonPath = path.join(packagePath, 'package.json');
const packageJson = fs.readJSONSync(packageJsonPath);
const packageJson = fs.readJSONSync(packageJsonPath) as ExtraPackageJsonFixture;
if (packageJson.afterPublish) {
afterPublishStrings.push({
packageName,
Expand All @@ -644,7 +650,7 @@ describe('publish command (e2e)', () => {
expect(maxConcurrency).toBe(concurrency);

for (const pkg of packagesToPublish) {
const packageJson = fs.readJSONSync(repo.pathTo(`packages/${pkg}/package.json`));
const packageJson = fs.readJSONSync(repo.pathTo(`packages/${pkg}/package.json`)) as ExtraPackageJsonFixture;
if (packageJson.afterPublish) {
// Verify that all postpublish hooks were called
expect(afterPublishStrings).toContainEqual({
Expand Down
5 changes: 3 additions & 2 deletions src/__e2e__/publishGit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { BeachballOptions } from '../types/BeachballOptions';
import { ChangeFileInfo } from '../types/ChangeInfo';
import { getPackageInfos } from '../monorepo/getPackageInfos';
import { getDefaultOptions } from '../options/getDefaultOptions';
import type { PackageJson } from '../types/PackageInfo';

describe('publish command (git)', () => {
let repositoryFactory: RepositoryFactory;
Expand Down Expand Up @@ -61,7 +62,7 @@ describe('publish command (git)', () => {

const newRepo = repositoryFactory.cloneRepository();

const packageJson = fs.readJSONSync(newRepo.pathTo('package.json'));
const packageJson = fs.readJSONSync(newRepo.pathTo('package.json')) as PackageJson;

expect(packageJson.version).toBe('1.1.0');
});
Expand Down Expand Up @@ -91,7 +92,7 @@ describe('publish command (git)', () => {
const newRepo = repositoryFactory.cloneRepository();
const changeFiles = getChangeFiles({ ...options1, path: newRepo.rootPath });
expect(changeFiles).toHaveLength(1);
const changeFileContent: ChangeFileInfo = fs.readJSONSync(changeFiles[0]);
const changeFileContent = fs.readJSONSync(changeFiles[0]) as ChangeFileInfo;
expect(changeFileContent.packageName).toBe('foo2');
});
});
4 changes: 2 additions & 2 deletions src/__fixtures__/changelog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function readChangelogJson(packagePath: string, filename?: string, noClea
return null;
}

const changelog: ChangelogJson = fs.readJSONSync(changelogJsonFile, { encoding: 'utf-8' });
const changelog = fs.readJSONSync(changelogJsonFile, { encoding: 'utf-8' }) as ChangelogJson;
if (noClean) {
return changelog;
}
Expand All @@ -50,7 +50,7 @@ export function readChangelogJson(packagePath: string, filename?: string, noClea
}

for (const comments of Object.values(entry.comments)) {
for (const comment of comments!) {
for (const comment of comments) {
if (comment.commit) {
comment.commit = fakeCommit;
}
Expand Down
4 changes: 2 additions & 2 deletions src/__fixtures__/gitDefaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const defaultRemoteBranchName = 'origin/master';
* users have configured a different default branch name.
* @param isRemoteRepo Whether this is the bare repo used as the remote
*/
export function setDefaultBranchName(cwd: string, isRemoteRepo?: boolean) {
export function setDefaultBranchName(cwd: string, isRemoteRepo?: boolean): void {
if (isRemoteRepo) {
// Change the name of the default branch on the repo used for the remote
// (for clones, this is unnecessary and can cause problems)
Expand All @@ -26,6 +26,6 @@ export function setDefaultBranchName(cwd: string, isRemoteRepo?: boolean) {
* (This should NOT be used to make full snapshots of git logs, or as the primary means of testing.
* But it can be useful for allowing backup checks for absence of strings like "warning" or "fatal".)
*/
export function optsWithLang(opts?: Parameters<typeof git>[1]) {
export function optsWithLang(opts?: Parameters<typeof git>[1]): Parameters<typeof git>[1] {
return { ...opts, env: { ...opts?.env, LANG: 'en_US.UTF-8' } };
}
4 changes: 2 additions & 2 deletions src/__fixtures__/mockLogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export function initMockLogs(options: MockLogsOptions = {}): MockLogs {

const logs: MockLogs = {
mocks: {} as MockLogs['mocks'],
setOverrideOptions: options => {
overrideOptions = options;
setOverrideOptions: opt => {
overrideOptions = opt;
},
clear: () => {
allLines = [];
Expand Down
Loading