Page MenuHomeSoftware Heritage

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

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

Diff Detail

Repository
rDSTO Storage manager
Branch
feature/typing
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7957
Build 11462: tox-on-jenkinsJenkins
Build 11461: arc lint + arc unit

Event Timeline

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.Sep 23 2019, 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.Sep 23 2019, 1:24 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 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?

  • init.py: switch to documented way of extending path
  • mypy: ignore swh.journal to work-around dependency loop
  • 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
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.Oct 1 2019, 8:24 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