Support assignment expressions (i.e. the ":=" operator)

[d-maurer]

Assignment expressions are helpful to get clearer and more concise code (in many situations).

Because the Python grammar allows only simple identifiers as target of those expressions, they should be safe (at least) in all contexts that allow assignments of the form variable = expr.

The Python (3.8) concrete grammar allows only identifier as target of assignment expressions; the abstract grammar, however, allows arbitrary expressions. This PR records an error when the target is not a Name (which corresponds to identifier in the concrete grammar) -- special handling would be necessary for more general targets.

[icemac]

@d-maurer Thank you for your PR. There is already a similar one: https://github.com/zopefoundation/RestrictedPython/pull/175 I prefer the implementation you did over the one over there, but I'd like to see a test both for the happy code path and for the exception to make sure it will run successfully in future Python versions.

[d-maurer]

Michael Howitz wrote at 2020-4-24 07:20 -0700:

@d-maurer Thank you for your PR. There is already a similar one: https://github.com/zopefoundation/RestrictedPython/pull/175 I prefer the implementation you did over the one over there, but I'd like to see a test both for the happy code path and for the exception to make sure it will run successfully in future Python versions.

Currently, RestrictedPython has no tests at all.

AccessControl contains very limited tests for RestrictedPython. A comment suggests that their purpose is not to seriously test RestrictedPython but to verify that Python works at all with the RestrictedPython policy provided by AccessControl.

Currently, the path for the exception cannot be achieved with Python code (due to Python concrete grammar restrictions); direct "AST" manipulation would be necessary.

[d-maurer]

@icemac I have added the wanted tests. I am not sure, however, whether you will like their location: I have put them into RestirctedPython/tests and there they are currently alone. Apparently, some configuration magic typically takes RestrictedPython tests from elsewhere (especially AccessControl).

[icemac]

@d-maurer In RestrictedPython we decided to put the tests outside the src directory: https://github.com/zopefoundation/RestrictedPython/tree/master/tests (I do not understand what you mean be the configuration magic happening.)

There are already helpers for determining the Python version in https://github.com/zopefoundation/RestrictedPython/blob/master/src/RestrictedPython/_compat.py

[d-maurer]

Michael Howitz wrote at 2020-5-8 07:19 -0700:

@d-maurer In RestrictedPython we decided to put the tests outside the src directory

Why?

It means:

• the tests are not installed by a regular installation; you need a source installation (or "git checkout") to see them

• it makes RestrictedPython special: it is the first package I met where the tests are not a regular submodule/subpackage

• you need a rarely used zope.testrunner option (--test-path) that it finds the tests

...

I do not understand what you mean be the configuration magic happening.

I tried to run the tests via zope.testrunner - and it found nothing. I saw that the "github" tests found tests. I looked at tox.ini to find out how this is possible and did not understand it. I concluded that some "configuration magic" must be at work.

[d-maurer]

Michael Howitz wrote at 2020-5-8 07:19 -0700: @d-maurer In RestrictedPython we decided to put the tests outside the src directory Why? ...

• you need a rarely used zope.testrunner option (--test-path) that it finds the tests

Apparently, the RestrictedPython tests require pytest (as runner) and do not run with zope.testrunner at all. The (zope.testrunner) option --test-path allows it to find the test modules but it does not find tests in those modules.

[dataflake]

I agree that the way this package is structured is a mess. I can't even figure out how to run tox without getting error messages. That test runner seems to dive into the entire site-packages in the virtual environment. I would like to help on this package but the way it is I feel unable to.

[dataflake]

If you look into tox.ini you can see that the tests are run using pytest. The configuration for it is in setup.cfg, that's where it sets up the paths to look for tests. I have made a change on master to remove . from that list and just leave tests - having the current folder meant the test collector went into the library path inside that folder and pick up tests for all kinds of stuff (and subsequently spewing error messages). Now you can actually run bin/tox and get something meaningful. I have also changed the tox configuration to allow parallel runs with -pall, that makes the overall run a lot faster and you don't get those miles of text output that I guess someone at some time found useful.

@icemac can you look/review again so we can move forward?

[icemac]

@dataflake You are right this package is special – you can call it a mess. The idea behind this package was to use some more modern approaches. (The one moving the tests outside one src I still do not get, I personally see no gain in it.)

[dataflake]

The main issue I see is that it uses tools and configurations that are completely out of step with all other packages. Approaching this package and helping out here is unnecessarily hard.