Merge branch 'main' into user/nabilat/face-v1.3-preview.1

Author: nabilat

eng/common/pipelines/templates/archetype-typespec-emitter.yml

@@ -241,6 +241,11 @@ extends:
               Paths:
               - "/*"
               - "!SessionRecords"
+          
+          - task: UseNode@1
+            displayName: 'Install Node.js'
+            inputs:
+              version: '22.x'
 
           - download: current
             displayName: Download pipeline artifacts
@@ -313,6 +318,11 @@ extends:
               - "/*"
               - "!SessionRecords"
 
+          - task: UseNode@1
+            displayName: 'Install Node.js'
+            inputs:
+              version: '22.x'
+
           - download: current
             displayName: Download pipeline artifacts
 
@@ -491,4 +501,4 @@ extends:
                   scriptType: "bash"
                   scriptLocation: "inlineScript"
                   inlineScript: npx tsp-spector upload-coverage --coverageFile $(Build.ArtifactStagingDirectory)/tsp-spector-coverage-azure.json --generatorName @azure-typespec/$(SpectorName) --storageAccountName typespec --containerName coverages --generatorVersion $(node -p -e "require('./package.json').version") --generatorMode azure
-                  workingDirectory: $(Build.SourcesDirectory)/eng/packages/$(SpectorName)
+                  workingDirectory: $(Build.SourcesDirectory)/eng/packages/$(SpectorName)

eng/pipelines/templates/stages/archetype-spec-gen-sdk.yml

@@ -279,9 +279,9 @@ extends:
                     -PRBody "$(GeneratedSDKInformation)"
                     -OpenAsDraft $true
 
-          - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}:
-            - template: /eng/common/pipelines/templates/steps/detect-api-changes.yml
-              parameters:
-                ArtifactPath: $(GeneratedSDK.StagedArtifactsFolder)
-                ArtifactName: $(PipelineArtifactsName)
-                RepoRoot: $(SdkRepoDirectory)
+            - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}:
+              - template: /eng/common/pipelines/templates/steps/detect-api-changes.yml
+                parameters:
+                  ArtifactPath: $(GeneratedSDK.StagedArtifactsFolder)
+                  ArtifactName: $(PipelineArtifactsName)
+                  RepoRoot: $(SdkRepoDirectory)

eng/tools/lint-diff/src/correlateResults.ts

