From dadb5cbc30ca340158d1c9240589ffac7943a2b4 Mon Sep 17 00:00:00 2001
From: Ross Wollman <rwoll@users.noreply.github.com>
Date: Tue, 28 Dec 2021 09:56:34 -0800
Subject: [PATCH] fix(html reporter): fix too much strikethrough in diffs
 (#11115)

Textual snapshot diffs were previously broken in the HTML Report. The strikethrough'd text extended beyond the intended region.

HTML Report Before:
<img width="693" alt="Screen Shot 2021-12-27 at 4 43 35 PM" src="https://user-images.githubusercontent.com/11915034/147518750-a60f9002-6eed-48a1-a412-20fabd076fa6.png">

HTML Report After:
<img width="206" alt="Screen Shot 2021-12-27 at 4 48 37 PM" src="https://user-images.githubusercontent.com/11915034/147518762-19a4c8f9-ccc3-4a3c-a962-5a42edc6fc5d.png">

This now matches what's expected and shown in the terminal (which has always been correct):

<img width="1384" alt="Screen Shot 2021-12-27 at 4 36 29 PM" src="https://user-images.githubusercontent.com/11915034/147518799-f538259e-5a45-4d6f-916c-a12ccb620c5b.png">

NB: This MR is a workaround, but not a root cause fix. It works, but I never fully got to the root cause so a bug upstream may be required. It's unclear whether it's (1) in [`colors`](https://www.npmjs.com/package/colors), (2) in [`ansi-to-html`](https://www.npmjs.com/package/ansi-to-html), or (3) Playwright's use of the two. Since the terminal output is correct, I suspect it is in `ansi-to-html`. For example:

```js
const colors = require("colors");
const Convert = require('ansi-to-html');
const convert = new Convert();

// original (strike incorrectly wraps everything in the HTML)
console.log(convert.toHtml(colors.strikethrough("crossed out") + ' ' + colors.red("red")))
// prints: <strike>crossed out <span style="color:#A00">red<span style="color:#FFF"></span></span></strike>

// workaround
console.log(convert.toHtml(colors.reset(colors.strikethrough("crossed out")) + ' ' + colors.red("red")))
// prints: <strike>crossed out</strike> <span style="color:#A00">red<span style="color:#FFF"></span></span>
```

Fixes #11116
---
 .../playwright-test/src/matchers/golden.ts    |  2 +-
 tests/playwright-test/golden.spec.ts          |  2 +-
 tests/playwright-test/reporter-html.spec.ts   | 50 +++++++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/packages/playwright-test/src/matchers/golden.ts b/packages/playwright-test/src/matchers/golden.ts
index d88f7dcc2f..ca499fe5a4 100644
--- a/packages/playwright-test/src/matchers/golden.ts
+++ b/packages/playwright-test/src/matchers/golden.ts
@@ -228,7 +228,7 @@ function diff_prettyTerminal(diffs: diff_match_patch.Diff[]) {
         html[x] = colors.green(text);
         break;
       case DIFF_DELETE:
-        html[x] = colors.strikethrough(colors.red(text));
+        html[x] = colors.reset(colors.strikethrough(colors.red(text)));
         break;
       case DIFF_EQUAL:
         html[x] = text;
diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts
index 1d156d060e..c1800bade0 100644
--- a/tests/playwright-test/golden.spec.ts
+++ b/tests/playwright-test/golden.spec.ts
@@ -72,7 +72,7 @@ Line7`,
   expect(result.exitCode).toBe(1);
   expect(result.output).toContain('Line1');
   expect(result.output).toContain('Line2' + colors.green('2'));
-  expect(result.output).toContain('line' + colors.strikethrough(colors.red('1')) + colors.green('2'));
+  expect(result.output).toContain('line' + colors.reset(colors.strikethrough(colors.red('1'))) + colors.green('2'));
   expect(result.output).toContain('Line3');
   expect(result.output).toContain('Line5');
   expect(result.output).toContain('Line7');
diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts
index ed9d677d5e..6bf898ec4b 100644
--- a/tests/playwright-test/reporter-html.spec.ts
+++ b/tests/playwright-test/reporter-html.spec.ts
@@ -374,3 +374,53 @@ test('should render text attachments as text', async ({ runInlineTest, page, sho
   await page.click('text=example-utf16.txt');
   await expect(page.locator('.attachment-link')).toHaveText(['foo', '{"foo":1}', 'utf16 encoded']);
 });
+
+test('should strikethough textual diff', async ({ runInlineTest, showReport, page }) => {
+  const result = await runInlineTest({
+    'helper.ts': `
+      export const test = pwt.test.extend({
+        auto: [ async ({}, run, testInfo) => {
+          testInfo.snapshotSuffix = '';
+          await run();
+        }, { auto: true } ]
+      });
+    `,
+    'a.spec.js-snapshots/snapshot.txt': `old`,
+    'a.spec.js': `
+      const { test } = require('./helper');
+      test('is a test', ({}) => {
+        expect('new').toMatchSnapshot('snapshot.txt');
+      });
+    `
+  }, { reporter: 'dot,html' });
+  expect(result.exitCode).toBe(1);
+  await showReport();
+  await page.click('text="is a test"');
+  const stricken = await page.locator('css=strike').innerText();
+  expect(stricken).toBe('old');
+});
+
+test('should strikethough textual diff with commonalities', async ({ runInlineTest, showReport, page }) => {
+  const result = await runInlineTest({
+    'helper.ts': `
+      export const test = pwt.test.extend({
+        auto: [ async ({}, run, testInfo) => {
+          testInfo.snapshotSuffix = '';
+          await run();
+        }, { auto: true } ]
+      });
+    `,
+    'a.spec.js-snapshots/snapshot.txt': `oldcommon`,
+    'a.spec.js': `
+      const { test } = require('./helper');
+      test('is a test', ({}) => {
+        expect('newcommon').toMatchSnapshot('snapshot.txt');
+      });
+    `
+  }, { reporter: 'dot,html' });
+  expect(result.exitCode).toBe(1);
+  await showReport();
+  await page.click('text="is a test"');
+  const stricken = await page.locator('css=strike').innerText();
+  expect(stricken).toBe('old');
+});