HypothesisWorks / hypothesis

Hypothesis is a powerful, flexible, and easy to use library for property-based testing.
https://hypothesis.works
Other
7.6k stars 586 forks source link

Using classes with attr lib causes some "weird" behavior when testing with private fields #2461

Closed rmoretto closed 4 years ago

rmoretto commented 4 years ago

Hello!

So, I'm starting to test a project that uses a lot of attrs flavored classes, and when I started using hypothesis to generate some instances of those classes I noticed a strange behavior that I don't know if it is expected or not.

Basically when I have a attrs class with some private attributes defined with attr.ib(init=False) it seems that between tests the field is "saved" with its last value.

I created two tests cases, one using normal classes and with attrs:

Without attrs test:

from copy import copy
from typing import List

from hypothesis import given
from hypothesis import strategies as st
from hypothesis.strategies import SearchStrategy

class MyStorageNoAttrs:
    def __init__(self, start_storage: List[str]):
        self.start_storage = start_storage

        self._internal_storage: List[str] = copy(self.start_storage)
        self._secondary_internal_storage: List[str] = []

    def transfer_to_secondary(self) -> None:
        while self._internal_storage:
            self._secondary_internal_storage.append(self._internal_storage.pop())

def no_attrs_storage_builder() -> SearchStrategy[MyStorageNoAttrs]:
    return st.builds(
        MyStorageNoAttrs,
        start_storage=st.lists(st.text(), min_size=10, max_size=50)
    )

# This test succeeds!
@given(test_storage=no_attrs_storage_builder())
def test_persistent_storage_no_attrs(test_storage: MyStorageNoAttrs) -> None:
    assert len(test_storage._internal_storage) == len(test_storage.start_storage)
    test_storage.transfer_to_secondary()
    assert len(test_storage._secondary_internal_storage) == len(test_storage.start_storage)

With attrs test:

from copy import copy
from typing import List

import attr
from hypothesis import given
from hypothesis import strategies as st
from hypothesis.strategies import SearchStrategy

@attr.s(auto_attribs=True)
class MyStorageWithAttrs:
    start_storage: List[str]

    _internal_storage: List[str] = attr.ib(init=False, default=[])
    _secondary_internal_storage: List[str] = attr.ib(init=False, default=[])

    def __attrs_post_init__(self) -> None:
        self._internal_storage = copy(self.start_storage)

    def transfer_to_secondary(self) -> None:
        while self._internal_storage:
            self._secondary_internal_storage.append(self._internal_storage.pop())

def attrs_storage_builder() -> SearchStrategy[MyStorageWithAttrs]:
    return st.builds(
        MyStorageWithAttrs,
        start_storage=st.lists(st.text(), min_size=10, max_size=50)
    )

# This test fails!
@given(test_storage=attrs_storage_builder())
def test_persistent_storage_attrs(test_storage: MyStorageWithAttrs) -> None:
    assert len(test_storage._internal_storage) == len(test_storage.start_storage)
    test_storage.transfer_to_secondary()
    assert len(test_storage._secondary_internal_storage) == len(test_storage.start_storage)  # <--- This assertion fails

# If I reset the secondary storage the tests passes
@given(test_storage=attrs_storage_builder())
def test_persistent_storage_attrs_with_reset(test_storage: MyStorageWithAttrs) -> None:
    test_storage._secondary_internal_storage = []  # <- Cleaning up the secondary storage

    assert len(test_storage._internal_storage) == len(test_storage.start_storage)
    test_storage.transfer_to_secondary()
    assert len(test_storage._secondary_internal_storage) == len(test_storage.start_storage)

In this test examples, it looks like that in the case of the class with attr the the _secondary_internal_storage variable is being passed between runs, using the setting verbosity=Vervosity.debug I get the following output:

Reusing examples from database
Generating new examples
Trying example: test_persistent_storage_attrs(
    test_storage=MyStorageWithAttrs(start_storage=['0'], _internal_storage=['0'], _secondary_internal_storage=[]),
)
6 bytes [[[1, [[1, [0, 0]], 0]], 0]] -> Status.VALID, 
Trying example: test_persistent_storage_attrs(
    test_storage=MyStorageWithAttrs(start_storage=['0'], _internal_storage=['0'], _secondary_internal_storage=['0']),
)
Traceback (most recent call last):
  File "/home/rodrigo/programations/horus/horus-api-gateway/.venv/lib/python3.6/site-packages/hypothesis/core.py", line 656, in _execute_once_for_engine
    result = self.execute_once(data)
  File "/home/rodrigo/programations/horus/horus-api-gateway/.venv/lib/python3.6/site-packages/hypothesis/core.py", line 611, in execute_once
    result = self.test_runner(data, run)
  File "/home/rodrigo/programations/horus/horus-api-gateway/.venv/lib/python3.6/site-packages/hypothesis/executors.py", line 52, in default_new_style_executor
    return function(data)
  File "/home/rodrigo/programations/horus/horus-api-gateway/.venv/lib/python3.6/site-packages/hypothesis/core.py", line 607, in run
    return test(*args, **kwargs)
  File "/home/rodrigo/programations/horus/horus-api-gateway/tests/test_test_persistence.py", line 36, in test_persistent_storage_attrs
    @given(test_storage=attrs_storage_builder())
  File "/home/rodrigo/programations/horus/horus-api-gateway/.venv/lib/python3.6/site-packages/hypothesis/core.py", line 531, in test
    result = self.test(*args, **kwargs)
  File "/home/rodrigo/programations/horus/horus-api-gateway/tests/test_test_persistence.py", line 40, in test_persistent_storage_attrs
    assert len(test_storage._secondary_internal_storage) == len(test_storage.start_storage)  # <--- This assertion fails
AssertionError: assert 2 == 1

Hopefully this output helps to illustrate that the field _secondary_internal_storage is somehow "persisting" between runs.

Reading the builder strategy doc I saw that hypothesis will try to infer required fields from attrs classes, maybe that is the underlying cause of this behavior?

I'm using:

Zac-HD commented 4 years ago

This isn't a Hypothesis problem, it's because you're using a mutable default argument - attr.ib(..., default=[]) should instead be attr.ib(..., factory=list). See https://www.attrs.org/en/stable/examples.html#defaults and this blog post for a longer explanation.

So Hypothesis did find a bug, though maybe not how you expected it to!

rmoretto commented 4 years ago

Oh, I see! I'm feeling kinda stupid, should have paid more attention to the documentation, thank you for the response!

Zac-HD commented 4 years ago

No worries, it happens to all of us. Feel proud that your bug report was so good that I could easily reproduce the problem - in fact good enough that I could diagnose the problem without reproducing it!