Update eval parameter passing during formatting

[tparchambault]

Portable eval call during formatting

The eval() built-in functions API is evolving in 3.13. This change uses a version check to add support for 3.13 while keeping support back to 3.9.

Closes #1019

[tparchambault]

Test failed of Py 3.12/3.13 fix over RHEL9.2 w/Py 3.9: TypeError: eval() takes no keyword arguments

Will fix with Py version check and branching.

[jw3]

Dont worry about that failure on the tools build, something upstream changed.

[tparchambault]

Tested fix on RHEL9, and FC41/Rawhide w/no observed issues related to the fix. However on FC40/py 3.12.2, eval() does not support keyword args. I was mistaken in my research when I tried to determine which py versions supported keyword args; Minor fix to change the conditional version check. Manually edited the code on FC40 and issue addressed.

[jw3]

@tparchambault this is good to go?

[tparchambault]

@tparchambault this is good to go?

This lastest fix needs to be tested on everything first. My first fix worked on our extreme version platforms (i.e. RHEL9 and FC41/Rawhide) but failed on FC40, so similarly, I've got to retest on our mid-range Py version supported platforms. I expect to get to it late morning or early afternoon today.

[tparchambault]

Fix tested over RHEL9.2, FC39, FC40, and FC41/Rawhide. No issues observed related to the eval() function embedded in the f() formatting function, no exceptions thrown. This can be merged into master for 1.4. I did not write an associated unit-test. Will open a separate ticket.

[egbicker]

I can confirm no issues seen in FC40 as well

[jw3]

I did not write an associated unit-test. Will open a separate ticket.

The unit test for this only needs to verify it can be called without exception, right?

[tparchambault]

In the dev env, we are running 3.9 based on the Pipfile. So when we run the unit-tests in that env we would only be checking one branch of the if/else so probably need to mock the version info to test the range of supported py version and check that the appropriate eval() variant is called for each.

[jw3]

@tparchambault right. In CI we install pipenv with the Python version specified by the CI job:

https://github.com/ctc-oss/fapolicy-analyzer/blob/master/.github/workflows/python.yml#L57

So a unit test that does not mock should indeed test the appropriate version there.

So if you add a second unit test that mocks all versions, that would cover the dev env too.

[tparchambault]

Still wrestling with the mocking scope of sys.version_info. into the f(). Will look at it more tomorrow. I'm trying to avoid changing the fix so that I use keywords approach like in https://stackoverflow.com/questions/38022926/how-to-unit-test-a-python-version-switch but right now, sys.version_info within the f() function is not seeing the mocked values. I'm probably not specifying the module path correctly. It would not be the first time. Anyhoo, tomorrow.

edit: This was ultimately the issue:

I'm probably not specifying the module path correctly.

[tparchambault]

I think we are good. Could have made the eval() mock slicker wrt checking the args passed in, but this unit-test tests coverage for 3.9, 3.12, and 3.13.

[tparchambault]

@jw3 I want to add a regex check of the eval() mock's args verifying the 3.13 call uses keyword args. If this goes in the latest rc as is, I'll add that check in the unit-test into the FC41 CI ticket.

[tparchambault]

Full raw argument string wasn't obviously available (that I could find in the pytest-mock docs.) call_args are available as a tuple, with the second element containing the kw dict if keywords are used. Used the population of that dict to determine if the correct eval() variant was invoked. Py 3.13's eval() parameter list has evolved from 3.12 but it does support kwargs, which is used in the f() function when running 3.13.

[jw3]

looks like 3.12 is failing?

FAILED fapolicy_analyzer/tests/test_format.py::test_f_supported_py_vers[py_ver0-foo-insert is foo] - AttributeError: 'called_once' is not a valid assertion. Use a spec for the mock if 'called_once' is meant to be an attribute.
FAILED fapolicy_analyzer/tests/test_format.py::test_f_supported_py_vers[py_ver1-bar-insert is bar] - AttributeError: 'called_once' is not a valid assertion. Use a spec for the mock if 'called_once' is meant to be an attribute.
FAILED fapolicy_analyzer/tests/test_format.py::test_f_supported_py_vers[py_ver2-baz-insert is baz] - AttributeError: 'called_once' is not a valid assertion. Use a spec for the mock if 'called_once' is meant to be an attribute.

[tparchambault]

Trying the assertion with the simpler called attribute to see if that is supported in the 3.12 runtime env. If it doesn't work, I'll have to visit the docs. 3.11 had no issue with the called_once mock attribute, so something has changed in the test framework between 3.11 and 3.12. Fingers crossed.

[tparchambault]

I think 3.12 is good now:

fapolicy_analyzer/tests/test_file_chooser_dialog.py ..... fapolicy_analyzer/tests/test_format.py ......... fapolicy_analyzer/tests/test_fs.py .

I didn't realize that our testing list has grown so much that it's now scrollable, so I missed it last night. I just saw the first page w/the Tools/Build failure; Appears that 3.9 through 3.12 are all good now. Let's see how 3.13 holds up when that new PR is merged.