aws / aws-encryption-sdk-python

AWS Encryption SDK
https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/introduction.html
Apache License 2.0
234 stars 83 forks source link

Test failures with Python 3.12 #644

Closed fabaff closed 6 months ago

fabaff commented 6 months ago

Problem:

Two tests are failing with Python 3.12 while they are passing with Python 3.11.

============================= test session starts ==============================
platform linux -- Python 3.12.2, pytest-7.4.4, pluggy-1.4.0
rootdir: /build/aws-encryption-sdk-3.1.1
configfile: setup.cfg
plugins: mock-3.12.0
collected 2626 items                                                           

test/functional/test_f_aws_encryption_sdk_client.py .................... [  0%]
........................................................................ [  3%]
[...]
test/unit/test_material_managers.py .................                    [ 27%]
test/unit/test_material_managers_base.py F                               [ 27%]
test/unit/test_material_managers_caching.py ............................ [ 28%]
[...]
test/unit/test_utils.py ..............                                   [100%]

=================================== FAILURES ===================================
________________________________ test_abstracts ________________________________

    def test_abstracts():
        with pytest.raises(TypeError) as excinfo:
            CryptoMaterialsCache()

>       excinfo.match(
            r"Can't instantiate abstract class CryptoMaterialsCache with abstract methods {}".format(
                ", ".join(
                    [
                        "get_decryption_materials",
                        "get_encryption_materials",
                        "put_decryption_materials",
                        "put_encryption_materials",
                    ]
                )
            )
        )
E       AssertionError: Regex pattern did not match.
E        Regex: "Can't instantiate abstract class CryptoMaterialsCache with abstract methods get_decryption_materials, get_encryption_materials, put_decryption_materials, put_encryption_materials"
E        Input: "Can't instantiate abstract class CryptoMaterialsCache without an implementation for abstract methods 'get_decryption_materials', 'get_encryption_materials', 'put_decryption_materials', 'put_encryption_materials'"

test/unit/test_caches_base.py:25: AssertionError
________________________________ test_abstracts ________________________________

    def test_abstracts():
        with pytest.raises(TypeError) as excinfo:
            CryptoMaterialsManager()

>       excinfo.match(
            r"Can't instantiate abstract class CryptoMaterialsManager with abstract methods {}".format(
                ", ".join(["decrypt_materials", "get_encryption_materials"])
            )
        )
E       AssertionError: Regex pattern did not match.
E        Regex: "Can't instantiate abstract class CryptoMaterialsManager with abstract methods decrypt_materials, get_encryption_materials"
E        Input: "Can't instantiate abstract class CryptoMaterialsManager without an implementation for abstract methods 'decrypt_materials', 'get_encryption_materials'"

test/unit/test_material_managers_base.py:25: AssertionError
=============================== warnings summary ===============================
test/functional/test_f_crypto.py::test_signer_key_bytes_cycle
  /build/aws-encryption-sdk-3.1.1/test/functional/test_f_crypto.py:46: CryptographyDeprecationWarning: Curve argument must be an instance of an EllipticCurve class. Did you pass a class by mistake? This will be an exception in a future version of cryptography.
    key = ec.generate_private_key(curve=ec.SECP384R1, backend=default_backend())

test/unit/test_compatability.py::TestWarnDeprecatedPython::test_happy_version
  /nix/store/srq6cpawjsy99hk77ws2hl1fxlzvydx3-python3.12-pytest-7.4.4/lib/python3.12/site-packages/_pytest/python.py:194: PytestRemovedIn8Warning: Passing None has been deprecated.
  See https://docs.pytest.org/en/latest/how-to/capture-warnings.html#additional-use-cases-of-warnings-in-tests for alternatives in common use cases.
    result = testfunction(**testargs)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
FAILED test/unit/test_caches_base.py::test_abstracts - AssertionError: Regex pattern did not match.
FAILED test/unit/test_material_managers_base.py::test_abstracts - AssertionError: Regex pattern did not match.
============ 2 failed, 2623 passed, 1 skipped, 2 warnings in 8.76s =============

