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

pytest 3.3: TerminalReporter.writer removed without deprecating it first #2984

Closed
jluebbe opened this Issue Nov 30, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@jluebbe

jluebbe commented Nov 30, 2017

We have a small plugin, which uses the TerminalReporter.writer attribute to continue printing to the terminal even when normal output is captured and the CaptureFixture is used.

The writer attribute was originally added by 1fc466e back in 2013. It was removed by 05cfdcc (which intended to revert 3b30c93), probably by mistake.

@nicoddemus

This comment has been minimized.

Member

nicoddemus commented Nov 30, 2017

@jluebbe thanks for reporting! Indeed writer was removed by accident while trying to revert 05cfdcc which rendered _tw read-only while preparing for 3.3 in #2945. The reason for that revert is because pytest-instafail plugin attempts to set _tw to something else. 😞

Not sure how to proceed here to be honest. Removing writer was a mistake, but I'm thinking re-adding TerminalReporter.writer only to deprecate it in the next release will only add to the confusion further. Opinions?

@RonnyPfannschmidt @The-Compiler

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Nov 30, 2017

since its a public and used api we have to bring it back and can deprecate it in the next feature release

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Nov 30, 2017

man - for extra fun, practically all new code in that linked commit is broken and the plug-in itself has better methods to handle it already existing back at that time (unlike other methods of the object those in particular dont maintain the state, so in all fairness we should deprecate all of them

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Nov 30, 2017

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Nov 30, 2017

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Nov 30, 2017

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Nov 30, 2017

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Nov 30, 2017

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Nov 30, 2017

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Nov 30, 2017

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Nov 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment