Skip to content

Make Tests Python 3.11 Compatible - #34506

Merged
feanil merged 6 commits into
masterfrom
feanil/fix_tests_for_python_3.11
Apr 19, 2024
Merged

Make Tests Python 3.11 Compatible#34506
feanil merged 6 commits into
masterfrom
feanil/fix_tests_for_python_3.11

Conversation

@feanil

@feanil feanil commented Apr 11, 2024

Copy link
Copy Markdown
Contributor

Multiple edx-platform Python tests fail when running on 3.11 Update those tests so that they will work correctly when we start testing on 3.11.

This PR does not introduce 3.11 testing because there are other costs and complexities to doing that so it is just making modifications to the tests so that they will be compatible with both versions of the code.

The goal is to land this ahead of the other parts of #34374 so that we can reduce the size of the breaking change PR.

  • fix: Be able to clear the process_cache manually in Python 3.11
  • fix: Provide a sequence to random.sample
  • fix: Create a bad unicode file differently.
  • fix: Fix function mocking.
  • fix: Don't use the deprecated location for Hashable
  • fix: Remove deprecated getargspec call.

@feanil feanil linked an issue Apr 11, 2024 that may be closed by this pull request
4 tasks
@feanil
feanil requested a review from kdmccormick April 12, 2024 14:24

@kdmccormick kdmccormick left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and thanks for the helpful commit messages.

@UsamaSadiq UsamaSadiq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simple suggestion, otherwise looking good to be merged.

Comment thread xmodule/tests/xml/factories.py Outdated
feanil added 6 commits April 18, 2024 13:28
Given code like the following

```
class Foo:
    @process_cached
    def bar(self):
        pass
```

In Python 3.8 referencing `bar` would not call its `__get__` method.

```
x = Foo().bar
```

However in Python 3.11, making the same call would call the `__get__`
method, permanently replacing the underlying `process_cached` object
with the partial function that references it.

This meant that code to clear the cache would work in Python 3.8 but
would break in 3.11

```
Foo().bar.cache.clear()  # Works in 3.8 but not in 3.11
```

In 3.11 this results in the following error:
```
E       AttributeError: 'functools.partial' object has no attribute 'cache'
```

To make this compatible in both version, we just add the cache as an
accessible attribute on the partial we generate for our wrapped
function.
The sample function used to automatically convert sets to sequences but
that is no longer supported starting in 3.11 so we have to do it
manually.

Reference: https://docs.python.org/3/library/random.html#random.sample
In Python 3.11 CSV files are allowed to have null characters so the test
data is no longer a valid. We update it to not have a valid unicode
character to still test this code path correctly.

I'm not concerned about the fact that the files with null will get past
this test beacause there are other checks on the content of the file
that catch if it doesn't have enough or the right fields so this should
be a safe change to make to the tests.

Relevant Change in Python: python/cpython#71767
The way the patch decorator was being used is not supported in python
3.11.  Use the patch decorator to auto generate the correct mock and
make the test a bit more readabale.  The new change is both 3.8 and
3.11 compatible.
The Hashable object was moved in python 3.3 and support for the old
location is dropped in python 3.10 the new location is available in
python 3.8 so we can just update this and it should work with both
python 3.8 and 3.11

https://docs.python.org/3.8/library/collections.html
This function was removed by python 3.11 so update to the alternate
call that is the current recommended replacement.

https://docs.python.org/3.11/library/inspect.html#inspect.getfullargspec
@feanil
feanil force-pushed the feanil/fix_tests_for_python_3.11 branch from 995036f to 6fb5963 Compare April 18, 2024 17:28
@feanil
feanil merged commit a08b7de into master Apr 19, 2024
@feanil
feanil deleted the feanil/fix_tests_for_python_3.11 branch April 19, 2024 14:07
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[edx-platform] Test Python 3.11

4 participants