Solution:

Adjust the tests for Python 3.12

Out of scope:

Is there anything the solution will intentionally NOT address?

texastony commented 6 months ago

@fabaff

Thanks for the issue!

I tried to reproduce this.

pyenv install 3.12.2; # can take a while, but I had 3.12.0
pyenv local 3.12.2;
tox -e py312-local --recreate

Everything passed.

Why did your test invocation resolve pytest to 7.4.4? Also, pytest-mock is off. Our test-requirements.txt pins pytest to 7.2.1.

The test failures you are seeing are not an indication of any behavioral failures, but changes in how Python formats some Exception strings.

They are annoying, but not indicative of anything troubling.

It is interesting that you see formatting changes with only changes to some testing dependencies.

If we see these errors in our regular CI, which runs against Python 3.7 through 3.12, we will address them then.

Otherwise, if you are interested in testing this library, I encourage you to test it like we do in our GitHub CI.

Otherwise, we cannot have reproducible failures.

More of my Test Session Print out:

GLOB sdist-make: /Volumes/workplace/aws-encryption-sdk-python/setup.py
py312-local recreate: /Volumes/workplace/aws-encryption-sdk-python/.tox/py312-local
py312-local installdeps: -rdev_requirements/test-requirements.txt
py312-local inst: /Volumes/workplace/aws-encryption-sdk-python/.tox/.tmp/package/1/aws-encryption-sdk-3.1.1.zip
py312-local installed: attrs==23.2.0,aws-encryption-sdk @ file:///Volumes/workplace/aws-encryption-sdk-python/.tox/.tmp/package/1/aws-encryption-sdk-3.1.1.zip#sha256=987418ab728034ec6d794c0f77ed1fd37181e1e711c515b223ba4272f0a1672f,boto3==1.34.53,botocore==1.34.53,cffi==1.16.0,coverage==7.4.3,cryptography==42.0.5,iniconfig==2.0.0,jmespath==1.0.1,mock==4.0.3,packaging==23.2,pluggy==1.4.0,pycparser==2.21,pytest==7.2.1,pytest-cov==4.0.0,pytest-mock==3.6.1,python-dateutil==2.8.2,s3transfer==0.10.0,six==1.16.0,urllib3==2.0.7,wrapt==1.16.0
py312-local run-test-pre: PYTHONHASHSEED='3793276828'
py312-local run-test: commands[0] | pytest --basetemp=/Volumes/workplace/aws-encryption-sdk-python/.tox/py312-local/tmp -l test/ -m local
=========================================================== test session starts ============================================================
platform darwin -- Python 3.12.2, pytest-7.2.1, pluggy-1.4.0
cachedir: .tox/py312-local/.pytest_cache
rootdir: /Volumes/workplace/aws-encryption-sdk-python, configfile: setup.cfg
plugins: mock-3.6.1, cov-4.0.0
collected 2721 items / 107 deselected / 2614 selected
fabaff commented 6 months ago

Thanks for your answer. Looks like a little collision between the distribution on PyPI and what works for distribtions :wink:

Why did your test invocation resolve pytest to 7.4.4?

The failures were seen in Nixpkgs during a rebuild for Python 3.12. The current available release of pytest in Nixpkgs is 7.4.4, thus we can't run the tests with 7.2.1.

I encourage you to test it like we do in our GitHub CI

Strictly following the upstream procedure for testing is unfortunately not always possible for distribution packages.

texastony commented 6 months ago

@fabaff

strictly following the upstream procedure for testing is unfortunately not always possible for distribution packages.

This is fair. A feature request to validate all tests pass with your configuration seems fair to me, but I would need to know what your configuration is, and how we can re-create this in our CI.

Alternatively, if you can configure your test to ignore these tests, that is probably the best solution, as the content of the error messages are still accurate.

Which brings a better Feature Request to mind: AWS Crypto Tools should annotate Regex tests on exception messages so that they can be easily ignored by consumers who's Python environment may change the messaging format.

Would that best address your issue?