playwright icon indicating copy to clipboard operation
playwright copied to clipboard

[Feature]: support stripANSIControlSequences in json reporter

Open gofr opened this issue 1 year ago • 5 comments
trafficstars

Version

1.48.2

Steps to reproduce

  1. Create simple test:
import { test, expect } from '@playwright/test';

test('color', () => {
expect('foo').toBe('bar');
})
  1. Run Playwright with colors turned off: FORCE_COLOR=0 DEBUG_COLORS=false npx playwright test

Expected behavior

I expect the output to lack any colors.

Actual behavior

Playwright's own output has no colors, but the output from expect does have colors. This part:

Error: expect(received).toBe(expected) // Object.is equality

Expected: "bar"
Received: "foo"

Additional context

This seems related to #32543.

https://github.com/microsoft/playwright/pull/32764 changed DEBUG_COLORS to respect the environment, here (on line 40): https://github.com/microsoft/playwright/blob/ecf6f27159de43cc12964e190e5241b4382abff5/packages/playwright/src/runner/workerHost.ts#L39-L40

If I make the same adjustment to FORCE_COLOR on line 39, the colors from expect disappear too.

Environment

System:
    OS: Linux 5.15 Ubuntu 22.04.5 LTS 22.04.5 LTS (Jammy Jellyfish)
    CPU: (8) x64 Intel(R) Core(TM) i5-10310U CPU @ 1.70GHz
    Memory: 1.29 GB / 11.69 GB
    Container: Yes
  Languages:
    Bash: 5.1.16 - /usr/bin/bash
  npmPackages:
    @playwright/test: ^1.48.2 => 1.48.2

gofr avatar Nov 19 '24 11:11 gofr

@gofr Playwright wants expect to have color, even when running without tty or color support. This way, reports like an html report will still get nice expect error messages. What's your usecase for disabling expect coloring?

dgozman avatar Nov 19 '24 13:11 dgozman

@dgozman I'm using the json reporter and am also getting the ANSI color codes in the error strings there.

That wasn't a problem until I ran into https://github.com/jestjs/jest/issues/15384 which causes my JSON to be invalid.

I guess an alternative would be to add the stripANSIControlSequences option that the junit reporter has to json too.

gofr avatar Nov 19 '24 13:11 gofr

@gofr Thank you for the details!

How does stripping ANSI control sequences help with unpaired surrogates in the text? These two problems look unrelated to me. Could you please explain in more details?

As a workaround, post-process your json with a regex to strip ansi. We do that quite often, here is an example from our source code:

const ansiRegex = new RegExp('([\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~])))', 'g');
export function stripAnsiEscapes(str: string): string {
  return str.replace(ansiRegex, '');
}

dgozman avatar Nov 20 '24 08:11 dgozman

@dgozman Since Jest's diffing doesn't handle surrogates, it can put ANSI control sequences in the middle of a surrogate pair. So my JSON ends up containing an invalid string like this:

"Expected: \u001b[32m\"\ud83d\u001b[7m\ude04\u001b[27m\"\u001b[39m"

Stripping the ANSI makes it valid again:

"Expected: \"\ud83d\ude04\""

In my case this is just a workaround for the Jest bug. I also thought not being able to fully turn off colors was a bug, but if that's working as intended I guess this is more of a feature request.

Stripping the ANSI myself is an option. Thanks for that example. 🙂

gofr avatar Nov 20 '24 10:11 gofr

@gofr Thank you for the explanation, this makes sense now! I'll repurpose this issue into stripANSIControlSequences feature request.

dgozman avatar Nov 21 '24 09:11 dgozman

Why was this issue closed?

Thank you for your involvement. This issue was closed due to limited engagement (upvotes/activity), lack of recent activity, and insufficient actionability. To maintain a manageable database, we prioritize issues based on these factors.

If you disagree with this closure, please open a new issue and reference this one. More support or clarity on its necessity may prompt a review. Your understanding and cooperation are appreciated.

pavelfeldman avatar Sep 04 '25 01:09 pavelfeldman