lib: fix bug with huge test output

[refack]

Fixes: #618

Checklist

• npm test passes
• contribution guidelines followed here

[refack]

/CC @nodejs/citgm @addaleax PTAL. ATM jobs stall and don't even produce the report.

[MylesBorins]

LGTM

[targos]

Shouldn't we display the last N lines of the output instead of the first? They're more likely to contain useful information about the error

[refack]

I think it's arbitrary. In the case of graseful-fs the failing test is at the top.
The bottom in actually the concatenated stderr. Which is this case is just noise.

[refack]

could we iterate over substrings rather than drop arbitrary content?

New code finished in reasonable time:

113199ms

need to review new code

[BridgeAR]

LGTM

[targos]

Ping @MylesBorins do you still want to review the new code?

[richardlau]

Add test(s)?

[refack]

citgm graceful-fs 😉
But seriously, I'll think of something...

[targos]

Rebased.
CI: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-continuous-integration-pipeline/84/console

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@4f45d04). Click here to learn what that means.
The diff coverage is 47.36%.

@@            Coverage Diff            @@
##             master     #619   +/-   ##
=========================================
  Coverage          ?   93.97%           
=========================================
  Files             ?       26           
  Lines             ?      830           
  Branches          ?        0           
=========================================
  Hits              ?      780           
  Misses            ?       50           
  Partials          ?        0

Impacted Files	Coverage Δ
lib/package-manager/test.js	98.11% <100%> (ø)
lib/reporter/logger.js	82.22% <38.46%> (ø)
lib/spawn.js	77.77% <60%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f45d04...2f7aeb7. Read the comment docs.

[targos]

CI is green but it doesn't run on Windows. I'll try to execute it on my computer today. Or maybe @richardlau if you have time?

[targos]

Or better: add a Windows machine to https://ci.nodejs.org/job/citgm-continuous-integration/

[richardlau]

Or better: add a Windows machine to https://ci.nodejs.org/job/citgm-continuous-integration/

I'll try to find time to do this later today (I need a few hours sleep first). We could also try adding Windows to the Travis testing.

[bzoz]

There is a bug with spawning and PATH (fix in #640). With that PR included, this PR works on my local box.

The CITGM tests are broken on Windows. With this PR we will be able to at least run them locally. Anyway - the CITGM is currently completely broken on Windows, this PR at least restores it to a working condition (and looks like nothing will get broken), I would be in favor of fast tracking this.

[MylesBorins]

Land away. I'm waiting for a train so can't do so right now. We can revisit the logic here later if we want to simplify it

…

On Tue, Dec 11, 2018, 9:49 AM Bartosz Sosnowski ***@***.*** wrote: There is a bug with spawning and PATH (fix in #640 <#640>). With that PR included, this PR works on my local box. The CITGM tests are broken on Windows. With this PR we will be able to at least run them locally. Anyway - the CITGM is currently completely broken on Windows, this PR at least restores it to a working condition (and looks like nothing will get broken), I would be in favor of fast tracking this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#619 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecVwuJmriLhc0nwfq42dKrW9CGwN-Sks5u38XlgaJpZM4Yc93m> .

[targos]

I'm rebasing this

[bzoz]

On my local box, tests results:

  323 passing (2m)
  27 failing

[targos]

On mine:

  333 passing (3m)
  17 failing

[targos]

Rebased with 89c487a to fix the linter issue

[targos]

@bzoz LGTY? Can I land this as-is?

[bzoz]

Yes