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

Making _CallOutcome.result private in 3.0 #1572

Closed
The-Compiler opened this issue May 31, 2016 · 5 comments
Closed

Making _CallOutcome.result private in 3.0 #1572

The-Compiler opened this issue May 31, 2016 · 5 comments
Labels
type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog

Comments

@The-Compiler
Copy link
Member

I propose to rename pluggy._CallOutcome.result to _result with pytest 3.0. It seems to be a common mistake to use .result instead of get_result():

If there was an exception in the wrapped hook, the result attribute will not exist at all, which will cause an AttributeError which hides the exception which actually happened.

To avoid that mistake, I think it'd be better to only use the attribute internally and only provide get_result as external API.

@The-Compiler The-Compiler added type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog vendored-pluggy labels May 31, 2016
@RonnyPfannschmidt
Copy link
Member

please report against pluggy

i think we could get by by creating a non-data descriptor that raises the actual exception from the actual traceback when used

(non-data descriptors get only triggered if the instance does not have an attribute of the same name)

@The-Compiler
Copy link
Member Author

I reported it here because it'll be a backwards compatibility break in pytest and mainly affect pytest code and users 😉

I guess that'd work as well, but it strikes me as quite weird that an attribute access would raise a (non-AttributeError) exception.

@RonnyPfannschmidt
Copy link
Member

i think a nice way to solve this is to make get_result use _result and to have result as a alias property for get_result

@nicoddemus
Copy link
Member

I like @RonnyPfannschmidt's idea. 😁

I also have been bitten by this issue myself.

@nicoddemus
Copy link
Member

This has been changed already on pluggy, but will become again a read-only property that calls get_result() under the covers in pytest-dev/pluggy#88.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog
Projects
None yet
Development

No branches or pull requests

3 participants