Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v25.1.0 text reporter generates to much white space #9441

Closed
folderol4 opened this issue Jan 22, 2020 · 20 comments
Closed

v25.1.0 text reporter generates to much white space #9441

folderol4 opened this issue Jan 22, 2020 · 20 comments

Comments

@folderol4
Copy link

🐛 Bug Report

Testing the output from the text reporter between v24.9.0 and v25.1.0 I noticed two changes in behavior that make the report harder to read. It maybe an istanbul issue, but I wanted to start here.

  1. The uncovered line #s are no longer truncated with ....
  2. If the number of uncovered line #s is very long the value will wrap. If 1 file causes 5 line wraps then every file that is reported is given the same 5 line wraps. Consistency is usually great, but in this case it results in the report being mostly white space and harder to read.

To Reproduce

  • Have a file that reports many many individual uncovered lines (100 or so individual uncovered lines should work).
  • Have 10 or so other files that have minimal to no uncovered lines.

Steps to reproduce the behavior:

  • Run the tests with the text reporter turned on.
  • Notice the huge gaps in between each file with minimal to no uncovered lines.

Expected behavior

The number of lines to wrap for each file in the text report should be calculated for each file and not use the largest line wrap count found.

envinfo

  System:
    OS: macOS 10.15.2
    CPU: (8) x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
    Memory: 943.94 MB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 12.14.1 - /usr/local/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 6.13.4 - /usr/local/bin/npm
  Managers:
    Homebrew: 2.2.1 - /usr/local/bin/brew
    pip3: 9.0.1 - ~/.pyenv/shims/pip3
    RubyGems: 3.0.3 - /usr/bin/gem
  Utilities:
    Make: 3.81 - /usr/bin/make
    GCC: 4.2.1 - /usr/bin/gcc
    Git: 2.19.1 - /usr/local/bin/git
    Clang: 1100.0.33.17 - /usr/bin/clang
    Subversion: 1.10.4 - /usr/bin/svn
  Servers:
    Apache: 2.4.41 - /usr/sbin/apachectl
  Virtualization:
    VirtualBox: 6.0.14 - /usr/local/bin/vboxmanage
  IDEs:
    Nano: 2.0.6 - /usr/bin/nano
    Vim: 8.1 - /usr/bin/vim
    Xcode: /undefined - /usr/bin/xcodebuild
  Languages:
    Bash: 3.2.57 - /bin/bash
    Go: 1.13.6 - /usr/local/go/bin/go
    Java: 10.0.1 - /usr/bin/javac
    Perl: 5.18.4 - /usr/bin/perl
    PHP: 7.3.9 - /usr/bin/php
    Python: 2.7.15 - /Users/michael.smith/.pyenv/shims/python
    Python3: 3.5.7 - /Users/michael.smith/.pyenv/shims/python3
    Ruby: 2.6.3 - /usr/bin/ruby
  Databases:
    SQLite: 3.28.0 - /usr/bin/sqlite3
  Browsers:
    Chrome: 79.0.3945.130
    Firefox: 72.0.1
    Safari: 13.0.4
@SimenB
Copy link
Member

SimenB commented Jan 22, 2020

This is a change in istanbul-reports@3 yeah, so you should open up an issue over there 🙂

EDIT: or at least I think so - could you include screenshots of before and after?

@SimenB SimenB closed this as completed Jan 22, 2020
@SimenB SimenB reopened this Jan 22, 2020
@SimenB
Copy link
Member

SimenB commented Jan 22, 2020

No, this is Jest's fault. We try to measure the width of process.stdout.columns, but fallback to Infinity, which causes the wrapping. This was done in #9192 due to the default behavior cutting the filenames, which I thought looked odd.

https://github.com/facebook/jest/blob/cdce184c549c17ac017253101dbeeee071bd4449/packages/jest-reporters/src/coverage_reporter.ts#L108

Will try to come up with a better solution. It's somewhat weird process.stdout.columns isn't picked up though, it shouldn't really wrap

