Address PR review comments

- Remove cli-node/src/paths.ts compat layer, migrate all cli-node
  internal usage to import targetPaths from @backstage/cli-common
- Use single-arg overrideTargetPaths('/root') where dir === rootDir
- Scope mockDir to each describe block in bump.test.ts to avoid
  shared state issues with overrideTargetPaths
- Remove unnecessary overrideTargetPaths from plugin-manager.test.ts
- Remove stale findPaths mock from createApp.test.ts
- Use overrideTargetPaths in getWorkspaceRoot.test.ts and cli-node tests

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

packages/backend-dynamic-feature-service/src/manager/plugin-manager.test.ts

@@ -39,7 +39,6 @@ import { ConfigSources } from '@backstage/config-loader';
 import { Logs, MockedLogger, LogContent } from '../__testUtils__/testUtils';
 import { PluginScanner } from '../scanner/plugin-scanner';
 import { targetPaths } from '@backstage/cli-common';
-import { overrideTargetPaths } from '@backstage/cli-common/testUtils';
 import { createMockDirectory } from '@backstage/backend-test-utils';
 import { rootLifecycleServiceFactory } from '@backstage/backend-defaults/rootLifecycle';
 import { BackstagePackageJson, PackageRole } from '@backstage/cli-node';
@@ -47,10 +46,6 @@ import { BackstagePackageJson, PackageRole } from '@backstage/cli-node';
 describe('backend-dynamic-feature-service', () => {
   const mockDir = createMockDirectory();
 
-  beforeAll(() => {
-    overrideTargetPaths(require('path').resolve(__dirname, '../../../..'));
-  });
-
   describe('loadPlugins', () => {
     afterEach(() => {
       jest.resetModules();

packages/cli-node/src/git/GitUtils.ts

@@ -15,7 +15,7 @@
  */
 
 import { assertError, ForwardedError } from '@backstage/errors';
-import { paths } from '../paths';
+import { targetPaths } from '@backstage/cli-common';
 import { runOutput } from '@backstage/cli-common';
 
 /**
@@ -24,7 +24,7 @@ import { runOutput } from '@backstage/cli-common';
 export async function runGit(...args: string[]) {
   try {
     const stdout = await runOutput(['git', ...args], {
-      cwd: paths.targetRoot,
+      cwd: targetPaths.rootDir,
     });
     return stdout.trim().split(/\r\n|\r|\n/);
   } catch (error) {
@@ -88,7 +88,7 @@ export class GitUtils {
     }
 
     const stdout = await runOutput(['git', 'show', `${showRef}:${path}`], {
-      cwd: paths.targetRoot,
+      cwd: targetPaths.rootDir,
     });
     return stdout;
   }

packages/cli-node/src/monorepo/PackageGraph.test.ts

@@ -14,21 +14,16 @@
  * limitations under the License.
  */
 
-import { resolve as resolvePath } from 'node:path';
 import { getPackages } from '@manypkg/get-packages';
 import { PackageGraph } from './PackageGraph';
 import { Lockfile } from './Lockfile';
 import { GitUtils } from '../git';
+import { overrideTargetPaths } from '@backstage/cli-common/testUtils';
 
 const mockListChangedFiles = jest.spyOn(GitUtils, 'listChangedFiles');
 const mockReadFileAtRef = jest.spyOn(GitUtils, 'readFileAtRef');
 
-jest.mock('../paths', () => ({
-  paths: {
-    targetRoot: '/',
-    resolveTargetRoot: (...paths: string[]) => resolvePath('/', ...paths),
-  },
-}));
+overrideTargetPaths('/');
 
 const testPackages = [
   {

packages/cli-node/src/monorepo/PackageGraph.ts

@@ -16,7 +16,7 @@
 
 import path from 'node:path';
 import { getPackages, Package } from '@manypkg/get-packages';
-import { paths } from '../paths';
+import { targetPaths } from '@backstage/cli-common';
 import { PackageRole } from '../roles';
 import { GitUtils } from '../git';
 import { Lockfile } from './Lockfile';
@@ -192,7 +192,7 @@ export class PackageGraph extends Map<string, PackageGraphNode> {
    * Lists all local packages in a monorepo.
    */
   static async listTargetPackages(): Promise<BackstagePackage[]> {
-    const { packages } = await getPackages(paths.targetDir);
+    const { packages } = await getPackages(targetPaths.dir);
 
     return packages as BackstagePackage[];
   }
@@ -332,7 +332,7 @@ export class PackageGraph extends Map<string, PackageGraphNode> {
       Array.from(this.values()).map(pkg => [
         // relative from root, convert to posix, and add a / at the end
         path
-          .relative(paths.targetRoot, pkg.dir)
+          .relative(targetPaths.rootDir, pkg.dir)
           .split(path.sep)
           .join(path.posix.sep) + path.posix.sep,
         pkg,
@@ -374,7 +374,7 @@ export class PackageGraph extends Map<string, PackageGraphNode> {
       let otherLockfile: Lockfile;
       try {
         thisLockfile = await Lockfile.load(
-          paths.resolveTargetRoot('yarn.lock'),
+          targetPaths.resolveRoot('yarn.lock'),
         );
         otherLockfile = Lockfile.parse(
           await GitUtils.readFileAtRef('yarn.lock', options.ref),

packages/cli-node/src/monorepo/isMonoRepo.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { paths } from '../paths';
+import { targetPaths } from '@backstage/cli-common';
 import fs from 'fs-extra';
 
 /**
@@ -23,7 +23,7 @@ import fs from 'fs-extra';
  * @public
  */
 export async function isMonoRepo(): Promise<boolean> {
-  const rootPackageJsonPath = paths.resolveTargetRoot('package.json');
+  const rootPackageJsonPath = targetPaths.resolveRoot('package.json');
   try {
     const pkg = await fs.readJson(rootPackageJsonPath);
     return Boolean(pkg?.workspaces?.packages);

packages/cli-node/src/monorepo/isMonorepo.test.ts

@@ -16,12 +16,10 @@
 
 import { isMonoRepo } from './isMonoRepo';
 import { createMockDirectory } from '@backstage/backend-test-utils';
+import { overrideTargetPaths } from '@backstage/cli-common/testUtils';
 
 const mockDir = createMockDirectory();
-
-jest.mock('../paths', () => ({
-  paths: { resolveTargetRoot: (...args: string[]) => mockDir.resolve(...args) },
-}));
+overrideTargetPaths(mockDir.path);
 
 describe('isMonoRepo', () => {
   it('should detect a monorepo', async () => {

packages/cli-node/src/pacman/PackageManager.test.ts

@@ -15,16 +15,13 @@
  */
 
 import { createMockDirectory } from '@backstage/backend-test-utils';
+import { overrideTargetPaths } from '@backstage/cli-common/testUtils';
 import { detectPackageManager } from './PackageManager';
 import { Yarn } from './yarn';
 import { withLogCollector } from '@backstage/test-utils';
 
 const mockDir = createMockDirectory();
-
-jest.mock('../paths', () => ({
-  ...jest.requireActual('../paths'),
-  paths: { resolveTargetRoot: (...args: string[]) => mockDir.resolve(...args) },
-}));
+overrideTargetPaths(mockDir.path);
 
 const mockYarnCreate = jest.spyOn(Yarn, 'create');
 

packages/cli-node/src/pacman/PackageManager.ts

@@ -16,7 +16,7 @@
 
 import { Yarn } from './yarn';
 import { Lockfile } from './Lockfile';
-import { paths } from '../paths';
+import { targetPaths } from '@backstage/cli-common';
 import { RunOptions } from '@backstage/cli-common';
 import fs from 'fs-extra';
 
@@ -91,7 +91,7 @@ export interface PackageManager {
  */
 export async function detectPackageManager(): Promise<PackageManager> {
   const hasYarnLockfile = await fileExists(
-    paths.resolveTargetRoot('yarn.lock'),
+    targetPaths.resolveRoot('yarn.lock'),
   );
   if (hasYarnLockfile) {
     return await Yarn.create();
@@ -99,7 +99,7 @@ export async function detectPackageManager(): Promise<PackageManager> {
 
   try {
     const packageJson = await fs.readJson(
-      paths.resolveTargetRoot('package.json'),
+      targetPaths.resolveRoot('package.json'),
     );
     if (packageJson.workspaces) {
       // technically this could be NPM as well

packages/cli-node/src/pacman/yarn/Yarn.test.ts

@@ -15,14 +15,11 @@
  */
 
 import { createMockDirectory } from '@backstage/backend-test-utils';
+import { overrideTargetPaths } from '@backstage/cli-common/testUtils';
 import { Yarn } from './Yarn';
 
 const mockDir = createMockDirectory();
-
-jest.mock('../../paths', () => ({
-  ...jest.requireActual('../../paths'),
-  paths: { resolveTargetRoot: (...args: string[]) => mockDir.resolve(...args) },
-}));
+overrideTargetPaths(mockDir.path);
 
 const yarnClassic = new Yarn({ version: '1.0.0', codename: 'classic' });
 const yarnBerry = new Yarn({ version: '3.0.0', codename: 'berry' });

packages/cli-node/src/pacman/yarn/Yarn.ts

@@ -23,7 +23,7 @@ import { PackageInfo, PackageManager } from '../PackageManager';
 import { Lockfile } from '../Lockfile';
 import { YarnVersion } from './types';
 import fs from 'fs-extra';
-import { paths } from '../../paths';
+import { targetPaths } from '@backstage/cli-common';
 import { run, runOutput, RunOptions } from '@backstage/cli-common';
 
 export class Yarn implements PackageManager {
@@ -47,7 +47,7 @@ export class Yarn implements PackageManager {
   }
 
   async getMonorepoPackages() {
-    const rootPackageJsonPath = paths.resolveTargetRoot('package.json');
+    const rootPackageJsonPath = targetPaths.resolveRoot('package.json');
     try {
       const pkg = await fs.readJson(rootPackageJsonPath);
       return pkg?.workspaces?.packages || [];

packages/cli-node/src/paths.ts

@@ -1,40 +0,0 @@
-/*
- * Copyright 2020 The Backstage Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import { targetPaths, findOwnPaths } from '@backstage/cli-common';
-
-/* eslint-disable-next-line no-restricted-syntax */
-const ownPaths = findOwnPaths(__dirname);
-
-/* eslint-disable-next-line no-restricted-syntax */
-export const paths = {
-  get ownDir() {
-    return ownPaths.dir;
-  },
-  get ownRoot() {
-    return ownPaths.rootDir;
-  },
-  get targetDir() {
-    return targetPaths.dir;
-  },
-  get targetRoot() {
-    return targetPaths.rootDir;
-  },
-  resolveOwn: ownPaths.resolve,
-  resolveOwnRoot: ownPaths.resolveRoot,
-  resolveTarget: targetPaths.resolve,
-  resolveTargetRoot: targetPaths.resolveRoot,
-};

packages/cli/src/modules/build/commands/repo/start.test.ts

@@ -18,7 +18,7 @@ import { PackageGraph } from '@backstage/cli-node';
 import { findTargetPackages } from './start';
 import { overrideTargetPaths } from '@backstage/cli-common/testUtils';
 
-overrideTargetPaths({ dir: '/root', rootDir: '/root' });
+overrideTargetPaths('/root');
 
 const mocks = {
   app: {

packages/cli/src/modules/migrate/commands/versions/bump.test.ts

@@ -23,10 +23,7 @@ import { YarnInfoInspectData } from '../../../../lib/versioning/packages';
 import { setupServer } from 'msw/node';
 import { rest } from 'msw';
 import { NotFoundError } from '@backstage/errors';
-import {
-  createMockDirectory,
-  MockDirectory,
-} from '@backstage/backend-test-utils';
+import { createMockDirectory } from '@backstage/backend-test-utils';
 
 // Avoid mutating the global agents used in other tests
 jest.mock('global-agent', () => ({
@@ -60,17 +57,10 @@ jest.mock('ora', () => ({
   },
 }));
 
-let mockDir: MockDirectory;
 jest.mock('@backstage/cli-common', () => {
   const actual = jest.requireActual('@backstage/cli-common');
   return {
     ...actual,
-    findPaths: () => ({
-      resolveTargetRoot: (...args: string[]) => mockDir.resolve(...args),
-      get targetDir() {
-        return mockDir.path;
-      },
-    }),
     run: jest.fn().mockReturnValue({
       exitCode: null,
       waitForExit: jest.fn().mockResolvedValue(undefined),
@@ -137,10 +127,10 @@ const expectLogsToMatch = (
 };
 
 describe('bump', () => {
-  mockDir = createMockDirectory();
-  beforeAll(() => overrideTargetPaths(mockDir.path));
+  const mockDir = createMockDirectory();
 
   beforeEach(() => {
+    overrideTargetPaths(mockDir.path);
     mockFetchPackageInfo.mockImplementation(async name => ({
       name: name,
       'dist-tags': {
@@ -944,8 +934,11 @@ describe('bump', () => {
 });
 
 describe('bumpBackstageJsonVersion', () => {
-  mockDir = createMockDirectory();
-  beforeAll(() => overrideTargetPaths(mockDir.path));
+  const mockDir = createMockDirectory();
+
+  beforeEach(() => {
+    overrideTargetPaths(mockDir.path);
+  });
 
   afterEach(() => {
     jest.resetAllMocks();
@@ -1079,10 +1072,14 @@ describe('createVersionFinder', () => {
 });
 
 describe('environment variables', () => {
+  const mockDir = createMockDirectory();
+
   const worker = setupServer();
   registerMswTestHooks(worker);
 
-  beforeAll(() => overrideTargetPaths(mockDir.path));
+  beforeEach(() => {
+    overrideTargetPaths(mockDir.path);
+  });
 
   beforeEach(() => {
     delete process.env.BACKSTAGE_MANIFEST_FILE;

packages/create-app/src/createApp.test.ts

@@ -42,7 +42,6 @@ jest.mock('@backstage/cli-common', () => {
   };
   return {
     ...actual,
-    findPaths: jest.fn(),
     findOwnPaths: () => mockOwnPaths,
   };
 });

packages/yarn-plugin/src/util/getWorkspaceRoot.test.ts

@@ -14,8 +14,6 @@
  * limitations under the License.
  */
 
-import { targetPaths } from '@backstage/cli-common';
-
 const setPlatform = (platform: string) => {
   Object.defineProperty(process, `platform`, {
     configurable: true,
@@ -37,7 +35,6 @@ describe('getWorkspaceRoot', () => {
   `('platform: $platform', ({ platform, native, portable }) => {
     let realPlatform: string;
     let getWorkspaceRoot: () => string;
-    let mockResolveRoot: jest.MockedFunction<typeof targetPaths.resolveRoot>;
 
     beforeEach(() => {
       realPlatform = process.platform;
@@ -45,21 +42,10 @@ describe('getWorkspaceRoot', () => {
 
       jest.resetModules();
 
-      mockResolveRoot = jest.fn();
-
-      jest.doMock('@backstage/cli-common', () => ({
-        ...jest.requireActual('@backstage/cli-common'),
-        targetPaths: {
-          get dir() {
-            return mockResolveRoot();
-          },
-          get rootDir() {
-            return mockResolveRoot();
-          },
-          resolveRoot: mockResolveRoot,
-        },
-      }));
-
+      const {
+        overrideTargetPaths,
+      } = require('@backstage/cli-common/testUtils');
+      overrideTargetPaths(native);
       getWorkspaceRoot = require('./getWorkspaceRoot').getWorkspaceRoot;
     });
 
@@ -68,8 +54,6 @@ describe('getWorkspaceRoot', () => {
     });
 
     it('returns an appropriately-formatted workspace root path', () => {
-      mockResolveRoot.mockReturnValue(native);
-
       expect(getWorkspaceRoot()).toEqual(portable);
     });
   });