-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
isatty #2550
isatty #2550
Conversation
@Suor Please take a look. |
tests/unit/test_progress.py
Outdated
if DVC_ISATTY: | ||
assert "0/10" in out_err.err | ||
else: | ||
assert out_err.err == "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if doesn't make much sense if we only test one of its branches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may test both depending on env vars when the test is run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But in conftest.py
it's set to True for all tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm yes but I feel this way is more future proof. I don't know what your test framework conventions are so can't really decide. At a minimum I think at least a comment would be required around these lines.
- update tests - add documentation
- partially reverts commit 524a0df188f566fb3e103f6965f1b6ac9c530886 - fixes https://github.com/iterative/dvc/pull/2550/files/ad1f245107c405697c4b61fe58a92fc0265c6588#r335811639
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
fixes #2549 non-tty output
Tqdm
whenisatty()
isatty()