#3374 If path name is very long then pytest will split name for several lines #3383

feuillemorte · 2018-04-10T19:27:59Z

Long names:

Long name full line:

Long name full line + 1 letter:

Very beautiful when progress prints in every line but it is a problem with progress count: it counts only after run a test (after report write a dot), so, there is a problem:

coveralls · 2018-04-10T19:56:09Z

Coverage increased (+0.03%) to 92.766% when pulling b9ebffb on feuillemorte:3374-long-filenames into 372bcdb on pytest-dev:features.

nicoddemus · 2018-04-10T21:37:33Z

Thanks @feuillemorte for the PR!

Personally I prefer to just write the full path name and let the terminal wrap that around for us if needed, otherwise this makes it hard to copy/paste the test name which is useful on occasion:

tests/test_very_long_name_test_very_long_name_test_very_long_name_test_very_long_nam
e_test_very_long_name.py ..                                                   [ 50%]

If we write to the edge of the screen in a way that if we would mess with the alignment of the progress indicator, we should start a new line instead. Using the example from #3374:

tests\test_general\test_find_last_operator\test_correct_expressions.py . [ 77%]
.......                                                                  [ 93%]
tests\test_general\test_find_last_operator\test_incorrect_expressions.py 
.                                                                        [ 95%]
tests\test_general\test_find_last_operator\test_monomials.py ..          [100%]

What do others think?

cc @InDevRus

feuillemorte · 2018-04-13T15:10:59Z

@nicoddemus
I fixed your comment and now it looks more convenient. Also I deleted test because I don't know how to set terminal width. If you can give me advice how to write a test, please, do it.

Look at the pictures how it works now:

Long names:

Long name full line:

Long name full line + 1 symbol:

But I found a little problem here:

I have 99% at 2 lines and 0% at the beginning. Is it a bug?

feuillemorte · 2018-04-13T15:40:46Z

Fixed some bug:

before:

after:

nicoddemus

Thanks @feuillemorte and sorry for the delay.

Besides my comments, please add a test as well.

nicoddemus · 2018-04-23T22:50:41Z

_pytest/terminal.py

@@ -216,6 +217,11 @@ def write_fspath_result(self, nodeid, res):
            fspath = self.startdir.bestrelpath(fspath)
            self._tw.line()
            self._tw.write(fspath + " ")
+            width = self._tw.fullwidth - self._tw.chars_on_current_line
+            if 0 < width < 10 or self._tw.chars_on_current_line % self._screen_width > self._screen_width - 10:


Why 10? I suggest using a variable with a descriptive name instead. The same can be done for the entire conditional, for example:

something_descriptive = 0 < width < 10 or self._tw.chars_on_current_line % self._screen_width > self._screen_width - 10

Also can you explain this logic here?

10 symbols because of this line:
.py [ 80%]
this is magic 10 symbols :)
Also all magic symbols was got by testing in terminal

logic:
if you have long name and remaining widht less then 10 symbols, then add new line.
OR
if you have long name (3 lines) and width of tail of this name more then screen width - 10 symbols, then also add new line

How I can write test on it? Does it have a value like
tempdir.runtests(--screen-width=120)
?

10 symbols because of this line:
.py [ 80%]
this is magic 10 symbols :)

Ahh I see, thanks. In those cases it is usually better to put this in a descriptive constant:

_PROGRESS_LENGTH = len(' [100%]') _MAX_LEN_LEFT = len('.py [ 80%]')

logic:
if you have long name and remaining widht less then 10 symbols, then add new line.
OR
if you have long name (3 lines) and width of tail of this name more then screen width - 10 symbols, then also add new line

Could you please separate each condition in their own variable with descriptive name? A descriptive variable name often is enough to explain a complex situation and avoid having to write comments or explain it to people.

How I can write test on it? Does it have a value like
tempdir.runtests(--screen-width=120)
?

I don't think so, but I believe testdir uses a hard coded value internally. You can try to match against the entire line instead of using glob wildcards.

I believe testdir uses a hard coded value internally.

I tried to run my test and it failed because this value was changed and my test with long name on 3 lines become to test with 1 line. So that's why I deleted it.

Also, could you please some test on your terminal after my fix and before merging? I'm some afraid of my magic constants :) It looks nice in my terminals (I tried 2 terminals), but who knows how it will not on my machine.

Hey @feuillemorte, did you push more commits? I don't see them here.

