Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

inject_define uses the wrong bindings if different classes with the same name are declared in the same file #43

Closed
xiyan128 opened this issue Jul 17, 2024 · 1 comment · Fixed by #62
Closed

inject_define uses the wrong bindings if different classes with the same name are declared in the same file #43

xiyan128 opened this issue Jul 17, 2024 · 1 comment · Fixed by #62

Comments

@xiyan128
Copy link
Contributor

https://github.com/duolingo/minject/actions/runs/9977074810/job/27570812505

A test in tests/test_registry_attrs.py sometimes fails.
@xiyan128 xiyan128 changed the title Fix flaky test Fix flaky test in test_registry_attrs.py Jul 17, 2024
@bcmills bcmills mentioned this issue Oct 4, 2024
Merged
@bcmills
Copy link
Contributor

bcmills commented Oct 7, 2024

This failure occurs when test_normal_instantiation is run before test_registry_instantiation:

bryan@bryan-macbook-pro:~/src/minject$ while hatch test --verbose --python 3.10 --cover-quiet --randomize --retries 5 --retry-delay 3 tests/test_registry_attrs.py::{test_registry_instantiation,test_normal_instantiation}; do true; done
─────────────────────────────────────────────────── hatch-test.py3.10 ────────────────────────────────────────────────────
================================================== test session starts ===================================================
platform darwin -- Python 3.10.14, pytest-8.3.3, pluggy-1.5.0 -- /Users/bryan/Library/Application Support/hatch/env/virtual/minject/gwfjBA3B/hatch-test.py3.10/bin/python3
cachedir: .pytest_cache
Using --randomly-seed=2461753379
rootdir: /Users/bryan/src/minject
configfile: pyproject.toml
plugins: randomly-3.15.0, rerunfailures-14.0, mock-3.14.0, xdist-3.6.1
collected 2 items

tests/test_registry_attrs.py::test_registry_instantiation PASSED                                                   [ 50%]
tests/test_registry_attrs.py::test_normal_instantiation PASSED                                                     [100%]

=================================================== 2 passed in 0.07s ====================================================
Combined data file .coverage.bryan-macbook-pro.6663.XjqDqBJx
Combined data file .coverage.bryan-macbook-pro.7132.XxecQjfx
─────────────────────────────────────────────────── hatch-test.py3.10 ────────────────────────────────────────────────────
================================================== test session starts ===================================================
platform darwin -- Python 3.10.14, pytest-8.3.3, pluggy-1.5.0 -- /Users/bryan/Library/Application Support/hatch/env/virtual/minject/gwfjBA3B/hatch-test.py3.10/bin/python3
cachedir: .pytest_cache
Using --randomly-seed=2916041666
rootdir: /Users/bryan/src/minject
configfile: pyproject.toml
plugins: randomly-3.15.0, rerunfailures-14.0, mock-3.14.0, xdist-3.6.1
collected 2 items

tests/test_registry_attrs.py::test_normal_instantiation PASSED                                                     [ 50%]
tests/test_registry_attrs.py::test_registry_instantiation RERUN                                                    [100%]
tests/test_registry_attrs.py::test_registry_instantiation RERUN                                                    [100%]
tests/test_registry_attrs.py::test_registry_instantiation RERUN                                                    [100%]
tests/test_registry_attrs.py::test_registry_instantiation RERUN                                                    [100%]
tests/test_registry_attrs.py::test_registry_instantiation RERUN                                                    [100%]
tests/test_registry_attrs.py::test_registry_instantiation FAILED                                                   [100%]

======================================================== FAILURES ========================================================
______________________________________________ test_registry_instantiation _______________________________________________

    def test_registry_instantiation() -> None:
        @inject_define
        class TestClass:
            a: str = inject_field(binding="hello")
            b: float = inject_field(binding=10.0)
            c: int = 100
            d: str = inject_field(binding="hello", default="world")
            e: bool = inject_field(binding=True, default=False)
            f: int = field(default=1000)
            g: int = inject_field(binding=1, default=2)

            def sum_nums(self) -> int:
                return self.c + int(self.b) + self.g + self.f

        registry = Registry()
        registry_instance = registry[TestClass]
>       assert registry_instance.sum_nums() == 1111
E       assert 112 == 1111
E        +  where 112 = sum_nums()
E        +    where sum_nums = <tests.test_registry_attrs.test_registry_instantiation.<locals>.TestClass object at 0x10090ef20>.sum_nums

tests/test_registry_attrs.py:24: AssertionError
================================================ rerun test summary info =================================================
RERUN tests/test_registry_attrs.py::test_registry_instantiation
RERUN tests/test_registry_attrs.py::test_registry_instantiation
RERUN tests/test_registry_attrs.py::test_registry_instantiation
RERUN tests/test_registry_attrs.py::test_registry_instantiation
RERUN tests/test_registry_attrs.py::test_registry_instantiation
================================================ short test summary info =================================================
FAILED tests/test_registry_attrs.py::test_registry_instantiation - assert 112 == 1111
========================================= 1 failed, 1 passed, 5 rerun in 15.26s ==========================================

bcmills added a commit that referenced this issue Oct 15, 2024
@bcmills
Include the line number of the class declaration in binding keys
0a36a46 
Previously, we were using only the class name and filename as the
binding key, which caused collisions if the same name was used to
declare more than one (local) class in the same file (as in
test_registry_attrs.py).

In the process, this simplifies the process of getting the stack frames:
we had been retrieving the same frames (redundantly) in two different
ways in the process of defining a field, but the potential sharing was
somewhat obscured because the code was split into very short functions.

Fixes #43.
@bcmills bcmills mentioned this issue Oct 15, 2024
Merged
@bcmills bcmills changed the title Fix flaky test in test_registry_attrs.py inject_define uses the wrong bindings if different classes with the same name are declared in the same file Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Include the line number of the class declaration in binding keys duolingo/minject
2 participants
@bcmills @xiyan128