Page MenuHomeSoftware Heritage

typing: minimal changes to make a no-op mypy run pass
ClosedPublic

Authored by zack on Sun, Sep 22, 4:07 PM.

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

zack created this revision.Sun, Sep 22, 4:07 PM

note that there is a dependency loop between this diff and D2024, because we do have an import loop between swh-storage and swh-journal (even after D1993)

vlorentz added inline comments.
swh/storage/tests/test_storage.py
47–49

why?

zack marked an inline comment as done.Mon, Sep 23, 1:24 PM
zack added inline comments.
swh/storage/tests/test_storage.py
47–49

because without it:

swh/storage/tests/test_storage.py:3926: error: Definition of "setUp" in base class "TestStorageData" is incompatible with definition in base class "SingleDbTestFixture"
swh/storage/tests/test_storage.py:3926: error: Definition of "setUp" in base class "TestStorageData" is incompatible with definition in base class "DbTestFixture"
swh/storage/tests/test_storage.py:3983: error: Definition of "setUp" in base class "TestStorageData" is incompatible with definition in base class "SingleDbTestFixture"
swh/storage/tests/test_storage.py:3983: error: Definition of "setUp" in base class "TestStorageData" is incompatible with definition in base class "DbTestFixture"
swh/storage/tests/test_api_client.py:133: error: Definition of "setUp" in base class "TestStorageData" is incompatible with definition in base class "SingleDbTestFixture"
swh/storage/tests/test_api_client.py:133: error: Definition of "setUp" in base class "TestStorageData" is incompatible with definition in base class "DbTestFixture"

and mypy is right, as the base class and the original fixture from s.s.t.storage_testing are used together, they should have compatible signatures

zack marked an inline comment as done.Mon, Sep 23, 1:24 PM
vlorentz added inline comments.Mon, Sep 23, 1:38 PM
swh/storage/tests/test_storage.py
47–49

You should remove these arguments to setUp from the parent classes then. I don't think we need it since douardda's refactoring of test classes months ago

zack added a subscriber: douardda.Mon, Sep 23, 1:49 PM
zack added inline comments.
swh/storage/tests/test_storage.py
47–49

Actually, wrong reference in mu previous message, the *args/**kwargs come from the DbTestFixture class in swh-core, not from swh-storage.

I don't mind removing those args from there, but: (1) I cannot do that in this diff; (2) can @douardda confirm they should go?

zack updated this revision to Diff 6835.Fri, Sep 27, 10:37 AM
  • init.py: switch to documented way of extending path
zack updated this revision to Diff 6842.Fri, Sep 27, 2:09 PM
  • mypy: ignore swh.journal to work-around dependency loop
zack updated this revision to Diff 6853.Sat, Sep 28, 12:45 PM
  • typing: minimal changes to make a no-op mypy run pass
  • init.py: switch to documented way of extending path
  • mypy: ignore swh.journal to work-around dependency loop
  • storage.py: ignore typing of optional get_journal_writer import
zack updated this revision to Diff 6854.Sat, Sep 28, 1:21 PM
  • mypy.ini: be less flaky w.r.t. the packages installed in tox
zack updated this revision to Diff 6855.Sat, Sep 28, 1:26 PM
  • tox: anticipate mypy run to just after flake8
douardda added inline comments.Mon, Sep 30, 1:20 PM
swh/storage/tests/test_storage.py
47–49

I'm ok with adding these generic args here. In fact, I have a WIP to simply remove this 'data class' from the tests, but it's not ready for review yet.

So meanwhile, the path of least work is just fine.

zack marked an inline comment as done.Tue, Oct 1, 8:24 AM
zack updated this revision to Diff 6874.Tue, Oct 1, 8:25 AM
  • typing: minimal changes to make a no-op mypy run pass
  • init.py: switch to documented way of extending path
  • mypy: ignore swh.journal to work-around dependency loop
  • storage.py: ignore typing of optional get_journal_writer import
  • mypy.ini: be less flaky w.r.t. the packages installed in tox
  • tox: anticipate mypy run to just after flake8