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

Fix writing to the incorrect stream #171

Merged
merged 3 commits into from
Jul 24, 2014
Merged

Fix writing to the incorrect stream #171

merged 3 commits into from
Jul 24, 2014

Conversation

CleanCut
Copy link
Contributor

In several places, the documentation makes it clear that plugins should be able to control the output via one means or another. In some cases, setting event.handled = True prevents default output. Those cases seem to be working fine.

In other cases, you are told to wrap (or otherwise modify) the stream attached to the event to control output. This mostly works, but I found three places in nose2 that were not writing to the correct stream (they were using the default stream instead of the stream on the event), which prevents a plugin from fully controlling the output.

This pull request simply fixes three writeln calls to get called on the event's stream, just as the rest of the function is already doing in each case.

In _reportSummary() I also moved the location of the writeln call down until after the event's stream had been set up.

This is important for my particular plugin, because the whole point of the plugin is to completely change the output.

@CleanCut CleanCut changed the title Horiz rule fix Fix writing to the incorrect stream Apr 29, 2014
@CleanCut
Copy link
Contributor Author

Your Travis CI setup is nice, btw. Especially since I can't figure out how to run the self-tests locally.

@thedrow
Copy link
Member

thedrow commented May 1, 2014

I'll need a failing test case (preferablly both unit and functional but functional is more importent here) that passes with the changes you made in order to merge this.
We have a high coverage policy and it will ensure that there are no regressions in the future.

@CleanCut
Copy link
Contributor Author

CleanCut commented May 2, 2014

I discovered that it was actually much more straightforward (and less buggy) to just take the example TextTestRunner and TextTestResult from unittest and write new versions than to try to adapt to nose's or nose2's plugin API and behavior. So Green is now a standalone test runner. I don't have any interest in finishing up the work required to get this pull request merged. If someone here wants the work so far, feel free to grab it before I delete my fork of nose2 in a few weeks.

Green is runnable now. It doesn't aspire to have all the features that nose or nose2 have, but it's working pretty well for me. If anyone wants to check it out I would welcome feedback.

@CleanCut CleanCut closed this May 2, 2014
@thedrow thedrow reopened this May 2, 2014
@thedrow
Copy link
Member

thedrow commented May 2, 2014

@jpellerin Should we merge this anyway despite the fact that it doesn't have tests?

@thedrow
Copy link
Member

thedrow commented May 26, 2014

@jpellerin Does this look right to you?

thedrow added a commit that referenced this pull request Jul 24, 2014
Fix writing to the incorrect stream
@thedrow thedrow merged commit 7e48591 into nose-devs:master Jul 24, 2014
@thedrow
Copy link
Member

thedrow commented Jul 24, 2014

@jpellerin I'm taking a chance here since you are not responding.

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.

2 participants