From aaf5921af041ee41c06b0bac2c79661f3086e81f Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Fri, 9 Sep 2022 22:06:54 -0700 Subject: [PATCH 1/3] Respect ignorePatterns in getPackageInfos --- src/__e2e__/bump.test.ts | 26 ++++----- src/__e2e__/getChangedPackages.test.ts | 12 ++-- src/__e2e__/publishGit.test.ts | 2 +- src/__e2e__/syncE2E.test.ts | 12 ++-- src/__fixtures__/repository.ts | 11 +++- .../changefile/readChangeFiles.test.ts | 29 ++++------ .../changelog/writeChangelog.test.ts | 14 ++--- .../monorepo/getPackageInfos.test.ts | 48 +++++++++++---- .../monorepo/getScopedPackages.test.ts | 2 +- .../validation/isChangeFileNeeded.test.ts | 58 ++++++++----------- src/changefile/promptForChange.ts | 2 +- src/commands/bump.ts | 2 +- src/commands/canary.ts | 2 +- src/commands/publish.ts | 2 +- src/commands/sync.ts | 2 +- src/monorepo/getPackageInfos.ts | 47 +++++++++------ src/validation/isValidGroupOptions.ts | 7 ++- src/validation/validate.ts | 4 +- 18 files changed, 155 insertions(+), 127 deletions(-) diff --git a/src/__e2e__/bump.test.ts b/src/__e2e__/bump.test.ts index 5a8cd5f73..873894d8c 100644 --- a/src/__e2e__/bump.test.ts +++ b/src/__e2e__/bump.test.ts @@ -39,7 +39,7 @@ describe('version bumping', () => { await bump({ path: repo.rootPath, bumpDeps: false } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); const pkg1NewVersion = '1.1.0'; expect(packageInfos['pkg-1'].version).toBe(pkg1NewVersion); @@ -69,8 +69,8 @@ describe('version bumping', () => { await bump({ path: workspaceARoot, bumpDeps: true } as BeachballOptions); - const packageInfosA = getPackageInfos(workspaceARoot); - const packageInfosB = getPackageInfos(workspaceBRoot); + const packageInfosA = getPackageInfos({ path: workspaceARoot }); + const packageInfosB = getPackageInfos({ path: workspaceBRoot }); expect(packageInfosA['@workspace-a/foo'].version).toBe('1.1.0'); expect(packageInfosB['@workspace-b/foo'].version).toBe('1.0.0'); @@ -105,7 +105,7 @@ describe('version bumping', () => { fromRef: oldCommit, } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); expect(packageInfos['pkg-1'].version).toBe(monorepo['packages']['pkg-1'].version); expect(packageInfos['pkg-2'].version).toBe(monorepo['packages']['pkg-2'].version); @@ -134,7 +134,7 @@ describe('version bumping', () => { await bump({ path: repo.rootPath, bumpDeps: true } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); const pkg1NewVersion = '1.1.0'; const dependentNewVersion = '1.0.1'; @@ -173,7 +173,7 @@ describe('version bumping', () => { groups: [{ include: 'packages/*', name: 'testgroup' }], } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); const newVersion = '1.1.0'; expect(packageInfos['pkg-1'].version).toBe(newVersion); @@ -212,7 +212,7 @@ describe('version bumping', () => { bumpDeps: true, } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); const groupNewVersion = '1.1.0'; expect(packageInfos['pkg-1'].version).toBe(groupNewVersion); @@ -241,7 +241,7 @@ describe('version bumping', () => { scope: ['!packages/foo'], } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); expect(packageInfos['foo'].version).toBe(monorepo['packages']['foo'].version); expect(packageInfos['bar'].version).toBe(monorepo['packages']['bar'].version); @@ -264,7 +264,7 @@ describe('version bumping', () => { scope: ['!packages/foo'], } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); expect(packageInfos['foo'].version).toBe(monorepo['packages']['foo'].version); expect(packageInfos['bar'].version).toBe('1.3.5'); expect(packageInfos['foo'].dependencies!['bar']).toBe(monorepo['packages']['foo'].dependencies!['bar']); @@ -295,7 +295,7 @@ describe('version bumping', () => { keepChangeFiles: true, } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); const pkg1NewVersion = '1.1.0'; expect(packageInfos['pkg-1'].version).toBe(pkg1NewVersion); @@ -333,7 +333,7 @@ describe('version bumping', () => { prereleasePrefix: 'beta', } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); const newVersion = '1.0.1-beta.0'; expect(packageInfos['pkg-1'].version).toBe(newVersion); @@ -374,7 +374,7 @@ describe('version bumping', () => { prereleasePrefix: 'beta', } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); const newVersion = '1.0.1-beta.0'; expect(packageInfos['pkg-1'].version).toBe(newVersion); @@ -415,7 +415,7 @@ describe('version bumping', () => { prereleasePrefix: 'beta', } as BeachballOptions); - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos({ path: repo.rootPath }); const pkg1NewVersion = '1.0.1-beta.1'; const othersNewVersion = '1.0.1-beta.0'; diff --git a/src/__e2e__/getChangedPackages.test.ts b/src/__e2e__/getChangedPackages.test.ts index be303215d..973f7880e 100644 --- a/src/__e2e__/getChangedPackages.test.ts +++ b/src/__e2e__/getChangedPackages.test.ts @@ -19,7 +19,7 @@ describe('getChangedPackages', () => { repositoryFactory = new RepositoryFactory('single'); const repo = repositoryFactory.cloneRepository(); const options = { fetch: false, path: repo.rootPath, branch: defaultBranchName } as BeachballOptions; - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos(options); expect(getChangedPackages(options, packageInfos)).toStrictEqual([]); @@ -36,7 +36,7 @@ describe('getChangedPackages', () => { branch: defaultBranchName, ignorePatterns: ['*.test.js', 'tests/**', 'yarn.lock'], } as BeachballOptions; - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos(options); repo.stageChange('src/foo.test.js'); repo.stageChange('tests/stuff.js'); @@ -49,7 +49,7 @@ describe('getChangedPackages', () => { repositoryFactory = new RepositoryFactory('monorepo'); const repo = repositoryFactory.cloneRepository(); const options = { fetch: false, path: repo.rootPath, branch: defaultBranchName } as BeachballOptions; - const packageInfos = getPackageInfos(repo.rootPath); + const packageInfos = getPackageInfos(options); expect(getChangedPackages(options, packageInfos)).toStrictEqual([]); @@ -65,7 +65,7 @@ describe('getChangedPackages', () => { const workspaceARoot = repo.pathTo('workspace-a'); const workspaceBRoot = repo.pathTo('workspace-b'); - const rootPackageInfos = getPackageInfos(repo.rootPath); + const rootPackageInfos = getPackageInfos(rootOptions); expect(getChangedPackages(rootOptions, rootPackageInfos)).toStrictEqual([]); @@ -73,11 +73,11 @@ describe('getChangedPackages', () => { const changedPackagesA = getChangedPackages( { ...rootOptions, path: workspaceARoot }, - getPackageInfos(workspaceARoot) + getPackageInfos({ path: workspaceARoot }) ); const changedPackagesB = getChangedPackages( { ...rootOptions, path: workspaceBRoot }, - getPackageInfos(workspaceBRoot) + getPackageInfos({ path: workspaceBRoot }) ); const changedPackagesRoot = getChangedPackages(rootOptions, rootPackageInfos); diff --git a/src/__e2e__/publishGit.test.ts b/src/__e2e__/publishGit.test.ts index 3d48e1d6f..5adfeeb61 100644 --- a/src/__e2e__/publishGit.test.ts +++ b/src/__e2e__/publishGit.test.ts @@ -76,7 +76,7 @@ describe('publish command (git)', () => { const options: BeachballOptions = getOptions(repo1); - const bumpInfo = gatherBumpInfo(options, getPackageInfos(repo1.rootPath)); + const bumpInfo = gatherBumpInfo(options, getPackageInfos(options)); // 3. Meanwhile, in repo2, also create a new change file const repo2 = repositoryFactory.cloneRepository(); diff --git a/src/__e2e__/syncE2E.test.ts b/src/__e2e__/syncE2E.test.ts index 20923c208..1d83f2110 100644 --- a/src/__e2e__/syncE2E.test.ts +++ b/src/__e2e__/syncE2E.test.ts @@ -91,7 +91,7 @@ describe('sync command (e2e)', () => { }); const repo = repositoryFactory.cloneRepository(); - const packageInfosBeforeSync = getPackageInfos(repo.rootPath); + const packageInfosBeforeSync = getPackageInfos({ path: repo.rootPath }); expect((await packagePublish(packageInfosBeforeSync['foopkg'], registry.getUrl(), '', '')).success).toBeTruthy(); expect((await packagePublish(packageInfosBeforeSync['barpkg'], registry.getUrl(), '', '')).success).toBeTruthy(); @@ -104,7 +104,7 @@ describe('sync command (e2e)', () => { await sync(getOptions(repo, { tag: '' })); - const packageInfosAfterSync = getPackageInfos(repo.rootPath); + const packageInfosAfterSync = getPackageInfos({ path: repo.rootPath }); expect(packageInfosAfterSync['foopkg'].version).toEqual('1.2.0'); expect(packageInfosAfterSync['barpkg'].version).toEqual('3.0.0'); @@ -123,7 +123,7 @@ describe('sync command (e2e)', () => { }); const repo = repositoryFactory.cloneRepository(); - const packageInfosBeforeSync = getPackageInfos(repo.rootPath); + const packageInfosBeforeSync = getPackageInfos({ path: repo.rootPath }); expect((await packagePublish(packageInfosBeforeSync['apkg'], registry.getUrl(), '', '')).success).toBeTruthy(); expect((await packagePublish(packageInfosBeforeSync['bpkg'], registry.getUrl(), '', '')).success).toBeTruthy(); @@ -136,7 +136,7 @@ describe('sync command (e2e)', () => { await sync(getOptions(repo, { tag: 'beta' })); - const packageInfosAfterSync = getPackageInfos(repo.rootPath); + const packageInfosAfterSync = getPackageInfos({ path: repo.rootPath }); expect(packageInfosAfterSync['apkg'].version).toEqual('2.0.0'); expect(packageInfosAfterSync['bpkg'].version).toEqual('2.2.0'); @@ -155,7 +155,7 @@ describe('sync command (e2e)', () => { }); const repo = repositoryFactory.cloneRepository(); - const packageInfosBeforeSync = getPackageInfos(repo.rootPath); + const packageInfosBeforeSync = getPackageInfos({ path: repo.rootPath }); const epkg = packageInfosBeforeSync['epkg']; const fpkg = packageInfosBeforeSync['fpkg']; @@ -177,7 +177,7 @@ describe('sync command (e2e)', () => { await sync(getOptions(repo, { tag: 'prerelease', forceVersions: true })); - const packageInfosAfterSync = getPackageInfos(repo.rootPath); + const packageInfosAfterSync = getPackageInfos({ path: repo.rootPath }); expect(packageInfosAfterSync['epkg'].version).toEqual('1.0.0-1'); expect(packageInfosAfterSync['fpkg'].version).toEqual('2.2.0'); diff --git a/src/__fixtures__/repository.ts b/src/__fixtures__/repository.ts index 113fd9d4a..c69ab6dec 100644 --- a/src/__fixtures__/repository.ts +++ b/src/__fixtures__/repository.ts @@ -73,10 +73,10 @@ ${gitResult.stderr.toString()}`); } /** - * Create (or update) and stage a file, creating the intermediate directories if necessary. + * Create or update a file, creating the intermediate directories if necessary. * Automatically uses root path; do not pass absolute paths here. */ - stageChange(newFilename: string, content?: string) { + writeChange(newFilename: string, content?: string) { const filePath = this.pathTo(newFilename); fs.ensureDirSync(path.dirname(filePath)); fs.ensureFileSync(filePath); @@ -84,7 +84,14 @@ ${gitResult.stderr.toString()}`); if (content) { fs.writeFileSync(filePath, content); } + } + /** + * Create (or update) and stage a file, creating the intermediate directories if necessary. + * Automatically uses root path; do not pass absolute paths here. + */ + stageChange(newFilename: string, content?: string) { + this.writeChange(newFilename, content); this.git(['add', newFilename]); } diff --git a/src/__functional__/changefile/readChangeFiles.test.ts b/src/__functional__/changefile/readChangeFiles.test.ts index ddbea7272..564a48ef0 100644 --- a/src/__functional__/changefile/readChangeFiles.test.ts +++ b/src/__functional__/changefile/readChangeFiles.test.ts @@ -32,8 +32,8 @@ describe('readChangeFiles', () => { repository.commitChange('foo'); generateChangeFiles(['foo'], repository.rootPath); - const packageInfos = getPackageInfos(repository.rootPath); - const changeSet = readChangeFiles({ path: repository.rootPath } as BeachballOptions, packageInfos); + const options = { path: repository.rootPath } as BeachballOptions; + const changeSet = readChangeFiles(options, getPackageInfos(options)); expect(changeSet).toHaveLength(1); expect(changeSet[0].change.commit).toBe(undefined); }); @@ -44,8 +44,8 @@ describe('readChangeFiles', () => { // fake doesn't exist, bar is private, foo is okay generateChangeFiles(['fake', 'bar', 'foo'], monoRepo.rootPath); - const packageInfos = getPackageInfos(monoRepo.rootPath); - const changeSet = readChangeFiles({ path: monoRepo.rootPath } as BeachballOptions, packageInfos); + const options = { path: monoRepo.rootPath } as BeachballOptions; + const changeSet = readChangeFiles(options, getPackageInfos(options)); expect(changeSet).toHaveLength(1); expect(logs.mocks.warn).toHaveBeenCalledWith(expect.stringContaining('Change detected for private package bar')); @@ -60,11 +60,8 @@ describe('readChangeFiles', () => { // fake doesn't exist, bar is private, foo is okay generateChangeFiles(['fake', 'bar', 'foo'], monoRepo.rootPath, true /*groupChanges*/); - const packageInfos = getPackageInfos(monoRepo.rootPath); - const changeSet = readChangeFiles( - { path: monoRepo.rootPath, groupChanges: true } as BeachballOptions, - packageInfos - ); + const options = { path: monoRepo.rootPath, groupChanges: true } as BeachballOptions; + const changeSet = readChangeFiles(options, getPackageInfos(options)); expect(changeSet).toHaveLength(1); expect(logs.mocks.warn).toHaveBeenCalledWith(expect.stringContaining('Change detected for private package bar')); @@ -77,11 +74,8 @@ describe('readChangeFiles', () => { const monoRepo = monoRepoFactory.cloneRepository(); generateChangeFiles(['bar', 'foo'], monoRepo.rootPath); - const packageInfos = getPackageInfos(monoRepo.rootPath); - const changeSet = readChangeFiles( - { path: monoRepo.rootPath, scope: ['packages/foo'] } as BeachballOptions, - packageInfos - ); + const options = { path: monoRepo.rootPath, scope: ['packages/foo'] } as BeachballOptions; + const changeSet = readChangeFiles(options, getPackageInfos(options)); expect(changeSet).toHaveLength(1); expect(logs.mocks.warn).not.toHaveBeenCalled(); }); @@ -90,11 +84,8 @@ describe('readChangeFiles', () => { const monoRepo = monoRepoFactory.cloneRepository(); generateChangeFiles(['bar', 'foo'], monoRepo.rootPath, true /*groupChanges*/); - const packageInfos = getPackageInfos(monoRepo.rootPath); - const changeSet = readChangeFiles( - { path: monoRepo.rootPath, scope: ['packages/foo'], groupChanges: true } as BeachballOptions, - packageInfos - ); + const options = { path: monoRepo.rootPath, scope: ['packages/foo'], groupChanges: true } as BeachballOptions; + const changeSet = readChangeFiles(options, getPackageInfos(options)); expect(changeSet).toHaveLength(1); expect(logs.mocks.warn).not.toHaveBeenCalled(); }); diff --git a/src/__functional__/changelog/writeChangelog.test.ts b/src/__functional__/changelog/writeChangelog.test.ts index 7e6061fe0..d2805a06b 100644 --- a/src/__functional__/changelog/writeChangelog.test.ts +++ b/src/__functional__/changelog/writeChangelog.test.ts @@ -49,7 +49,7 @@ describe('writeChangelog', () => { generateChangeFiles([getChange('foo', 'comment 2')], repository.rootPath); const beachballOptions = { path: repository.rootPath } as BeachballOptions; - const packageInfos = getPackageInfos(repository.rootPath); + const packageInfos = getPackageInfos(beachballOptions); const changes = readChangeFiles(beachballOptions, packageInfos); await writeChangelog(beachballOptions, changes, { foo: 'patch' }, { foo: new Set(['foo']) }, packageInfos); @@ -83,7 +83,7 @@ describe('writeChangelog', () => { generateChangeFiles([getChange('foo', 'comment 2')], ...params); const beachballOptions = { path: monoRepo.rootPath, groupChanges: true } as BeachballOptions; - const packageInfos = getPackageInfos(monoRepo.rootPath); + const packageInfos = getPackageInfos(beachballOptions); const changes = readChangeFiles(beachballOptions, packageInfos); await writeChangelog(beachballOptions, changes, { foo: 'patch', bar: 'patch' }, {}, packageInfos); @@ -127,7 +127,7 @@ describe('writeChangelog', () => { }, }; - const packageInfos = getPackageInfos(monoRepo.rootPath); + const packageInfos = getPackageInfos(beachballOptions as { path: string }); const changes = readChangeFiles(beachballOptions as BeachballOptions, packageInfos); await writeChangelog(beachballOptions as BeachballOptions, changes, {}, {}, packageInfos); @@ -158,7 +158,7 @@ describe('writeChangelog', () => { }, }; - const packageInfos = getPackageInfos(monoRepo.rootPath); + const packageInfos = getPackageInfos(beachballOptions as { path: string }); const changes = readChangeFiles(beachballOptions as BeachballOptions, packageInfos); await writeChangelog( @@ -203,7 +203,7 @@ describe('writeChangelog', () => { }, }; - const packageInfos = getPackageInfos(monoRepo.rootPath); + const packageInfos = getPackageInfos(beachballOptions as { path: string }); const changes = readChangeFiles(beachballOptions as BeachballOptions, packageInfos); await writeChangelog( @@ -243,7 +243,7 @@ describe('writeChangelog', () => { }, }; - const packageInfos = getPackageInfos(monoRepo.rootPath); + const packageInfos = getPackageInfos(beachballOptions as { path: string }); const changes = readChangeFiles(beachballOptions as BeachballOptions, packageInfos); await writeChangelog(beachballOptions as BeachballOptions, changes, {}, {}, packageInfos); @@ -286,7 +286,7 @@ describe('writeChangelog', () => { }, }; - const packageInfos = getPackageInfos(monoRepo.rootPath); + const packageInfos = getPackageInfos(beachballOptions as { path: string }); const changes = readChangeFiles(beachballOptions as BeachballOptions, packageInfos); // Verify that the comment of only the intended change file is changed diff --git a/src/__functional__/monorepo/getPackageInfos.test.ts b/src/__functional__/monorepo/getPackageInfos.test.ts index fa7524a73..f2f2b6a1f 100644 --- a/src/__functional__/monorepo/getPackageInfos.test.ts +++ b/src/__functional__/monorepo/getPackageInfos.test.ts @@ -76,18 +76,18 @@ describe('getPackageInfos', () => { it('throws if run outside a git repo', () => { tempDir = tmpdir(); - expect(() => getPackageInfos(tempDir!)).toThrow(/not in a git repository/); + expect(() => getPackageInfos({ path: tempDir! })).toThrow(/not in a git repository/); }); it('returns empty object if no packages are found', () => { tempDir = tmpdir(); gitFailFast(['init'], { cwd: tempDir }); - expect(getPackageInfos(tempDir)).toEqual({}); + expect(getPackageInfos({ path: tempDir })).toEqual({}); }); it('works in single-package repo', () => { const repo = singleFactory.cloneRepository(); - let packageInfos = getPackageInfos(repo.rootPath); + let packageInfos = getPackageInfos({ path: repo.rootPath }); packageInfos = cleanPackageInfos(repo.rootPath, packageInfos); expect(packageInfos).toMatchInlineSnapshot(` Object { @@ -108,7 +108,7 @@ describe('getPackageInfos', () => { // both yarn and npm define "workspaces" in package.json it('works in yarn/npm monorepo', () => { const repo = monorepoFactory.cloneRepository(); - let packageInfos = getPackageInfos(repo.rootPath); + let packageInfos = getPackageInfos({ path: repo.rootPath }); packageInfos = cleanPackageInfos(repo.rootPath, packageInfos); expect(packageInfos).toMatchInlineSnapshot(` Object { @@ -158,7 +158,7 @@ describe('getPackageInfos', () => { fs.writeFileSync(repo.pathTo('pnpm-lock.yaml'), ''); fs.writeFileSync(repo.pathTo('pnpm-workspace.yaml'), 'packages: ["packages/*", "packages/grouped/*"]'); - const rootPackageInfos = getPackageInfos(repo.rootPath); + const rootPackageInfos = getPackageInfos({ path: repo.rootPath }); expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(` Object { "a": "packages/grouped/a/package.json", @@ -178,7 +178,7 @@ describe('getPackageInfos', () => { projects: [{ projectFolder: 'packages' }, { projectFolder: 'packages/grouped' }], }); - const rootPackageInfos = getPackageInfos(repo.rootPath); + const rootPackageInfos = getPackageInfos({ path: repo.rootPath }); expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(` Object { "a": "packages/grouped/a/package.json", @@ -196,7 +196,7 @@ describe('getPackageInfos', () => { fs.writeJSONSync(repo.pathTo('package.json'), { name: 'lerna-monorepo', version: '1.0.0', private: true }); fs.writeJSONSync(repo.pathTo('lerna.json'), { packages: ['packages/*', 'packages/grouped/*'] }); - const rootPackageInfos = getPackageInfos(repo.rootPath); + const rootPackageInfos = getPackageInfos({ path: repo.rootPath }); expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(` Object { "a": "packages/grouped/a/package.json", @@ -212,7 +212,7 @@ describe('getPackageInfos', () => { const repo = multiWorkspaceFactory.cloneRepository(); // For this test, only snapshot the package names and paths - const rootPackageInfos = getPackageInfos(repo.rootPath); + const rootPackageInfos = getPackageInfos({ path: repo.rootPath }); expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(` Object { "@workspace-a/a": "workspace-a/packages/grouped/a/package.json", @@ -231,7 +231,7 @@ describe('getPackageInfos', () => { `); const workspaceARoot = repo.pathTo('workspace-a'); - const packageInfosA = getPackageInfos(workspaceARoot); + const packageInfosA = getPackageInfos({ path: workspaceARoot }); expect(getPackageNamesAndPaths(workspaceARoot, packageInfosA)).toMatchInlineSnapshot(` Object { "@workspace-a/a": "packages/grouped/a/package.json", @@ -243,7 +243,7 @@ describe('getPackageInfos', () => { `); const workspaceBRoot = repo.pathTo('workspace-b'); - const packageInfosB = getPackageInfos(workspaceBRoot); + const packageInfosB = getPackageInfos({ path: workspaceBRoot }); expect(getPackageNamesAndPaths(workspaceBRoot, packageInfosB)).toMatchInlineSnapshot(` Object { "@workspace-b/a": "packages/grouped/a/package.json", @@ -262,6 +262,32 @@ describe('getPackageInfos', () => { const repo = multiWorkspaceFactory.cloneRepository(); repo.updateJsonFile('workspace-a/packages/foo/package.json', { name: 'foo' }); repo.updateJsonFile('workspace-b/packages/foo/package.json', { name: 'foo' }); - expect(() => getPackageInfos(repo.rootPath)).toThrow(); + expect(() => getPackageInfos({ path: repo.rootPath })).toThrow(); + }); + + it('does not throw if ignored fixture packages have the same name', () => { + // This setup mimics workspace-tools + const repo = singleFactory.cloneRepository(); + for (const fixture of ['monorepo-1', 'monorepo-2']) { + const fixturePath = `src/__fixtures__/${fixture}`; + repo.writeChange( + `${fixturePath}/package.json`, + JSON.stringify({ name: fixture, workspaces: { packages: ['packages/*'] } }) + ); + repo.writeChange(`${fixturePath}/yarn.lock`); + repo.writeChange(`${fixturePath}/packages/package-a/package.json`, JSON.stringify({ name: 'package-a' })); + repo.writeChange(`${fixturePath}/packages/package-b/package.json`, JSON.stringify({ name: 'package-b' })); + } + repo.commitAll(); + + // First verify that it DOES throw with no ignores (otherwise the test is invalid) + expect(() => getPackageInfos({ path: repo.rootPath })).toThrow(); + // Then verify it ignores as requested + expect(() => + getPackageInfos({ + path: repo.rootPath, + ignorePatterns: ['src/__fixtures__/**'], + }) + ).not.toThrow(); }); }); diff --git a/src/__functional__/monorepo/getScopedPackages.test.ts b/src/__functional__/monorepo/getScopedPackages.test.ts index 8fe0617a3..01bb52c75 100644 --- a/src/__functional__/monorepo/getScopedPackages.test.ts +++ b/src/__functional__/monorepo/getScopedPackages.test.ts @@ -14,7 +14,7 @@ describe('getScopedPackages', () => { beforeAll(() => { repoFactory = new RepositoryFactory('monorepo'); repo = repoFactory.cloneRepository(); - packageInfos = getPackageInfos(repo.rootPath); + packageInfos = getPackageInfos({ path: repo.rootPath }); }); afterAll(() => { repoFactory.cleanUp(); diff --git a/src/__functional__/validation/isChangeFileNeeded.test.ts b/src/__functional__/validation/isChangeFileNeeded.test.ts index b8d61dc7c..5d8e96521 100644 --- a/src/__functional__/validation/isChangeFileNeeded.test.ts +++ b/src/__functional__/validation/isChangeFileNeeded.test.ts @@ -25,42 +25,36 @@ describe('isChangeFileNeeded', () => { }); it('is false when no changes have been made', () => { - const result = isChangeFileNeeded( - { - branch: defaultRemoteBranchName, - path: repository.rootPath, - fetch: false, - } as BeachballOptions, - getPackageInfos(repository.rootPath) - ); + const options = { + branch: defaultRemoteBranchName, + path: repository.rootPath, + fetch: false, + } as BeachballOptions; + const result = isChangeFileNeeded(options, getPackageInfos(options)); expect(result).toBeFalsy(); }); it('is true when changes exist in a new branch', () => { repository.checkout('-b', 'feature-0'); repository.commitChange('myFilename'); - const result = isChangeFileNeeded( - { - branch: defaultRemoteBranchName, - path: repository.rootPath, - fetch: false, - } as BeachballOptions, - getPackageInfos(repository.rootPath) - ); + const options = { + branch: defaultRemoteBranchName, + path: repository.rootPath, + fetch: false, + } as BeachballOptions; + const result = isChangeFileNeeded(options, getPackageInfos(options)); expect(result).toBeTruthy(); }); it('is false when changes are CHANGELOG files', () => { repository.checkout('-b', 'feature-0'); repository.commitChange('CHANGELOG.md'); - const result = isChangeFileNeeded( - { - branch: defaultRemoteBranchName, - path: repository.rootPath, - fetch: false, - } as BeachballOptions, - getPackageInfos(repository.rootPath) - ); + const options = { + branch: defaultRemoteBranchName, + path: repository.rootPath, + fetch: false, + } as BeachballOptions; + const result = isChangeFileNeeded(options, getPackageInfos(options)); expect(result).toBeFalsy(); }); @@ -71,15 +65,11 @@ describe('isChangeFileNeeded', () => { repo.checkout('-b', 'feature-0'); repo.commitChange('CHANGELOG.md'); - expect(() => { - isChangeFileNeeded( - { - branch: defaultRemoteBranchName, - path: repo.rootPath, - fetch: true, - } as BeachballOptions, - getPackageInfos(repo.rootPath) - ); - }).toThrow(); + const options = { + branch: defaultRemoteBranchName, + path: repo.rootPath, + fetch: true, + } as BeachballOptions; + expect(() => isChangeFileNeeded(options, getPackageInfos(options))).toThrow(); }); }); diff --git a/src/changefile/promptForChange.ts b/src/changefile/promptForChange.ts index 4baf84f63..cdf8d0e45 100644 --- a/src/changefile/promptForChange.ts +++ b/src/changefile/promptForChange.ts @@ -20,7 +20,7 @@ export async function promptForChange(options: BeachballOptions): Promise !info.private && scopedPackages.has(pkg))); diff --git a/src/monorepo/getPackageInfos.ts b/src/monorepo/getPackageInfos.ts index ba8cd8afb..da635cc78 100644 --- a/src/monorepo/getPackageInfos.ts +++ b/src/monorepo/getPackageInfos.ts @@ -1,26 +1,33 @@ import fs from 'fs-extra'; +import minimatch from 'minimatch'; import path from 'path'; import { getWorkspaces, listAllTrackedFiles, findPackageRoot, findProjectRoot } from 'workspace-tools'; -import { PackageInfos } from '../types/PackageInfo'; +import { BeachballOptions } from '../types/BeachballOptions'; +import { PackageInfo, PackageInfos } from '../types/PackageInfo'; import { infoFromPackageJson } from './infoFromPackageJson'; /** - * Get a mapping from package name to package info for all packages in the workspace - * (reading from package.json files) + * Get a mapping from package name to package info for all packages that are in scope in the workspace + * (reading from package.json files). */ -export function getPackageInfos(cwd: string): PackageInfos { +export function getPackageInfos(options: Pick): PackageInfos { + const { path: cwd, ignorePatterns } = options; const projectRoot = findProjectRoot(cwd); const packageRoot = findPackageRoot(cwd); + const ignoreRegex = ignorePatterns?.map(pattern => minimatch.makeRe(pattern, { matchBase: true })); + const isPackageIgnored = (info: PackageInfo) => + !!ignoreRegex && ignoreRegex.some(regex => regex.test(path.relative(cwd, info.packageJsonPath))); + return ( - (projectRoot && getPackageInfosFromWorkspace(projectRoot)) || - (projectRoot && getPackageInfosFromNonWorkspaceMonorepo(projectRoot)) || + (projectRoot && getPackageInfosFromWorkspace(projectRoot, isPackageIgnored)) || + (projectRoot && getPackageInfosFromNonWorkspaceMonorepo(projectRoot, isPackageIgnored)) || (packageRoot && getPackageInfosFromSingleRepo(packageRoot)) || {} ); } -function getPackageInfosFromWorkspace(projectRoot: string) { +function getPackageInfosFromWorkspace(projectRoot: string, isPackageIgnored: (pkg: PackageInfo) => boolean) { try { const packageInfos: PackageInfos = {}; @@ -33,7 +40,10 @@ function getPackageInfosFromWorkspace(projectRoot: string) { const packageJsonPath = path.join(packagePath, 'package.json'); try { - packageInfos[packageJson.name] = infoFromPackageJson(packageJson, packageJsonPath); + const packageInfo = infoFromPackageJson(packageJson, packageJsonPath); + if (!isPackageIgnored(packageInfo)) { + packageInfos[packageJson.name] = packageInfo; + } } catch (e) { // Pass, the package.json is invalid console.warn(`Invalid package.json file detected ${packageJsonPath}: `, e); @@ -47,7 +57,7 @@ function getPackageInfosFromWorkspace(projectRoot: string) { } } -function getPackageInfosFromNonWorkspaceMonorepo(projectRoot: string) { +function getPackageInfosFromNonWorkspaceMonorepo(projectRoot: string, isPackageIgnored: (pkg: PackageInfo) => boolean) { const packageJsonFiles = listAllTrackedFiles(['**/package.json', 'package.json'], projectRoot); const packageInfos: PackageInfos = {}; @@ -58,15 +68,18 @@ function getPackageInfosFromNonWorkspaceMonorepo(projectRoot: string) { try { const packageJsonFullPath = path.join(projectRoot, packageJsonPath); const packageJson = fs.readJSONSync(packageJsonFullPath); - if (packageInfos[packageJson.name]) { - hasDuplicatePackage = true; - throw new Error( - `Two packages in different workspaces have the same name. Please rename one of these packages:\n- ${ - packageInfos[packageJson.name].packageJsonPath - }\n- ${packageJsonPath}` - ); + const packageInfo = infoFromPackageJson(packageJson, packageJsonFullPath); + if (!isPackageIgnored(packageInfo)) { + if (packageInfos[packageJson.name]) { + hasDuplicatePackage = true; + throw new Error( + `Two packages in different workspaces have the same name. Please rename one of these packages:\n- ${ + packageInfos[packageJson.name].packageJsonPath + }\n- ${packageJsonPath}` + ); + } + packageInfos[packageJson.name] = infoFromPackageJson(packageJson, packageJsonFullPath); } - packageInfos[packageJson.name] = infoFromPackageJson(packageJson, packageJsonFullPath); } catch (e) { if (hasDuplicatePackage) { throw e; // duplicate package error should propagate diff --git a/src/validation/isValidGroupOptions.ts b/src/validation/isValidGroupOptions.ts index 1e861a01d..b04108327 100644 --- a/src/validation/isValidGroupOptions.ts +++ b/src/validation/isValidGroupOptions.ts @@ -1,8 +1,9 @@ -import { VersionGroupOptions } from '../types/BeachballOptions'; +import { BeachballOptions } from '../types/BeachballOptions'; import { getPackageGroups } from '../monorepo/getPackageGroups'; import { getPackageInfos } from '../monorepo/getPackageInfos'; -export function isValidGroupOptions(root: string, groups: VersionGroupOptions[]) { +export function isValidGroupOptions(options: BeachballOptions) { + const { path: root, groups } = options; if (!Array.isArray(groups)) { return false; } @@ -13,7 +14,7 @@ export function isValidGroupOptions(root: string, groups: VersionGroupOptions[]) } } - const packageInfos = getPackageInfos(root); + const packageInfos = getPackageInfos(options); const packageGroups = getPackageGroups(packageInfos, root, groups); // make sure no disallowed changetype options exist inside an individual package diff --git a/src/validation/validate.ts b/src/validation/validate.ts index aa8a6fa6c..c7430bb3d 100644 --- a/src/validation/validate.ts +++ b/src/validation/validate.ts @@ -36,7 +36,7 @@ export function validate(options: BeachballOptions, validateOptions?: Partial Date: Fri, 9 Sep 2022 22:11:09 -0700 Subject: [PATCH 2/3] Change files --- change/beachball-d9a5a809-bde2-42fe-8358-3d9843e604ec.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/beachball-d9a5a809-bde2-42fe-8358-3d9843e604ec.json diff --git a/change/beachball-d9a5a809-bde2-42fe-8358-3d9843e604ec.json b/change/beachball-d9a5a809-bde2-42fe-8358-3d9843e604ec.json new file mode 100644 index 000000000..c1dd75094 --- /dev/null +++ b/change/beachball-d9a5a809-bde2-42fe-8358-3d9843e604ec.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Respect ignorePatterns in getPackageInfos", + "packageName": "beachball", + "email": "elcraig@microsoft.com", + "dependentChangeType": "patch" +} From 9d5a63ae40a887ae5b135ab220c3d630740f0b92 Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Sun, 11 Sep 2022 17:26:22 -0700 Subject: [PATCH 3/3] fix comparison --- src/__functional__/monorepo/getPackageInfos.test.ts | 2 +- src/monorepo/getPackageInfos.ts | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/__functional__/monorepo/getPackageInfos.test.ts b/src/__functional__/monorepo/getPackageInfos.test.ts index f2f2b6a1f..726e24599 100644 --- a/src/__functional__/monorepo/getPackageInfos.test.ts +++ b/src/__functional__/monorepo/getPackageInfos.test.ts @@ -265,7 +265,7 @@ describe('getPackageInfos', () => { expect(() => getPackageInfos({ path: repo.rootPath })).toThrow(); }); - it('does not throw if ignored fixture packages have the same name', () => { + it.only('does not throw if ignored fixture packages have the same name', () => { // This setup mimics workspace-tools const repo = singleFactory.cloneRepository(); for (const fixture of ['monorepo-1', 'monorepo-2']) { diff --git a/src/monorepo/getPackageInfos.ts b/src/monorepo/getPackageInfos.ts index da635cc78..12327b93a 100644 --- a/src/monorepo/getPackageInfos.ts +++ b/src/monorepo/getPackageInfos.ts @@ -15,9 +15,12 @@ export function getPackageInfos(options: Pick minimatch.makeRe(pattern, { matchBase: true })); - const isPackageIgnored = (info: PackageInfo) => - !!ignoreRegex && ignoreRegex.some(regex => regex.test(path.relative(cwd, info.packageJsonPath))); + const ignoreRegex = ignorePatterns?.map(pattern => new minimatch.Minimatch(pattern, { matchBase: true })); + const isPackageIgnored = (info: PackageInfo) => { + if (!ignoreRegex) return false; + const comparePath = path.relative(cwd, info.packageJsonPath); + return ignoreRegex.some(pattern => pattern.match(comparePath)); + }; return ( (projectRoot && getPackageInfosFromWorkspace(projectRoot, isPackageIgnored)) ||