First cut of test for the Airbrussh formatter #13
Conversation
robd
force-pushed
the
add-formatter-test
branch
4 times, most recently
from
b0dfc94 to
80a32a5
Compare
|
@mattbrictson I guess maybe you're not about at the moment, but this passes on all ruby / sshkit versions now. I rebased this on #14, so PR #14 should probably be merged before this one to make the history clear. I hope this is useful, let me know what you think. I will now move on to try and rework Airbrussh to use the new SSHKit command logging API. |
robd
force-pushed
the
add-formatter-test
branch
8 times, most recently
from
fed33a7 to
7eba160
Compare
|
@robd Thanks for starting to fill in much-needed test coverage for Airbrussh. I hadn't realized that
I see you have suggested 1.9+. I would really like to drop 1.9, but there is evidence that 1.9 is still being used. I've opened a separate issue to discuss that.
I agree that the line-, method-, and class-length checks in particular cause trouble for writing tests. For actual (i.e. non-test) code though, I would like to try to honor the Rubocop recommendations.
Definitely! Give me a little more time to review these changes and I'll let you know. |
| def assert_log_file_lines(*command_lines) | ||
| preamble_lines = [ | ||
| /#{blue('INFO')} ---------------------------------------------------------------------------\n/, | ||
| /#{blue('INFO')} START [\d\-]+ [\d\:]+ \+\d+ cap\n/, |
There was a problem hiding this comment.
@robd I think this regex needs to be tweaked to work in my time zone. The minus sign isn't getting matched:
FAIL["test_formats_execute_without_color", TestFormatter, 2015-05-24 13:45:46 -0700]
test_formats_execute_without_color#TestFormatter (1432500346.55s)
Expected /INFO START [\d\-]+ [\d\:]+ \+\d+ cap\n/ to === " INFO START 2015-05-24 16:34:30 -0700 cap\n".
There was a problem hiding this comment.
Ah OK, of course. With all of these tests, it's a bit hard to know how tightly to match the output. Also, much of this is testing the SSHKit pretty formatter, but since I don't really know what SSHKit did historically, I think it's worth testing here.
I will fix and rebase this PR on master, and also rebase #15. Will comment when this is done.
|
OK, rebased on master and hopefully fixed negative timezone offset. |
|
Tests pass on my machine and this looks good to merge. I am going to review #17 first, since I think it supersedes this PR. |
This is a first cut of a test for the Airbrussh formatter in order to try to protect against breakages when I update it to work with the latest SSHKit.
I guess the first question is, is there something like this test already that I am missing? How do you know if things are working - is it manual at the moment?
A few other thoughts came up when I was working on this:
Some of the Rubocop rules are a PITA IMHO! I guess Rubocop rules in Airbrussh are quite new, but I wanted to mention I disabled the line length checks and the other rules which I disagreed strongly with in the test. We can talk about these if you want.
Let me know your thoughts - thanks!