@@ -10,6 +10,7 @@ export async function correlateRuns(
   afterChecks: AutorestRunResult[],
 ): Promise<Map<string, BeforeAfter>> {
   const runCorrelations = new Map<string, BeforeAfter>();
+  console.log("\nCorrelating runs...");
   for (const results of afterChecks) {
     const { readme, tag } = results;
     const key = tag ? `${readme}#${tag}` : readme;
@@ -83,7 +84,7 @@ export function getViolations(
   const newViolations: LintDiffViolation[] = [];
   const existingViolations: LintDiffViolation[] = [];
 
-  for (const [runKey, { before, after }] of runCorrelations.entries()) {
+  for (const [, { before, after }] of runCorrelations.entries()) {
     const beforeViolations = before
       ? getLintDiffViolations(before).filter(
           (v) => (isFailure(v.level) || isWarning(v.level)) && v.source?.length > 0,
@@ -98,12 +99,12 @@ export function getViolations(
           affectedSwaggers.has(relativizePath(v.source[0].document).slice(1))),
     );
 
-    const [newitems, existingItems] = getNewItems(beforeViolations, afterViolations);
-    console.log(`Correlated Run: ${runKey}`);
-    console.log(`New violations: ${newitems.length}`);
-    console.log(`Existing violations: ${existingItems.length}`);
+    const [newItems, existingItems] = getNewItems(beforeViolations, afterViolations);
+    console.log("Correlation:");
+    console.log(`\tBefore: Readme: ${before?.readme} Tag: ${before?.tag}`);
+    console.log(`\tAfter: Readme : ${after.readme} Tag: ${after.tag}`);
 
-    newViolations.push(...newitems);
+    newViolations.push(...newItems);
     existingViolations.push(...existingItems);
   }
 

eng/tools/lint-diff/src/generateReport.ts

@@ -20,6 +20,9 @@ export async function generateLintDiffReport(
   baseBranch: string,
   compareSha: string,
 ): Promise<boolean> {
+  
+  console.log("Generating LintDiff report...");
+
   let pass = true;
   let outputMarkdown = "";
 
@@ -34,7 +37,7 @@ export async function generateLintDiffReport(
   // Compared Specs | New Version | Base Version
   // <tag name> | link: readme.md#tag-<tag-name> | link: readme.md#tag-<tag-name>
   // ... | ... | ...
-  for (const [_, { before, after }] of runCorrelations.entries()) {
+  for (const [, { before, after }] of runCorrelations.entries()) {
     const afterName = getName(after);
     const beforeName = before ? getName(before) : "default";
     const afterPath = getPath(after);
@@ -47,7 +50,6 @@ export async function generateLintDiffReport(
 
   const [newViolations, existingViolations] = getViolations(runCorrelations, affectedSwaggers);
 
-  console.log("Populating armRpcs for new violations");
   for (const newItem of newViolations) {
     // TODO: Potential performance issue, make parallel
     newItem.armRpcs = await getRelatedArmRpcFromDoc(newItem.code);
@@ -56,12 +58,7 @@ export async function generateLintDiffReport(
   newViolations.sort(compareLintDiffViolations);
   existingViolations.sort(compareLintDiffViolations);
 
-  if (newViolations.some((v) => isFailure(v.level))) {
-    // New violations with level error or fatal fail the build. If all new
-    // violations are warnings, the build passes.
-    pass = false;
-  }
-
+  console.log(`New violations: ${newViolations.length}`);
   if (newViolations.length > 0) {
     outputMarkdown += "**[must fix]The following errors/warnings are intorduced by current PR:**\n";
     if (newViolations.length > 50) {
@@ -76,11 +73,21 @@ export async function generateLintDiffReport(
       outputMarkdown += getNewViolationReportRow(violation, compareSha);
     }
 
+    if (newViolations.some((v) => isFailure(v.level))) {
+      console.log("\t❌ At least one violation has error or fatal level. LintDiff will fail.");
+      // New violations with level error or fatal fail the build. If all new
+      // violations are warnings, the build passes.
+      pass = false;
+    } else { 
+      console.log("\t✅ No new violations with error or fatal level. LintDiff will pass.");
+    }
+
+    LogViolations("New violations list", newViolations);
+
     outputMarkdown += "\n";
   }
 
-  // The following errors/warnings exist before current PR submission
-  // Rule | Message | Location (link to file, line # at SHA)
+  console.log(`Existing violations: ${existingViolations.length}`);
   if (existingViolations.length > 0) {
     outputMarkdown += "**The following errors/warnings exist before current PR submission:**\n";
     if (existingViolations.length > 50) {
@@ -95,6 +102,8 @@ export async function generateLintDiffReport(
       outputMarkdown += `| ${iconFor(level)} [${code}](${getDocUrl(code)}) | ${message}<br />Location: [${getPathSegment(relativizePath(getFile(violation)))}#L${getLine(violation)}](${getFileLink(compareSha, relativizePath(getFile(violation)), getLine(violation))}) |\n`;
     }
 
+    LogViolations("Existing violations list", existingViolations);
+
     outputMarkdown += `\n`;
   }
 
@@ -104,6 +113,19 @@ export async function generateLintDiffReport(
   return pass;
 }
 
+function LogViolations(heading: string, violations: LintDiffViolation[]) {
+  console.log(`::group::${heading}`);
+  for (const violation of violations) {
+    const source = getFile(violation);
+    const line = getLine(violation);
+    console.log(`Violation: ${source}${line ? `:${line}` : ""}`);
+    console.log(`  Level: ${violation.level}`);
+    console.log(`  Code: ${violation.code}`);
+    console.log(`  Message: ${violation.message}`);
+  }
+  console.log("::endgroup::");
+}
+
 export async function generateAutoRestErrorReport(
   autoRestErrors: { result: AutorestRunResult; errors: AutoRestMessage[] }[],
   outFile: string,

eng/tools/lint-diff/src/lint-diff.ts

@@ -111,6 +111,11 @@ async function runLintDiff(
     changedFilesPath,
   );
 
+  if (beforeList.size === 0 && afterList.size === 0) {
+    console.log("No changes found. Exiting.");
+    return;
+  }
+
   // It may be possible to run these in parallel as they're running against
   // different directories.
   const beforeChecks = await runChecks(beforePath, beforeList);
@@ -145,4 +150,14 @@ async function runLintDiff(
     process.exitCode = 1;
     console.error(`Lint-diff failed. See workflow summary report in ${outFile} for details.`);
   }
+
+  if (
+    process.env.GITHUB_SERVER_URL &&
+    process.env.GITHUB_REPOSITORY &&
+    process.env.GITHUB_RUN_ID
+  ) {
+    console.log(
+    `See workflow summary at: ${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`
+    );
+  }
 }

eng/tools/lint-diff/src/processChanges.ts

@@ -41,10 +41,21 @@ export async function getRunList(
   const [beforeState, _] = await buildState(changedSpecFiles, beforePath);
   const [afterState, afterSwaggers] = await buildState(changedSpecFiles, afterPath);
   const affectedSwaggers = new Set<string>(afterSwaggers);
-
-  console.log(`affected swaggers: ${[...affectedSwaggers].join(", ")}`);
   const [beforeTagMap, afterTagMap] = reconcileChangedFilesAndTags(beforeState, afterState);
 
+
+  console.log("Before readme and tags:");
+  console.table([...beforeTagMap].map(([readme, tags]) => ({ readme, tags })), ['readme', 'tags']);
+  console.log("\n");
+
+  console.log("After readme and tags:");
+  console.table([...afterTagMap].map(([readme, tags]) => ({ readme, tags })), ['readme', 'tags']);
+  console.log("\n");
+
+  console.log("Affected swaggers:"); 
+  console.table([...affectedSwaggers].map((swagger) => ({ swagger })), ['swagger']);
+  console.log("\n");
+
   return [beforeTagMap, afterTagMap, affectedSwaggers];
 }
 

eng/tools/lint-diff/src/runChecks.ts

@@ -20,7 +20,6 @@ export async function runChecks(
 
   for (const [readme, tags] of runList.entries()) {
     const changedFilePath = join(path, readme);
-    console.log(`Linting ${changedFilePath}`);
 
     // TODO: Move this into getRunList
     let openApiType = await getOpenapiType(changedFilePath);
@@ -51,8 +50,13 @@ export async function runChecks(
         `${tagArg} ` +
         `${changedFilePath}`;
 
-      console.log(`autorest command: ${autorestCommand}`);
+      console.log(`::group::Autorest for type: ${openApiType} readme: ${readme} tag: ${tag}`);
+      console.log(`\tAutorest command: ${autorestCommand}`);
+
       const executionResult = await executeCommand(autorestCommand);
+      
+      console.log(executionResult.stderr + executionResult.stdout);
+
       const lintDiffResult = {
         autorestCommand,
         rootPath: path,
@@ -62,8 +66,10 @@ export async function runChecks(
         ...executionResult,
       };
       logAutorestExecutionErrors(lintDiffResult);
+      console.log("::endgroup::");
+
       result.push(lintDiffResult);
-      console.log("Lint diff result length:", lintDiffResult.stdout.length);
+      console.log(`\tAutorest result length: ${lintDiffResult.stderr.length + lintDiffResult.stdout.length}\n`);
     }
   }
 
@@ -109,19 +115,14 @@ export function logAutorestExecutionErrors(runResult: AutorestRunResult) {
     const stderrContainsLevelError = runResult.stderr.includes(`${autoRestPrefix}"level":"error"`);
     const stderrContainsLevelFatal = runResult.stderr.includes(`${autoRestPrefix}"level":"fatal"`);
 
-    // TODO: Clean up output formatting to be consistent with new output standards
-    console.log(
-      `Execution of AutoRest with LintDiff done. ` +
-        `Error is not null: true, ` +
-        `stdout contains AutoRest 'error': ${stdoutContainsLevelError}, ` +
-        `stdout contains AutoRest 'fatal': ${stdoutContainsLevelFatal}, ` +
-        `stderr contains AutoRest 'error': ${stderrContainsLevelError}, ` +
-        `stderr contains AutoRest 'fatal': ${stderrContainsLevelFatal}`,
-    );
-  } else {
-    // TODO: Include markdown type?
     console.log(
-      `::debug:: Execution completed with no errors for tag: ${runResult.tag}, markdown: ${runResult.readme}, rootPath: ${runResult.rootPath}`,
+      `\tAutorest completed with errors:
+\t\tExit code: ${runResult.error.code}
+\t\tError is not null: true, 
+\t\tstdout contains AutoRest 'error': ${stdoutContainsLevelError}
+\t\tstdout contains AutoRest 'fatal': ${stdoutContainsLevelFatal} 
+\t\tstderr contains AutoRest 'error': ${stderrContainsLevelError} 
+\t\tstderr contains AutoRest 'fatal': ${stderrContainsLevelFatal}`,
     );
   }
 }