Fix different behavior with unittest when warlus operator

[aless10]

The AssertionRewriter class did not handle the assertions with the ast.NamedExpr (aka 'walrus operator'), that changed the value of a variable used in the assertion (an example can be seen in the related issue or in the tests added).

This caused different behavior from unittest/python and pytest.

This PR aims to handle this case (although very remote)

Addresses python version >= 3.8

closes #10743

[aless10]

It looks like one of the checks failed but I don't know if it depends on the changes or what. What is the next action in this situation? I'd like to help in some way. Thanks

[RonnyPfannschmidt]

i restarted the job, its unrelated to your change

[nicoddemus]

Thanks a lot for tacking this @aless10!

Left two comments, could you take a look? Thanks!

[nicoddemus]

I noticed this as well, can't we save the first a value in a separate temporary variable, before evaluating the walrus operator? This would provide the same (correct) error message as in test_assertion_walrus_no_variable_name_conflict.

[aless10]

I tried to do so, but with my approach I did not change the value of the variable a but I saved the new value of a in another temporary variable. The problem with this is that if that variable is used later in another assert there will be an error. I think is doable, I just need to try different approach. Thanks

[nicoddemus]

The problem with this is that if that variable is used later in another assert there will be an error

IIUC the problem, you can use self.variable() to obtain a new, unique variable to save the value to. This would prevent any conflicts in later asserts.

[aless10]

Thanks for the tip: I refactored using the variable method but I don't think this is enough to solve the problem. For example in this test case

def test_walrus_conversion_succeed():
    a = "Hello"
    assert a != (a := a.lower())
    assert a == 'hello'

if I change the name of the first "a", then I got an error because it is referencing something that has no reference (in fact "a" is defined outside the first assert statement. I tried to change the name of the new "a" in the walrus operator, but then the second assert fails because the value of "a" is still the original one.
The only solution that came to my mind is using a map/state that lives through the asserts that enables me to retrieves the values if they have changed.
That said, this situation is very rare and the tests cases that came to my mind are far from being used in real case scenario (but this is only my opinion). I'm thinking if the juice worth the squeeze

[nicoddemus]

That said, this situation is very rare and the tests cases that came to my mind are far from being used in real case scenario (but this is only my opinion). I'm thinking if the juice worth the squeeze

Yeah if it proves too problematic, I'm happy with having just the original fix, and we can instead add a note to the rewrite docs somewhere about this corner case. I agree it should be very rare to happen in practice.

Please feel free to revert to the previous working version.

[aless10]

So, after #10788, all the tests pass. Right now, we have covered all of the cases that are tested in test_assertrewrite.py::TestIssue10743 where I tried to imagine most of the corner cases. Let me know if you like my solution or if you prefer to revert and add a note for this remote case. Thanks

[nicoddemus]

This was copied from visit_Name, perhaps we can update the comment to mention that this handles the walrus operator instead?

[aless10]

I don't know why all the checks are failing. Last time they passed was at commit 3e90bf5 and it looks like the failing tests are not referenced by these changes.
I tried to reproduce the error and actually the tox commands fails but if I run the tests inside a docker container, the tests pass. I'm trying to understand what's going on. Sorry for the inconvenience

[aless10]

I don't know why all the checks are failing. Last time they passed was at commit 3e90bf5 and it looks like the failing tests are not referenced by these changes. I tried to reproduce the error and actually the tox commands fails but if I run the tests inside a docker container, the tests pass. I'm trying to understand what's going on. Sorry for the inconvenience

OK it looks like I had to align with this #10788 fix. Sorry

[RonnyPfannschmidt]

this looks good and self-contained, but i don't have the bandwidth to get into it soon

[nicoddemus]

I think we can update this to mean that walrus operator now is properly supported by the assertion rewriting, correct?

Suggested change

	Fixed different behavior from std lib unittest of asserts with expression that contains the walrus operator in it that changes the value of a variable.
	The assertion rewriting mechanism now works correctly when assertion expressions that contain the walrus operator.

[aless10]

LGTM. I pushed the changes. Thanks

[nicoddemus]

Thanks! I will merge in a few days unless other maintainers want to take a closer look.