[identity] src/build testing

Author: deyaaeldeen

common/tools/dev-tool/src/commands/admin/migrate-tests.ts

@@ -160,11 +160,6 @@ async function updatePackageJson(projectFolder: string, packageName: string): Pr
     delete packageJson.scripts["build:node"];
     delete packageJson.scripts["minify"];
 
-    packageJson.scripts["test:node:esm"] = "dev-tool run test:vitest --esm";
-    if (packageJson.scripts["test:node"].includes("--no-test-proxy")) {
-      packageJson.scripts["test:node:esm"] += " --no-test-proxy";
-    }
-
     if (
       packageName.startsWith("@azure-tests/perf-") ||
       packageName === "@azure/dev-tool" ||

common/tools/dev-tool/src/commands/run/build-test.ts

@@ -2,11 +2,12 @@
 // Licensed under the MIT License.
 
 import path from "node:path";
-import { cpSync, existsSync, mkdirSync, readdirSync, statSync, writeFileSync } from "node:fs";
+import { cpSync, existsSync, mkdirSync, readdirSync, writeFileSync } from "node:fs";
+import { stat } from "node:fs/promises";
 import { leafCommand, makeCommandInfo } from "../../framework/command";
 import { createPrinter } from "../../util/printer";
 import { resolveProject } from "../../util/resolveProject";
-import { resolveConfig } from "../../util/resolveTsConfig";
+import { type Config, resolveConfig } from "../../util/resolveTsConfig";
 import { spawnSync } from "node:child_process";
 
 const log = createPrinter("build-test");
@@ -22,11 +23,57 @@ export const commandInfo = makeCommandInfo("build-test", "build a package for te
     default: "tsconfig.browser.config.json",
     description: "path to the browser config file",
   },
+  "node-config": {
+    kind: "string",
+    default: "tsconfig.test.node.json",
+    description: "path to the node config file",
+  },
 });
 
+async function validateConfigFile(configPath: string): Promise<boolean> {
+  try {
+    const configStat = await stat(configPath);
+    if (!configStat.isFile()) {
+      log.error(`The path ${configPath} is not a file.`);
+      return false;
+    }
+    return true;
+  } catch (error) {
+    return false;
+  }
+}
+
+async function findAndRunTypeScriptConfig(
+  config: string,
+  fallbackConfig: string,
+): Promise<boolean> {
+  const configsToTry = [config, fallbackConfig];
+
+  for (const config of configsToTry) {
+    if (await validateConfigFile(config)) {
+      if (!(await runTypeScript(config))) {
+        return false;
+      }
+      log.info(`Typechecking succeeded using ${config}`);
+      return true;
+    }
+  }
+
+  log.error(
+    `No valid TypeScript config found. Please provide a valid config file using --node-config or ensure ${config} or ${fallbackConfig} exists.`,
+  );
+  return false;
+}
+
 export default leafCommand(commandInfo, async (options) => {
   const browserTest = options["browser-test"];
   const browserConfig = options["browser-config"];
+  const nodeConfig = options["node-config"];
+  const defaultConfig = "tsconfig.test.json";
+
+  if (!browserTest && !(await findAndRunTypeScriptConfig(nodeConfig, defaultConfig))) {
+    return false;
+  }
 
   const info = await resolveProject(process.cwd());
 
@@ -73,15 +120,8 @@ export default leafCommand(commandInfo, async (options) => {
   }
 
   if (browserTest) {
-    // Test for valid file
-    if (!existsSync(browserConfig)) {
-      log.error(`The file ${browserConfig} does not exist.`);
-      return false;
-    }
-
-    const browserConfigStat = statSync(browserConfig);
-    if (!browserConfigStat.isFile()) {
-      log.error(`The file ${browserConfig} does not exist.`);
+    if (!(await validateConfigFile(browserConfig))) {
+      log.error(`The browser config file ${browserConfig} does not exist.`);
       return false;
     }
 
@@ -93,6 +133,21 @@ export default leafCommand(commandInfo, async (options) => {
   return true;
 });
 
+async function runTypeScript(tsConfig: string): Promise<boolean> {
+  const res = spawnSync(`tsc -p ${tsConfig}`, [], {
+    stdio: "inherit",
+    shell: true,
+    cwd: process.cwd(),
+  });
+
+  if (res.status || res.signal) {
+    log.error(`TypeScript compilation failed for ${tsConfig}:`, res);
+    return false;
+  }
+
+  return true;
+}
+
 async function compileForEnvironment(
   type: string,
   tsConfig: string,
@@ -116,6 +171,12 @@ async function compileForEnvironment(
     mkdirSync(browserTestPath, { recursive: true });
   }
 
+  // Check if the TypeScript config uses package name imports or "internal" paths
+  const shouldSkipOverrides = hasPackageOrInternalPaths(tsConfigJSON);
+  if (shouldSkipOverrides) {
+    log.info("Detected package name or internal path mappings, skipping file overrides");
+  }
+
   // Create import map
   const imports: Record<string, string> = {};
   for (const [key, value] of importMap.entries()) {
@@ -128,25 +189,38 @@ async function compileForEnvironment(
   };
   writeFileSync(path.join(browserTestPath, "package.json"), JSON.stringify(packageJson, null, 2));
 
-  const res = spawnSync(`tsc -p ${tsConfig}`, [], {
-    stdio: "inherit",
-    shell: true,
-    cwd: process.cwd(),
-  });
-
-  if (res.status || res.signal) {
-    log.error(res);
+  if (!(await runTypeScript(tsConfig))) {
     return false;
   }
 
-  for (const [override, original] of overrideMap.entries()) {
-    log.info(`Replacing for : ${original} => ${override}`);
-    copyOverrides(type, outputPath, original);
+  // Only apply overrides if not using package name or internal path imports
+  if (!shouldSkipOverrides) {
+    for (const [override, original] of overrideMap.entries()) {
+      log.info(`Replacing for : ${original} => ${override}`);
+      copyOverrides(type, outputPath, original);
+    }
   }
 
   return true;
 }
 
+function hasPackageOrInternalPaths(tsConfig: Config): boolean {
+  const paths = tsConfig?.compilerOptions?.paths;
+  if (!paths || typeof paths !== "object") {
+    return false;
+  }
+
+  const pathKeys = Object.keys(paths);
+
+  // Check for package name imports (e.g., @azure/identity, @azure/identity/*)
+  const hasPackageImports = pathKeys.some((key) => key.startsWith("@") && key.includes("/"));
+
+  // Check for internal path imports (e.g., internal/*)
+  const hasInternalImports = pathKeys.some((key) => key.startsWith("internal/"));
+
+  return hasPackageImports || hasInternalImports;
+}
+
 function copyOverrides(type: string, rootDir: string, filePath: string): void {
   const fileParsed = path.parse(filePath);
   const fileToReplace = path.join(process.cwd(), path.normalize(fileParsed.dir));

eng/pipelines/run-for-all-packages.yml

@@ -44,9 +44,5 @@ stages:
             displayName: "Build All Packages"
 
           - script: |
-              node common/scripts/install-run-rush.js test -t @azure/identity -t @azure/app-configuration -t @azure/event-hubs -p 1 --verbose
+              node common/scripts/install-run-rush.js test -t @azure/identity -t @azure/app-configuration -t @azure/event-hubs -t @azure/storage-blob -t @azure-rest/synapse-access-control -p 1 --verbose
             displayName: "Run Unit Tests for Selected Packages"
-
-          - script: |
-              node common/scripts/install-run-rush.js test:node:esm --only @azure/app-configuration --only @azure/storage-blob --only @azure-rest/synapse-access-control -p 1 --verbose
-            displayName: "Run Node ESM Tests for Selected Packages"

sdk/identity/identity/eslint.config.mjs

@@ -5,6 +5,7 @@ export default azsdkEslint.config([
   {
     rules: {
       "@azure/azure-sdk/ts-naming-options": "warn",
+      "@typescript-eslint/ban-ts-comment": "warn"
     },
   },
 ]);

sdk/identity/identity/package.json

@@ -31,8 +31,7 @@
     "pack": "npm pack 2>&1",
     "test": "npm run test:node && npm run test:browser",
     "test:browser": "npm run clean && dev-tool run build-package && dev-tool run build-test && dev-tool run test:vitest --browser",
-    "test:node": "dev-tool run test:vitest",
-    "test:node:esm": "dev-tool run test:vitest --esm",
+    "test:node": "dev-tool run build-test --no-browser-test && dev-tool run test:vitest",
     "test:node:managed-identity": "dev-tool run test:vitest -- --test-timeout 180000 --config ./vitest.managed-identity.config.ts",
     "test:node:no-timeouts": "dev-tool run test:vitest -- --test-timeout=0",
     "update-snippets": "dev-tool run update-snippets"

sdk/identity/identity/src/errors.ts

@@ -76,7 +76,7 @@ export const CredentialUnavailableErrorName = "CredentialUnavailableError";
  */
 export class CredentialUnavailableError extends Error {
   constructor(message?: string, options?: { cause?: unknown }) {
-    // @ts-expect-error - TypeScript does not recognize this until we use ES2022 as the target; however, all our major runtimes do support the `cause` property
+    // @ts-ignore - TypeScript does not recognize this until we use ES2022 as the target; however, all our major runtimes do support the `cause` property
     super(message, options);
     this.name = CredentialUnavailableErrorName;
   }
@@ -143,7 +143,7 @@ export class AuthenticationError extends Error {
 
     super(
       `${errorResponse.error} Status code: ${statusCode}\nMore details:\n${errorResponse.errorDescription},`,
-      // @ts-expect-error - TypeScript does not recognize this until we use ES2022 as the target; however, all our major runtimes do support the `cause` property
+      // @ts-ignore - TypeScript does not recognize this until we use ES2022 as the target; however, all our major runtimes do support the `cause` property
       options,
     );
     this.statusCode = statusCode;
@@ -234,7 +234,7 @@ export class AuthenticationRequiredError extends Error {
   ) {
     super(
       options.message,
-      // @ts-expect-error - TypeScript does not recognize this until we use ES2022 as the target; however, all our major runtimes do support the `cause` property
+      // @ts-ignore - TypeScript does not recognize this until we use ES2022 as the target; however, all our major runtimes do support the `cause` property
       options.cause ? { cause: options.cause } : undefined,
     );
     this.scopes = options.scopes;

sdk/identity/identity/test/authTestUtils.ts

@@ -1,8 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import type { AuthenticationError } from "../src/index.js";
-import { AzureAuthorityHosts } from "../src/index.js";
+import type { AuthenticationError } from "@azure/identity";
+import { AzureAuthorityHosts } from "@azure/identity";
 import { assert } from "vitest";
 
 /**

sdk/identity/identity/test/httpRequestsCommon.ts

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-import type { AccessToken, GetTokenOptions, TokenCredential } from "../src/index.js";
+import type { AccessToken, GetTokenOptions, TokenCredential } from "@azure/identity";
 import type { AzureLogLevel, AzureLogger } from "@azure/logger";
 import type { RawHttpHeaders, RestError } from "@azure/core-rest-pipeline";
 

sdk/identity/identity/test/internal/logger.spec.ts

@@ -1,14 +1,14 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import type { AccessToken, GetTokenOptions, TokenCredential } from "../../src/index.js";
-import type { CredentialLogger } from "../../src/util/logging.js";
+import type { AccessToken, GetTokenOptions, TokenCredential } from "@azure/identity";
+import type { CredentialLogger } from "internal/util/logging.js";
 import {
   credentialLogger,
   credentialLoggerInstance,
   formatError,
   formatSuccess,
-} from "../../src/util/logging.js";
+} from "internal/util/logging.js";
 import { describe, it, assert } from "vitest";
 
 describe("Identity logging utilities", function () {

sdk/identity/identity/test/internal/node/azureCliCredential.spec.ts

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-import { AzureCliCredential } from "../../../src/credentials/azureCliCredential.js";
+import { AzureCliCredential } from "internal/credentials/azureCliCredential.js";
 import type { GetTokenOptions } from "@azure/core-auth";
 import child_process from "node:child_process";
 import { describe, it, assert, expect, vi, beforeEach, afterEach } from "vitest";