node icon indicating copy to clipboard operation
node copied to clipboard

test_runner: adding colors to the xs and dots in the dot test runner

Open puskin opened this issue 1 year ago • 10 comments
trafficstars

Fixes: https://github.com/nodejs/node/issues/51770

Making sure that, when executing the dot test reporter, dots are white and Xs are red, for better visibility

puskin avatar Jun 14 '24 09:06 puskin

Review requested:

  • [ ] @nodejs/test_runner

nodejs-github-bot avatar Jun 14 '24 09:06 nodejs-github-bot

IIRC isn't there a PR already open doing this?

Maybe one or the other should be closed in favor?

avivkeller avatar Jun 14 '24 12:06 avivkeller

IIRC isn't there a PR already open doing this?

Maybe one or the other should be closed in favor?

yeah I noticed, but I pushed mine forward anyway because the creator or the other PR didn't push in the last 2 months

puskin avatar Jun 14 '24 13:06 puskin

No problem! If this gets merged, I'll close the other one.

avivkeller avatar Jun 14 '24 13:06 avivkeller

Good work on pushing the progress! (Just a thought - I think the origin author might deserve a co-author in the commit message? As the change looks quite identical).

jakecastelli avatar Jun 14 '24 15:06 jakecastelli

@puskin94 the PR changes the test_runner submodule, so the first commit message should be prefixed with test_runner and not lib. Could you please rebase and fix the message? Let me know if you need assistance with that 🙂

atlowChemi avatar Jun 14 '24 15:06 atlowChemi

@puskin94 the PR changes the test_runner submodule, so the first commit message should be prefixed with test_runner and not lib. Could you please rebase and fix the message? Let me know if you need assistance with that 🙂

@atlowChemi it took me a bit to understand the right order of execution of the commands, but we should be there now :) Thanks!

puskin avatar Jun 15 '24 08:06 puskin

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

nodejs-github-bot avatar Jun 15 '24 23:06 nodejs-github-bot

Can anyone help me out here? I see the PR is stuck and I guess it is because of broken jenkins tests... I tried to reproduce them but everything is passing locally. What should I do? How can I move?

Thanks in advance!

puskin avatar Jun 24 '24 12:06 puskin

well this is an example of a failure:

02:35:52         # Subtest: test-runner/output/dot_output_custom_columns.js
02:35:52         not ok 42 - test-runner/output/dot_output_custom_columns.js
02:35:52           ---
02:35:52           duration_ms: 265.745104
02:35:52           location: '/home/iojs/build/workspace/node-test-commit-arm/test/parallel/test-runner-output.mjs:156:5'
02:35:52           failureType: 'testCodeFailure'
02:35:52           error: |-
02:35:52             Expected values to be strictly equal:
02:35:52             + actual - expected
02:35:52             
02:35:52             + '..............................\n' +
02:35:52             +   '.........................................\n' +
02:35:52             +   '.............................\n'
02:35:52             - '[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c\n' +
02:35:52             -   '[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c\n' +
02:35:52             -   '[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c\n'
02:35:52           code: 'ERR_ASSERTION'
02:35:52           name: 'AssertionError'
02:35:52           expected: |-
02:35:52             [32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c
02:35:52             [32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c
02:35:52             [32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c[32m.c
02:35:52             
02:35:52           actual: |-
02:35:52             ..............................
02:35:52             .........................................
02:35:52             .............................
02:35:52             
02:35:52           operator: 'strictEqual'
02:35:52           stack: |-
02:35:52             assertSnapshot (/home/iojs/build/workspace/node-test-commit-arm/test/common/assertSnapshot.js:56:12)
02:35:52             async Module.spawnAndAssert (/home/iojs/build/workspace/node-test-commit-arm/test/common/assertSnapshot.js:84:3)
02:35:52             async TestContext.<anonymous> (file:///home/iojs/build/workspace/node-test-commit-arm/test/parallel/test-runner-output.mjs:150:5)
02:35:52             async Test.run (node:internal/test_runner/test:857:9)
02:35:52             async Promise.all (index 41)
02:35:52             async Suite.run (node:internal/test_runner/test:1217:7)
02:35:52             async Test.processPendingSubtests (node:internal/test_runner/test:565:7)
02:35:52           ...

it seems the CI doesn't always run in a TTY so output is not colored.

you should probably add this snippet to emulate coloring in all envs:

https://github.com/nodejs/node/blob/5905596719efc7d419b5743056f364d4d6089626/test/fixtures/test-runner/output/default_output.js#L1-L4

MoLow avatar Jun 26 '24 06:06 MoLow

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

nodejs-github-bot avatar Jul 04 '24 01:07 nodejs-github-bot

Is this MR waiting for something else before "Landing" ? I am reading the docs and it looks like I have all the required steps? Or am I missing something?

puskin avatar Jul 22 '24 07:07 puskin

Landed in 8536b674d842

MoLow avatar Jul 23 '24 09:07 MoLow