Skip to content

Conversation

@blueyed
Copy link
Contributor

@blueyed blueyed commented Jan 15, 2020

Via #6435.

assert safe_getattr(helper, "raise_exception", "default") == "default"
assert safe_getattr(helper, "raise_fail", "default") == "default"
with pytest.raises(BaseException):
assert safe_getattr(helper, "raise_baseexception", "default")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good find, i recall that we have outcome exception handling - if that's the case can you cover that as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean.
I've noticed that it's not covered with 85a813b#diff-4ec37b1ad5b7d41fdac56c907d08dde1R335-R340 (reverted there, but still).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, i missed that bit, thanks for calling out

i think i would have noted it better if it was named raise_fail_outcome, what do you think about the naming

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blueyed thanks, lovely

@blueyed blueyed force-pushed the cover-safe_getattr branch from 5b3b02b to 6f7a95c Compare January 15, 2020 10:20
@blueyed blueyed merged commit 29703a5 into pytest-dev:master Jan 15, 2020
@blueyed blueyed deleted the cover-safe_getattr branch January 15, 2020 10:20
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