fix(junit): env variable should take precedence over config (#32842)

Fixes https://github.com/microsoft/playwright/issues/32826

packages/playwright/src/reporters/base.ts

@@ -567,28 +567,24 @@ export function resolveOutputFile(reporterName: string, options: {
     }
   }):  { outputFile: string, outputDir?: string } |undefined {
   const name = reporterName.toUpperCase();
-  let outputFile;
+  let outputFile = resolveFromEnv(`PLAYWRIGHT_${name}_OUTPUT_FILE`);
-  if (options.outputFile)
+  if (!outputFile && options.outputFile)
     outputFile = path.resolve(options.configDir, options.outputFile);
-  if (!outputFile)
-    outputFile = resolveFromEnv(`PLAYWRIGHT_${name}_OUTPUT_FILE`);
-  // Return early to avoid deleting outputDir.
   if (outputFile)
     return { outputFile };
 
-  let outputDir;
+  let outputDir = resolveFromEnv(`PLAYWRIGHT_${name}_OUTPUT_DIR`);
-  if (options.outputDir)
+  if (!outputDir && options.outputDir)
     outputDir = path.resolve(options.configDir, options.outputDir);
-  if (!outputDir)
-    outputDir = resolveFromEnv(`PLAYWRIGHT_${name}_OUTPUT_DIR`);
   if (!outputDir && options.default)
     outputDir = resolveReporterOutputPath(options.default.outputDir, options.configDir, undefined);
+  if (!outputDir)
+    outputDir = options.configDir;
 
-  if (!outputFile) {
+  const reportName = process.env[`PLAYWRIGHT_${name}_OUTPUT_NAME`] ?? options.fileName ?? options.default?.fileName;
-    const reportName = options.fileName ?? process.env[`PLAYWRIGHT_${name}_OUTPUT_NAME`] ?? options.default?.fileName;
   if (!reportName)
     return undefined;
-    outputFile = path.resolve(outputDir ?? process.cwd(), reportName);
+  outputFile = path.resolve(outputDir, reportName);
-  }
+
   return { outputFile, outputDir };
 }

tests/playwright-test/reporter-junit.spec.ts

@@ -487,10 +487,6 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       test('with env var should create relative to cwd', async ({ runInlineTest }, testInfo) => {
         const result = await runInlineTest({
           'foo/package.json': `{ "name": "foo" }`,
-          // unused config along "search path"
-          'foo/bar/playwright.config.js': `
-            module.exports = { projects: [ {} ] };
-          `,
           'foo/bar/baz/tests/a.spec.js': `
             import { test, expect } from '@playwright/test';
             const fs = require('fs');
@@ -508,10 +504,6 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       test('support PLAYWRIGHT_JUNIT_OUTPUT_FILE', async ({ runInlineTest }, testInfo) => {
         const result = await runInlineTest({
           'foo/package.json': `{ "name": "foo" }`,
-          // unused config along "search path"
-          'foo/bar/playwright.config.js': `
-            module.exports = { projects: [ {} ] };
-          `,
           'foo/bar/baz/tests/a.spec.js': `
             import { test, expect } from '@playwright/test';
             const fs = require('fs');
@@ -526,6 +518,27 @@ for (const useIntermediateMergeReport of [false, true] as const) {
         expect(fs.existsSync(testInfo.outputPath('foo', 'bar', 'baz', 'my-report.xml'))).toBe(true);
       });
 
+      test('PLAYWRIGHT_JUNIT_OUTPUT_FILE should take precedence over config', async ({ runInlineTest }, testInfo) => {
+        const result = await runInlineTest({
+          'package.json': `{ "name": "foo" }`,
+          'bar/playwright.config.js': `
+            module.exports = {
+              reporter: [['junit', { outputFile: 'results.xml' }]],
+              projects: [ {} ]
+            };
+          `,
+          'bar/baz/tests/a.spec.js': `
+            import { test, expect } from '@playwright/test';
+            const fs = require('fs');
+            test('pass', ({}, testInfo) => {
+            });
+          `
+        }, { 'config': 'bar/playwright.config.js' }, { 'PLAYWRIGHT_JUNIT_OUTPUT_FILE': 'bar/my-report.xml' });
+        expect(result.exitCode).toBe(0);
+        expect(result.passed).toBe(1);
+        expect(fs.existsSync(testInfo.outputPath('bar', 'my-report.xml'))).toBe(true);
+      });
+
       test('support PLAYWRIGHT_JUNIT_OUTPUT_DIR and PLAYWRIGHT_JUNIT_OUTPUT_NAME', async ({ runInlineTest }, testInfo) => {
         const result = await runInlineTest({
           'playwright.config.js': `
@@ -542,6 +555,27 @@ for (const useIntermediateMergeReport of [false, true] as const) {
         expect(result.passed).toBe(1);
         expect(fs.existsSync(testInfo.outputPath('foo', 'bar', 'baz', 'my-report.xml'))).toBe(true);
       });
+
+      test('PLAYWRIGHT_JUNIT_OUTPUT_NAME should take precedence over config', async ({ runInlineTest }, testInfo) => {
+        const result = await runInlineTest({
+          'package.json': `{ "name": "foo" }`,
+          'bar/playwright.config.js': `
+            module.exports = {
+              reporter: [['junit', {}]],
+              projects: [ {} ]
+            };
+          `,
+          'bar/baz/tests/a.spec.js': `
+            import { test, expect } from '@playwright/test';
+            const fs = require('fs');
+            test('pass', ({}, testInfo) => {
+            });
+          `
+        }, { 'config': 'bar/playwright.config.js' }, { 'PLAYWRIGHT_JUNIT_OUTPUT_NAME': 'foo/my-report.xml' });
+        expect(result.exitCode).toBe(0);
+        expect(result.passed).toBe(1);
+        expect(fs.existsSync(testInfo.outputPath('bar', 'foo', 'my-report.xml'))).toBe(true);
+      });
     });
 
     test('testsuites time is test run wall time', async ({ runInlineTest }) => {