@SimenB
Copy link
Member

SimenB commented Jan 22, 2020

@michael-smith-tanium what do you get if you run node -p process.stdout.columns in your shell?

@folderol4
Copy link
Author

folderol4 commented Jan 22, 2020

Sure. Would you still like before and after screenshots?

➜ git:(update-jest-deps) node -p process.stdout.columns
202

@SimenB
Copy link
Member

SimenB commented Jan 22, 2020

Would you still like before and after screenshots?

Yes please 🙂

Super odd it says 202 but is not limited to it... What happens if you remove the maxCols from node_modules/@jest/reporters/build/coverage_reporter.js? Just remove the entire {maxCols: process.stdout.columns || Infinity}. Istanbul itself should default to 80

@folderol4
Copy link
Author

folderol4 commented Jan 22, 2020

24.9.0:
24 9 0

25.1.0:
25 1 0

@folderol4
Copy link
Author

folderol4 commented Jan 22, 2020

Removing that line does default things to 80. The file names and uncovered line #s columns are truncated but it works. If it helps, for some reason process.stdout.columns is resolving to undefined inside of that coverage_reporter.js file.

@folderol4
Copy link
Author

node -p process.stdout.columns is reporting 202, but hardcoding that in the report still wraps. I stopped wrapping at 160. Seeing everything not wrapped I noticed that the summary text is printing in the middle of the text report. That maybe a separate issue.

@SimenB
Copy link
Member

SimenB commented Jan 22, 2020

Hmm, interesting... So it goes beyond its column width even though we try to max it at process.stdout.columns? @coreyfarrell seen this before?

@folderol4
Copy link
Author

Just to be clear in case I am causing confusion. When I inspect process.stdout.columns in node_modules/@jest/reporters/build/coverage_reporter.js it is undefined. So it isn't going beyond what is specified it is just falling back to Infinity which is probably not desired.

The question is why my process.stdout.columns is undefined. When I run that same command with node -p I get a value. Maybe it is possible I have something specific to my environment that is wiping the columns value.

@SimenB
Copy link
Member

SimenB commented Jan 22, 2020

Oh, I read "but hardcoding that in the report still wraps" and thought it was still broken?
It being undefined is weird though - would you be able to put together a reproduction?

@coreyfarrell
Copy link
Contributor

istanbul-reports defaults to maxCols: process.stdout.columns || 80 but jest provides process.stdout.columns || Infinity. I'd suggest maybe 120 might be a better fallback than Infinity. As for process.stdout.columns being undefined I have no ideas. This must be something in jest, the repository in question or the environment which tests are being run (example if stdout is being redirected to a file).

@folderol4
Copy link
Author

Is there something I can provide here that could help or do you have what you need for now?

@SimenB
Copy link
Member

SimenB commented Feb 6, 2020

Sorry, haven't had the time to look into this.

I'm confused about this statement:

node -p process.stdout.columns is reporting 202, but hardcoding that in the report still wraps.

If you hard code maxCols: 202 it wraps in your terminal? So even if process.stdout.columns !== undefined it would still be broken?

@gregid
Copy link

gregid commented Mar 1, 2020

I am experiencing the same problem but only when running on gitlab CI, the Uncovered Line #s never truncates making the report fit the largest item.

@folderol4
Copy link
Author

I bumped to v25.5.4 and am not seeing the issue as prominently as in 25.1.0. I'm going to close this.

@gregid
Copy link

gregid commented Jun 5, 2020

Updated to version 26.0.1 the problem still persists (tested on gitlab CI, image: node:latest)

@folderol4
Copy link
Author

Feel free to reopen if you'd like @gregid. You can respond to SimenB's last question that I didn't answer.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
@SimenB
Copy link
Member

SimenB commented Feb 24, 2022

For anyone coming back to this issue later - in #9442 I tested removing the width, and a bunch of file names now get abbreviated, which imo is a worse experience.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants