search

mahmoudimus / nose-timer

A timer plugin for nosetests (how much time does every test take?)
MIT License
126 stars 33 forks source link

Disabled color support for Windows #61

Closed ereOn closed 9 years ago

ereOn commented 9 years ago

Windows + nosetests sadly doesn't support color output in the terminal.

This PR changes the default on Windows to disable color output (which otherwise makes the output unreadable).

mahmoudimus commented 9 years ago

Thanks for the pull request @ereOn!

skudriashev commented 9 years ago

@ereOn can you just use --timer-no-color switch?

skudriashev commented 9 years ago

@ereOn make sure tests pass.

ereOn commented 9 years ago

@skudriashev That last pull request was done in a hurry : I will fix the tests.

That being said, I could use --timer-no-color but what's the point of having a default behavior that can't possibly work on Windows ?

mahmoudimus commented 9 years ago

@ereOn I generally agree with you. Should at least say "Disabled by default on Windows.", but then we get into the whole argument @skudriashev brought up, which is how do we guarantee no regressions going fwd?

ereOn commented 9 years ago

@mahmoudimus I modified the tests to account for my changes.

However, the failing test was just checking the number of options added to the parser. I'm not sure this really brings anything in terms of regressions anyway.

skudriashev commented 9 years ago

@ereOn can merge until pep8 errors are fixed.

ereOn commented 9 years ago

Fixed the indentation. Tests are passing ! :)

mahmoudimus commented 9 years ago

@ereOn, can you address this comment: https://github.com/mahmoudimus/nose-timer/pull/61#discussion_r30355286 ?

skudriashev commented 9 years ago

Merged.

skudriashev commented 9 years ago

@ereOn please, address comment https://github.com/mahmoudimus/nose-timer/pull/61#discussion_r30355286 with separate patch.

ereOn commented 9 years ago

@skudriashev By any chance, have you updated a new release on Pypi ?