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.

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

test_ucode: address UnusedElaboratable warning. #25

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

kivikakk
Copy link
Contributor

Fixes #17.

This is in fact an UnusedElaboratable being reported. If you stick # amaranth: UnusedElaboratable=no at the top of ucoderom.py, it goes away.

The origin is test_ucode_layout_gen:

# This is a test by itself because creating the signature from the microcode
# assembly file can be tricky.
@pytest.mark.module(UCodeROM(main_file=StringIO(M5META_TEST_FILE),
enum_map={"bar": Bar}))
@pytest.mark.clks((1.0 / 12e6,))
def test_ucode_layout_gen(sim_mod):
_, m = sim_mod
m.elaborate(None)

It doesn't happen if you run it in isolation, I'm guessing due to how all the MustUse machinery works (or some interaction with SimulatorFixture), but one other test before it is enough:

$ pdm test tests/sim/test_top.py::test_exception tests/sim/test_ucode.py::test_ucode_layout_gen
=================================== test session starts ===================================
platform darwin -- Python 3.12.3, pytest-8.1.1, pluggy-1.4.0
rootdir: /Users/kivikakk/g/sentinel
configfile: pyproject.toml
collected 2 items

tests/sim/test_top.py .                                                             [ 50%]
tests/sim/test_ucode.py .                                                           [100%]

==================================== 2 passed in 1.44s ====================================
/Users/kivikakk/g/sentinel/src/sentinel/ucoderom.py:69: UnusedElaboratable: <amaranth.hdl._dsl.Module object at 0x1072cb1d0> created but never used
  m = Module()
UnusedElaboratable: Enable tracemalloc to get the object allocation traceback

Fragment.get calls elaborate and sets the "used" flag, so this stops the warning.

`Fragment.get` calls `elaborate` and sets the "used" flag, so this stops
the warning.
@cr1901
Copy link
Owner

cr1901 commented Jul 10, 2024

Looks fine to me... could you add a comment to the changed lines for future-me when I inevitably forget? I don't touch the Fragment stuff typically, so I'm not familiar with how it works.

$ pdm test tests/sim/test_top.py::test_exception tests/sim/test_ucode.py::test_ucode_layout_gen
=================================== test session starts ===================================
platform darwin -- Python 3.12.3, pytest-8.1.1, pluggy-1.4.0
rootdir: /Users/kivikakk/g/sentinel
configfile: pyproject.toml
collected 2 items

tests/sim/test_top.py .                                                             [ 50%]
tests/sim/test_ucode.py .                                                           [100%]

==================================== 2 passed in 1.44s ====================================
/Users/kivikakk/g/sentinel/src/sentinel/ucoderom.py:69: UnusedElaboratable: <amaranth.hdl._dsl.Module object at 0x1072cb1d0> created but never used
  m = Module()
UnusedElaboratable: Enable tracemalloc to get the object allocation traceback

Thank you for isolating the issue :). I still have to use grep -v UnusedEla to prevent my console from being overrun with spurious warnings, but at least it's a starting point.

@kivikakk
Copy link
Contributor Author

I don't really know what you would like the comment to say (I can't say I'm overly familiar with Fragment either, but I looked at where MustUse is touched and there it was!), but I've done my best!

@cr1901 cr1901 merged commit d7e6e8c into cr1901:main Jul 11, 2024
@cr1901
Copy link
Owner

cr1901 commented Jul 11, 2024

Fair enough... merged :D! Thanks for the contribution :)!

@kivikakk kivikakk deleted the fix-unused-elaboratable branch July 11, 2024 15:39
cr1901 added a commit that referenced this pull request Sep 25, 2024
test_ucode: address UnusedElaboratable warning.
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.

Obscure ResourceWarning in pytest tests
2 participants