From 3170963f42f3d564c13acbbfdc5514b18b57282a Mon Sep 17 00:00:00 2001
From: David Paquette <david.paquette@usask.ca>
Date: Thu, 14 Sep 2023 13:58:09 -0600
Subject: [PATCH] fix: render JUnit attachment paths relative to outputFile
 (#27024)

---
 packages/playwright/src/reporters/junit.ts   | 22 ++++++++++----
 tests/playwright-test/reporter-junit.spec.ts | 32 ++++++++++++++++++++
 2 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/packages/playwright/src/reporters/junit.ts b/packages/playwright/src/reporters/junit.ts
index c176d8680c..6adb7d4168 100644
--- a/packages/playwright/src/reporters/junit.ts
+++ b/packages/playwright/src/reporters/junit.ts
@@ -31,6 +31,7 @@ class JUnitReporter extends EmptyReporter {
   private totalFailures = 0;
   private totalSkipped = 0;
   private outputFile: string | undefined;
+  private resolvedOutputFile: string | undefined;
   private stripANSIControlSequences = false;
 
   constructor(options: { outputFile?: string, stripANSIControlSequences?: boolean } = {}) {
@@ -51,6 +52,10 @@ class JUnitReporter extends EmptyReporter {
     this.suite = suite;
     this.timestamp = new Date();
     this.startTime = monotonicTime();
+    if (this.outputFile) {
+      assert(this.config.configFile || path.isAbsolute(this.outputFile), 'Expected fully resolved path if not using config file.');
+      this.resolvedOutputFile = this.config.configFile ? path.resolve(path.dirname(this.config.configFile), this.outputFile) : this.outputFile;
+    }
   }
 
   override async onEnd(result: FullResult) {
@@ -79,11 +84,9 @@ class JUnitReporter extends EmptyReporter {
 
     serializeXML(root, tokens, this.stripANSIControlSequences);
     const reportString = tokens.join('\n');
-    if (this.outputFile) {
-      assert(this.config.configFile || path.isAbsolute(this.outputFile), 'Expected fully resolved path if not using config file.');
-      const outputFile = this.config.configFile ? path.resolve(path.dirname(this.config.configFile), this.outputFile) : this.outputFile;
-      await fs.promises.mkdir(path.dirname(outputFile), { recursive: true });
-      await fs.promises.writeFile(outputFile, reportString);
+    if (this.resolvedOutputFile) {
+      await fs.promises.mkdir(path.dirname(this.resolvedOutputFile), { recursive: true });
+      await fs.promises.writeFile(this.resolvedOutputFile, reportString);
     } else {
       console.log(reportString);
     }
@@ -190,7 +193,14 @@ class JUnitReporter extends EmptyReporter {
       for (const attachment of result.attachments) {
         if (!attachment.path)
           continue;
-        const attachmentPath = path.relative(this.config.rootDir, attachment.path);
+
+        let attachmentPath = path.relative(this.config.rootDir, attachment.path);
+        try {
+          if (this.resolvedOutputFile)
+            attachmentPath = path.relative(path.dirname(this.resolvedOutputFile), attachment.path);
+        } catch {
+          systemOut.push(`\nWarning: Unable to make attachment path ${attachment.path} relative to report output file ${this.outputFile}`);
+        }
 
         try {
           await fs.promises.access(attachment.path);
diff --git a/tests/playwright-test/reporter-junit.spec.ts b/tests/playwright-test/reporter-junit.spec.ts
index c3fe77fd05..cc4175219e 100644
--- a/tests/playwright-test/reporter-junit.spec.ts
+++ b/tests/playwright-test/reporter-junit.spec.ts
@@ -307,6 +307,38 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       expect(result.exitCode).toBe(0);
     });
 
+    test('should render attachment paths relative to report file when report file name is specified', async ({ runInlineTest }, testInfo) => {
+      test.skip(useIntermediateMergeReport, 'Blob report hashes attachment paths');
+      const result = await runInlineTest({
+        'project/playwright.config.ts': `
+            module.exports = { reporter: [['junit', { outputFile: '../my-report/junit/a.xml' }]] };
+        `,
+        'project/a.test.js': `
+          import { test, expect } from '@playwright/test';
+          test.use({ screenshot: 'on' });
+          test('one', async ({ page }, testInfo) => {
+            await page.setContent('hello');
+            const file = testInfo.outputPath('file.txt');
+            require('fs').writeFileSync(file, 'my file', 'utf8');
+            testInfo.attachments.push({ name: 'my-file', path: file, contentType: 'text/plain' });
+            console.log('log here');
+          });
+        `,
+      }, { reporter: '', config: './project/playwright.config.ts' });
+
+      expect(result.exitCode).toBe(0);
+      expect(fs.existsSync(testInfo.outputPath(path.join('my-report', 'junit', 'a.xml')))).toBeTruthy();
+      const xml = parseXML(fs.readFileSync(testInfo.outputPath(path.join('my-report', 'junit', 'a.xml'))).toString());
+      const testcase = xml['testsuites']['testsuite'][0]['testcase'][0];
+      expect(testcase['system-out'].length).toBe(1);
+      expect(testcase['system-out'][0].trim()).toBe([
+        `log here`,
+        `\n[[ATTACHMENT|..${path.sep}..${path.sep}test-results${path.sep}a-one${path.sep}file.txt]]`,
+        `\n[[ATTACHMENT|..${path.sep}..${path.sep}test-results${path.sep}a-one${path.sep}test-finished-1.png]]`,
+      ].join('\n'));
+      expect(result.exitCode).toBe(0);
+    });
+
     function parseXML(xml: string): any {
       let result: any;
       xml2js.parseString(xml, (err, r) => result = r);