[v9.x backport] assert: fix diff color output

[codebytere]

Original PR: 19464

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[targos]

CI: https://ci.nodejs.org/job/node-test-pull-request/15278/

Edit: ARM test re-run: https://ci.nodejs.org/job/node-test-binary-arm/1675/

[ryzokuken]

@targos the rerun passed, right? Why didn't the GH interface update? Rerunning the full CI just to make sure.

That said, I hope the PR contents are ready to be merged, given that you've approved them?

[ryzokuken]

CI Rerun: https://ci.nodejs.org/job/node-test-pull-request/15372/

[ryzokuken]

LinuxOne failed? Lemme check if was a flake, it has to be.

[ryzokuken]

The failure, BTW.

-------------------------------------------------- label=rhel72-s390x --------------------------------------------------
URL        https://ci.nodejs.org/job/node-test-commit-linuxone/label=rhel72-s390x/2093/console
Reason
  not ok 481 parallel/test-eslint-require-buffer
    ---
    duration_ms: 0.862
    severity: fail
    exitcode: 1
    stack: |-
      /data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:148
              throw err;
              ^

      AssertionError [ERR_ASSERTION]: 'Use const { Buffer } = require(\'buffer\'); at the beginning of this file' === 'Use const Buffer = require(\'buffer\').Buffer; at the beginning of this file' ('Use const { Buffer } = require(\'buffer\'); at the beginning of this file' === 'Use const Buffer = require(\'buffer\').Buffer; at the beginning of this file')
          at assertMessageMatches (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:442:24)
          at testInvalidTemplate (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:494:29)
          at Function.RuleTester.it (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:581:25)
          at Function.itDefaultHandler (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:143:23)
          at test.invalid.forEach.invalid (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:580:32)
          at Array.forEach (<anonymous>)
          at Function.RuleTester.describe (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:579:30)
          at Function.describeDefaultHandler (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:160:19)
          at Function.RuleTester.describe (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:578:24)
          at Function.describeDefaultHandler (/data/iojs/build/workspace/node-test-commit-linuxone/label/rhel72-s390x/tools/node_modules/eslint/lib/testers/rule-tester.js:160:19)
    ...

[ryzokuken]

LinuxOne rebuild: https://ci.nodejs.org/job/node-test-commit-linuxone/2095/

[ryzokuken]

Okay, so the rebuild failed again.

@codebytere maybe there's something wrong with the code. Could you take a look? Look if there are any buffer requires that got chopped up and are now failing on us.

P.S. This still looks a lot like a flake tbqh because it's not failing on any other platforms (it should've if it were real, right?) but let's still check.

[codebytere]

@ryzokuken yes, looking now!

[ryzokuken]

@codebytere wait a second, why are we still backporting to v9.x?

[codebytere]

@ryzokuken i was actually unsure about this, i wasn't sure if you were only backporting security fixes to 9.x but it was labeled backport-requested so ¯\(ツ)/¯

[apapirovski]

@codebytere I think those are old at this point. I would just go straight to 8.x and ignore the 9.x requests. (This particular one probably doesn't land on 8.x since I think we introduced the colour code in 9.x?)

[ryzokuken]

@codebytere v9.x is in maintenance, so I don't think we need to. v9.x reaches EOL within June anyway.

[ryzokuken]

Plus I ran a silent rerun, it failed again. This might involve more work than anticipated anyway.

[codebytere]

Sounds good to me, closing this out in that case. For future backports, would be be more helpful for me to focus on LTS or current?

[ryzokuken]

Yeah, make sure to make PRs for active releases only (LTSes are helpful because they're active for a longer duration) and try to get people to merge them as soon as possible if you can. I tend to keep pinging people here or on chat, which seems to work pretty well. Doing this would ensure that none of your effort goes to waste 😅.

Again, thanks for the work you put in and sorry this couldn't be merged.

[richardlau]

Saw this after glancing through the #node-build logs. To follow up re. the failing test:

 not ok 481 parallel/test-eslint-require-buffer
...
      AssertionError [ERR_ASSERTION]: 'Use const { Buffer } = require(\'buffer\'); at the beginning of this file' === 'Use const Buffer = require(\'buffer\').Buffer; at the beginning of this file' ('Use const { Buffer } = require(\'buffer\'); at the beginning of this file' === 'Use const Buffer = require(\'buffer\').Buffer; at the beginning of this file')

On master test-eslint-require-buffer was broken for a long time in a similar way but wasn't noticed because the eslint tests were not actually being run. This was fixed as part of #20372.

My guess, without digging into the commit history, is that there's probably a PR or two that would need to be backported but probably isn't worth it for v9.x. I don't know why it would only fail on one platform.