Skip to content
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

updates for 0.7 and more systematic test output #26

Merged
merged 2 commits into from
Jul 6, 2018

Conversation

ssfrr
Copy link
Collaborator

@ssfrr ssfrr commented Jun 29, 2018

This updates for 0.7, including capturing logging output, which is a little more complicated because the logger caches STDERR, so it has to be handled separately.

This also reworks the testing so that it's more systematic. Previously there was a mix of things captured and things that were printed, and you had to read things carefully to figure out whether it was behaving correctly. The expected output is now as below. The incrementing numbers are so that you can tell if something got captured/suppressed that shouldn't have (there will be a skipped number) or if something doesn't get suppressed it'll show up in the output and say SUPPRESSED so it should be pretty obvious.

At some point in the future it would probably be good to use the shell to pipe this output to a file and compare to an expected output.

Also there's getting to be a lot of duplication in the code. I'm planning on a separate PR to clean up the code a bit, but I figured keeping this separate would make it easier to review.

01 PRINTED STDERR
02 PRINTED STDOUT
03 PRINTED STDOUT
04 PRINTED STDERR
05 PRINTED GREEN STDOUT
06 PRINTED GREEN STDERR
07 PRINTED NORMAL STDOUT
08 PRINTED NORMAL STDERR
09 PRINTED GREEN STDOUT
10 PRINTED GREEN STDERR
11 PRINTED NORMAL STDOUT
12 PRINTED NORMAL STDERR
13 PRINTED GREEN STDOUT
14 PRINTED GREEN STDERR
15 PRINTED NORMAL STDOUT
16 PRINTED NORMAL STDERR
17 PRINTED STDERR
18 PRINTED STDOUT
19 PRINTED STDOUT
20 PRINTED STDERR
21 PRINTED STDOUT
22 PRINTED STDERR
23 PRINTED STDOUT
24 PRINTED STDERR
25 PRINTED STDOUT
26 PRINTED STDERR
27 PRINTED STDOUT
28 PRINTED STDERR
29 PRINTED STDOUT
30 PRINTED STDERR
31 PRINTED STDOUT
32 PRINTED STDERR
[ Info: 33 PRINTED LOGINFO
[ Info: 34 PRINTED LOGINFO
[ Info: 35 PRINTED LOGINFO
Test Summary: | Pass  Total
Suppressor    |    7      7
   Testing Suppressor tests passed

@ssfrr
Copy link
Collaborator Author

ssfrr commented Jun 29, 2018

Linux 0.6 failure seems to be some kind of spurious docker thing - can someone kick it?

@tpapp
Copy link

tpapp commented Jun 29, 2018

Just make a trivial commit. Eg the logs suggest that there may be a stray STDIN left somewhere.

@ssfrr
Copy link
Collaborator Author

ssfrr commented Jun 29, 2018

I grepped for STDIN and didn't see anything - where are you seeing that?

Anyways I hadn't updated the capitalization in the README and pushed. Travis is passing now. The coverage is showing none of the 0.7 code being run, but I think that's just because coverage is broken on 0.7...

@ssfrr
Copy link
Collaborator Author

ssfrr commented Jul 4, 2018

If no one objects I'll plan on merging this tomorrow.

@Ismael-VC
Copy link
Contributor

Dear @ssfrr,

Could you please add a test case for when haslogging is true? In order to exercise the new code paths.

Best regards.

@ssfrr
Copy link
Collaborator Author

ssfrr commented Jul 5, 2018

haslogging() will be false on 0.6 and true on 0.7, so it should be getting exercised. I believe there are still issues with Coverage on 0.7, which I think is why it's not showing up as covered.

@Ismael-VC
Copy link
Contributor

Thank you, LGTM.

@Ismael-VC Ismael-VC closed this Jul 6, 2018
@Ismael-VC Ismael-VC reopened this Jul 6, 2018
@Ismael-VC
Copy link
Contributor

Sorry wrong button.

@Ismael-VC Ismael-VC merged commit 38deaba into JuliaIO:master Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants