Add a default value to CallInfo.result

[alanbato]

Fixes #3554

Any comments/questions welcome :)

[nicoddemus]

Great, thanks @alanbato!

I added some formatting to the CHANGELOG entry, after CI passes this is good to merge IMHO.

[coveralls]

Coverage increased (+0.03%) to 92.732% when pulling 198e993 on alanbato:fix_callinfo_rrp into 7c8d072 on pytest-dev:master.

[alanbato]

Thanks @nicoddemus!

[RonnyPfannschmidt]

This is a breaking change, it has to be reverted

[nicoddemus]

Is CallInfo an external API? What breaks by this change?

[nicoddemus]

@RonnyPfannschmidt is right, indeed it is used by:

• pytest_runtest_makereport(item, call)
• pytest_exception_interact(node, call, report)

Someone might be checking hasattr(call, 'result') to check if the call succeeded.

I will open a PR to revert this from master shortly. I think it is OK to move this to features though?

[nicoddemus]

Opened #3562, thanks @RonnyPfannschmidt for catching my slip up

[RonnyPfannschmidt]

AS per original issue callinfo needs a New state, something Like pre-call

[nicoddemus]

AS per original issue callinfo needs a New state, something Like pre-call

I see, because result = None is a valid function return value. But you propose to update __repr__ to handle this "new state" then?

[RonnyPfannschmidt]

yes

[nicoddemus]

Agreed. @alanbato want to tackle that?

[alanbato]

Sure! Just to be sure though, do we want to include a new attribute that represents the different states? Something like CallInfo.is_finished. Or just do not has_attr(self, result) in the __repr__ func to print a different version of the repr string?

[RonnyPfannschmidt]

@alanbato i beleive a getattr(self, 'result', NOTSET) followed by an is check is helpful there

NOTSET is defined in compat.py, and im thinking switching to attr.NOTHING in the long term

[nicoddemus]

@RonnyPfannschmidt sounds good. I think the same check must be done for excinfo, I think it is also set only when an exception occurs.

[RonnyPfannschmidt]

@nicoddemus that one is None at the class level

[nicoddemus]

Oh OK, thanks

[blueyed]

This was reverted in #3562 - what is the plan for it?

[RonnyPfannschmidt]

well, i outlined the requirements for a correct fix above

[blueyed]

@alanbato
Do you still plan to create a new PR?

[blueyed]

New attempt in #4381.