Page MenuHomeSoftware Heritage

tests: refactor main storage tests
ClosedPublic

Authored by douardda on Oct 8 2019, 4:52 PM.

Details

Summary
  • use pytest instead of unittest.TestCase plumbing
  • extract data from the TestStorageData into a data storage_data module; this module also provide a simple helper StorageData class that mimics the original class (access by attributes),
  • implement a series of pytest fixtures for these storage specific tests,
  • get rid of most hypothesis-based tests.

Diff Detail

Repository
rDSTO Storage manager
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 8370
Build 12120: tox-on-jenkinsJenkins
Build 12119: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
ardumont added inline comments.
swh/storage/tests/conftest.py
41

'memory' (we fixed it together ;)

also olasd migrated the existing entries in storage recently.

mark test_in_memory and test_api_client as xfail

until D2095 and D2119

rebase + fix tests in tox (update mypy.ini and requirements-test.txt)

also xfail test_snapshot (until it has been ported to pytest/pytest_postgresql)

in D2117

xfail does not mean xerror...

also fix test_db to not depends on running tests via pifpaf

use memory cls for the journal_writer instead of deprecated inmemory

I think the xfails should have a comment for future readers stumbling upon this commit.

swh/storage/tests/conftest.py
49–60

not done

59

not done

91–95

not done

tox.ini
28–35

Why?

This revision now requires changes to proceed.Oct 11 2019, 4:45 PM
swh/storage/tests/conftest.py
91–95

well, I would have expected a comment about the fact there is no comment in this code stating that this function is stolen from pytest-postgresql ;-), and explaining why it is needed.

tox.ini
28–35

this chunk could be moved in D2095 I guess. I removed it since I gradually drop support for ENABLE_ORIGIN_IDS in tests.

Reactivate [testenv:py3-no-origin-ids] in tox

tox.ini
28–35

py3-no-origin-ids runs tests for the new behavior. Now tox only runs tests for the old behavior.

add some comment in conftest to explain a bit the postgres stuff

and add sha1_git as unicity key for the hypothesis based gen_contents
helper function.

douardda added inline comments.
swh/storage/tests/conftest.py
49–60

I do prefer gen_xxx since it's a function that generates these objects. xxx_lists seems less clear to me.

vlorentz added inline comments.
swh/storage/tests/conftest.py
49–60

but it doesn't follow Hypothesis' naming convention.

63–76

If we're going with the gen_ prefix, then these functions should have it too.

And swh_origins is way too different from swh_contents to have such a close name.

73

why 20?

using these many origins even for tests that won't use it wil make Hypothesis explore much more space than needed.

This revision now requires changes to proceed.Oct 11 2019, 5:32 PM
douardda added inline comments.
swh/storage/tests/conftest.py
73

Have you noticed I do not use hypothesis for exploring anything here? I'm using it as an easy way of generating (fixed-size) datasets.

But, it would make sense to use the gen_origins() in this fixture, instead of reimplementing it.

swh/storage/tests/conftest.py
63–76

I agree with you, the names for swh_origins and swh_contents are confusing here.

73

20: because I do not ask hypothesis to explore anything, I just generate ONE list of 20 (by default) origin/content objects.

swh/storage/tests/conftest.py
17

Don't we want a fixture sqldir similar to the datadir we introduced in swh-core for the sql files?

37

Isn't a memory objstorage enough?

73

But, it would make sense to use the gen_origins() in this fixture, instead of reimplementing it.

Yes, i don't get why you did not do it since it's the same code ;)

swh/storage/tests/conftest.py
81

I don't even know what's the DBJanitor you are talking about ;)

swh/storage/tests/data/storage.yml
9

?
If we use a memory objstorage, we can skip that (i guess there is some stuff replacing this somewhere ;)

13

memory but that might be fixed in other commits...

Looks good to me.

Got a couple of questions/remarks but nothing too blocking.

fix definition and usage of swh_contents and swh_origins fixtures

also add a revision that use an in memory obj storage in swh_storage fixture

revert usage for self._test_origin_ids and use_url to fix py3-no-origin-ids

also replace boolean hypothesis strategy by pytest.mark.parametrize, so we do
not need to reset the storage in these tests (since the test is truly
executed twice).

also replace boolean hypothesis strategy by pytest.mark.parametrize, so we do
not need to reset the storage in these tests (since the test is truly
executed twice).

Nice!

douardda edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Oct 14 2019, 1:19 PM
douardda edited the summary of this revision. (Show Details)

smalle rework related to skipping a few tests due to origin id handling