manrajgrover / halo

💫 Beautiful spinners for terminal, IPython and Jupyter
MIT License
2.86k stars 148 forks source link

FIXES #81: Add tests for checking color of spinner in stdout #101

Closed eoinnoble closed 5 years ago

eoinnoble commented 5 years ago

Description of new feature, or changes

_get_test_output has been refactored to take a no_ansi arg, which defaults to True. We then use this in the new test to keep the ANSI escape characters and compare them to those supplied by colorama.

It feels a little like we're testing another library with this test but I can understand why you feel like you need to have it in there.

I am only able to run the py36 tests at present, and they do not currently pass for me (see my other PR for a fix for that test). To keep this PR clean I have left them like that. The test that I have written obviously passes, though.

Checklist

Related Issues and Discussions

81

People to notify

@manrajgrover

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 329


Files with Coverage Reduction New Missed Lines %
halo/_utils.py 2 78.57%
<!-- Total: 2 -->
Totals Coverage Status
Change from base Build 327: 0.0%
Covered Lines: 272
Relevant Lines: 289

💛 - Coveralls
manrajgrover commented 5 years ago

@eoinnoble Looks like test cases are failing. Could you please look into it?

eoinnoble commented 5 years ago

It was the assert statement, should be self.assertEquals(color_ascii in output[1], True) if you don't want to use a bare one. I'm not sure if git has picked up my change, can you restart the automated checks from your end?

manrajgrover commented 5 years ago

@eoinnoble Thanks for fixing. Currently, Github is facing a lot of issues. Once they are fixed, I'll rerun Travis.

manrajgrover commented 5 years ago

Hi @eoinnoble, apologies for the delay! Thanks for working on it! :100:

We should also test halo notebook. I'll keep the issue open if you're up for it.

eoinnoble commented 5 years ago

Sure, I can take a stab at that over the coming days.