@nicoddemus I fixed your comments, but I can't push because I can't to write a test. I try to get an object of terminal and get screen width. It says me 80 symbols, but actually it's wrong value inside runpytest() function (159 symbols):
(sorry, I can't paste image because some amazon ip's blocked in Russia, please, look url)

test_01_very_long_name_very_long_name_very_long_name_very_long_name_very_long_name_very_long_name_very_l=""" import pytest from _pytest.terminal import TerminalReporter term = TerminalReporter(pytest.config) assert term._screen_width == 1 @pytest.mark.parametrize('i', range(10)) def test_bar(i): assert 1 """,

test_01_very_long_name_very_long_name_very_long_name_very_long_name_very_long_name_very_long_name_very_l.py:4: in <module> assert term._screen_width == 1 E assert 80 == 1

output:
https://pp.userapi.com/c824503/v824503903/11fc2b/8XBN-QcJDh0.jpg

@nicoddemus I pushed some code but without tests

nicoddemus · 2018-04-23T22:51:53Z

_pytest/terminal.py

@@ -355,7 +361,10 @@ def pytest_runtest_logfinish(self, nodeid):
                self._write_progress_information_filling_space()
            else:
                past_edge = self._tw.chars_on_current_line + self._PROGRESS_LENGTH + 1 >= self._screen_width
-                if past_edge:
+                width = self._tw.chars_on_current_line % self._screen_width + 9


Again a magic constant, please use a named constant instead. 😁

I see 9 should be _MAX_LEN_LEFT - 1 if we add a constant for it, is that right?

nicoddemus

@feuillemorte thanks again for following up! This is shaping up nicely, I just think we should work a bit on improving the code.

Sorry if I seem picky, but the readability in this kind of code (which seems simple but is kind of tricky) is very important for future maintenance.

Thanks for your patience! 👍

nicoddemus · 2018-04-25T00:42:31Z

_pytest/terminal.py

@@ -216,6 +217,11 @@ def write_fspath_result(self, nodeid, res):
            fspath = self.startdir.bestrelpath(fspath)
            self._tw.line()
            self._tw.write(fspath + " ")
+            width = self._tw.fullwidth - self._tw.chars_on_current_line
+            if 0 < width < 10 or self._tw.chars_on_current_line % self._screen_width > self._screen_width - 10:


10 symbols because of this line:
.py [ 80%]
this is magic 10 symbols :)

Ahh I see, thanks. In those cases it is usually better to put this in a descriptive constant:

_PROGRESS_LENGTH = len(' [100%]') _MAX_LEN_LEFT = len('.py [ 80%]')

nicoddemus · 2018-04-25T00:43:38Z

_pytest/terminal.py

@@ -216,6 +217,11 @@ def write_fspath_result(self, nodeid, res):
            fspath = self.startdir.bestrelpath(fspath)
            self._tw.line()
            self._tw.write(fspath + " ")
+            width = self._tw.fullwidth - self._tw.chars_on_current_line


width is misleading, I suggest width_remaining

nicoddemus · 2018-04-25T00:59:20Z

_pytest/terminal.py

+            if 0 < width < 10 or self._tw.chars_on_current_line % self._screen_width > self._screen_width - 10:
+                self._tw.line()
+            elif width < 0:
+                self.is_fspath_extra_width = True


I suggest _is_fspath_past_edge (if I understand the code correctly)

nicoddemus · 2018-04-25T01:06:27Z

_pytest/terminal.py

@@ -216,6 +217,11 @@ def write_fspath_result(self, nodeid, res):
            fspath = self.startdir.bestrelpath(fspath)
            self._tw.line()
            self._tw.write(fspath + " ")
+            width = self._tw.fullwidth - self._tw.chars_on_current_line
+            if 0 < width < 10 or self._tw.chars_on_current_line % self._screen_width > self._screen_width - 10:


logic:
if you have long name and remaining widht less then 10 symbols, then add new line.
OR
if you have long name (3 lines) and width of tail of this name more then screen width - 10 symbols, then also add new line

Could you please separate each condition in their own variable with descriptive name? A descriptive variable name often is enough to explain a complex situation and avoid having to write comments or explain it to people.

nicoddemus · 2018-04-25T01:07:10Z

_pytest/terminal.py

@@ -355,7 +361,10 @@ def pytest_runtest_logfinish(self, nodeid):
                self._write_progress_information_filling_space()
            else:
                past_edge = self._tw.chars_on_current_line + self._PROGRESS_LENGTH + 1 >= self._screen_width
-                if past_edge:
+                width = self._tw.chars_on_current_line % self._screen_width + 9


