Should not do mutating variable resolution

[DirkRichter]

Using parameter --listener RobotStackTracer in regular robot run results in different test result (pass vs fail):

running robot Test.robot

running robot --listener RobotStackTracer Test.robot

This --listener RobotStackTracer is now used in latest pabot version and i wonder, why my tests fail with pabot but pass in robot.

Versions:
Windows 10
python 3.9.6
robotframework-pabot 2.0.0
robotframework 4.0.3

[mkorpela]

Thanks @DirkRichter !

[pekkaklarck]

Is variable resolution needed in general? I'm afraid detecting does resolving a variable do something that shouldn't be done is pretty much impossible. For example, ${var.attr} can call a property that does something that should only be done once.

[mkorpela]

I think there is value in it, but resolving that allows code execution should not happen or happen only after a failure.

[DirkRichter]

Thx. Now i understand, why ${d.pop('x')} results in KeyError. When key 'x' is removed from ${d} inside RobotStackTracer, then real robot can not find key 'x'.

Thus in general a robot listener should always be side-effect-free (https://dev.to/dev0928/what-is-a-side-effect-of-a-function-in-python-36ei).
Pure functional listeners could ensure this e.g. via https://pypi.org/project/effect/. Unfortunately writing such listeners would become much more complex.

[pekkaklarck]

Yeah, the problem is that ${d.pop('x')} modifies d and that affects execution later. I agree listeners shouldn't have side-effects and in general thus shouldn't resolve variables. I would also say that variable usages shouldn't have side-effects so ${d.pop('x')} is questionable. Using a keyword to pop the value would be better.

[mkorpela]

@Snooz82 I put a PR for this. #3

Seems that your the only one with PyPI deployment keys, so can not deploy.

[mkorpela]

It might be good also to remove variable resolution from higher calls in the stack.
Original solution took the value at the time of start_kw (this is good) but has side-effects that can impact the execution.
Proposed change in PR works only in end_kw in case of a failure, but this might have values that are not anymore correct for higher calls in the stack.

[mkorpela]

Fixed in next version.