I see 9 should be _MAX_LEN_LEFT - 1 if we add a constant for it, is that right?

nicoddemus · 2018-04-25T01:11:02Z

_pytest/terminal.py

@@ -216,6 +217,11 @@ def write_fspath_result(self, nodeid, res):
            fspath = self.startdir.bestrelpath(fspath)
            self._tw.line()
            self._tw.write(fspath + " ")
+            width = self._tw.fullwidth - self._tw.chars_on_current_line
+            if 0 < width < 10 or self._tw.chars_on_current_line % self._screen_width > self._screen_width - 10:


How I can write test on it? Does it have a value like
tempdir.runtests(--screen-width=120)
?

I don't think so, but I believe testdir uses a hard coded value internally. You can try to match against the entire line instead of using glob wildcards.

nicoddemus · 2018-05-01T15:22:47Z

@feuillemorte thanks a lot for the follow up! 👍

I've tried your branch on Windows and the results are not very good unfortunately.

Here's two screenshots using the code from your branch, one for the native command prompt and another for Cmder (which is another command prompt and the one I use daily):

native cmd (feuillemorte:3374-long-filenames)

Cmder (feuillemorte:3374-long-filenames)

Now screenshots using the master branch:

native cmd (master)

Cmder (master)

I'm not sure this is something specific to Windows though. @RonnyPfannschmidt can you please try this in your system as well and see how it looks?

nicoddemus · 2019-06-06T22:44:41Z

Hi @feuillemorte,

It has been a long time since it has last seen activity, plus we have made sweeping changes on master to drop Python 2.7 and 3.4 support, so this PR has some conflicts which require attention.

In order to clear up our queue and let us focus on the active PRs, I'm closing this PR for now.

Please don't consider this a rejection of your PR, we just want to get this out of sight until you have the time to tackle this again. If you get around to work on this in the future, please don't hesitate in re-opening this!

Thanks for your work, the team definitely appreciates it!

feuillemorte added 3 commits April 10, 2018 22:22

#3374 Added support for long file names

c8cc18a

#3374 Added test

0514142

#3374 Added changelog

a63f37b

feuillemorte added topic: reporting related to terminal output and user-facing messages and errors type: enhancement new feature or API change, should be merged into features branch labels Apr 10, 2018

#3374 Fix comments

30925cf

#3374 Fix bug

0ed67c7

nicoddemus requested changes Apr 23, 2018

View reviewed changes

nicoddemus requested changes Apr 25, 2018

View reviewed changes

#3374 fix comments

b9ebffb

nicoddemus closed this Jun 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#3374 If path name is very long then pytest will split name for several lines #3383

#3374 If path name is very long then pytest will split name for several lines #3383

feuillemorte commented Apr 10, 2018

coveralls commented Apr 10, 2018 •

edited

Loading

nicoddemus commented Apr 10, 2018

feuillemorte commented Apr 13, 2018

feuillemorte commented Apr 13, 2018

nicoddemus left a comment

nicoddemus Apr 23, 2018

feuillemorte Apr 24, 2018 •

edited

Loading

nicoddemus Apr 25, 2018

nicoddemus Apr 25, 2018

nicoddemus Apr 25, 2018

feuillemorte Apr 25, 2018

nicoddemus Apr 26, 2018

feuillemorte Apr 27, 2018

feuillemorte Apr 27, 2018

nicoddemus Apr 23, 2018

nicoddemus Apr 25, 2018

nicoddemus left a comment

nicoddemus Apr 25, 2018

nicoddemus Apr 25, 2018

nicoddemus Apr 25, 2018

nicoddemus Apr 25, 2018

nicoddemus Apr 25, 2018

nicoddemus Apr 25, 2018

nicoddemus commented May 1, 2018

nicoddemus commented Jun 6, 2019

#3374 If path name is very long then pytest will split name for several lines #3383

#3374 If path name is very long then pytest will split name for several lines #3383

Conversation

feuillemorte commented Apr 10, 2018

coveralls commented Apr 10, 2018 • edited Loading

nicoddemus commented Apr 10, 2018

feuillemorte commented Apr 13, 2018

feuillemorte commented Apr 13, 2018

nicoddemus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feuillemorte Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicoddemus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicoddemus commented May 1, 2018

native cmd (feuillemorte:3374-long-filenames)

Cmder (feuillemorte:3374-long-filenames)

native cmd (master)

Cmder (master)

nicoddemus commented Jun 6, 2019

coveralls commented Apr 10, 2018 •

edited

Loading

feuillemorte Apr 24, 2018 •

edited

Loading