Details

swh/model/tests/generate_testdata.py
21	the data model accepts any signed 64-bit number as timestamp
swh/model/tests/test_generate_testdata.py
33–35	The test should check url unicity. Currently, you test only checks unicity of (type, url).
49–54	This set should check unicity of each of the hashes.

This revision now requires changes to proceed.Oct 29 2019, 2:45 PM

rebase + add missing dep on pytz

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DMOD/job/tox/146/
See console output for more information: https://jenkins.softwareheritage.org/job/DMOD/job/tox/146/console

Harbormaster failed remote builds in B8675: Diff 7469!Oct 29 2019, 2:50 PM

douardda added inline comments.Oct 29 2019, 2:52 PM

swh/model/tests/generate_testdata.py
21	sure but do we need such a wide space?
swh/model/tests/test_generate_testdata.py
33–35	right, type is still around!

update test_gen_origins_x accorgin to vlorentz comment

Build is green
See https://jenkins.softwareheritage.org/job/DMOD/job/tox/147/ for more details.

Harbormaster completed remote builds in B8676: Diff 7470.Oct 29 2019, 2:57 PM

vlorentz requested changes to this revision.Oct 29 2019, 4:18 PM

vlorentz added inline comments.

swh/model/tests/generate_testdata.py
21	Yes, because we want tests to check that negative and big values are supported too.
swh/model/tests/test_generate_testdata.py
33–35	`models = {Origin.from_dict(d).url for d in origins}`

This revision now requires changes to proceed.Oct 29 2019, 4:18 PM

douardda added inline comments.Oct 29 2019, 4:27 PM

swh/model/tests/generate_testdata.py
21	IMHO this is something to test specifically, not that useful for 'general-purpose' generated datasets. If we want/need to "check that negative and big values are supported", we need specifically crafted tests, not rely on random being nice with us. Also, notice that currently hypothesis_stategies.contents() do not generate ctime field at all.

douardda added inline comments.Oct 29 2019, 4:28 PM

swh/model/tests/test_generate_testdata.py
33–35	makes sense, thx

simplify a bit a couple of tests

as pointed by vlorentz

Build is green
See https://jenkins.softwareheritage.org/job/DMOD/job/tox/148/ for more details.

Harbormaster completed remote builds in B8684: Diff 7478.Oct 29 2019, 4:32 PM

vlorentz added inline comments.Oct 29 2019, 4:34 PM

swh/model/tests/generate_testdata.py
21	The same could be said of using different `PROTOCOLS`, having an IRI in `DOMAINS`, having `#` and `?` in `PATHS`, having different `CONTENT_STATUS`es, etc. If it's cheap to explore a large range of values, why not do it?

vlorentz added inline comments.Oct 29 2019, 4:37 PM

requirements.txt
8 ↗	(On Diff #7478)	should be in `requirements-tests.txt`

Oh and also, it would be nice of these gen_ functions to return model objects instead of dicts (as we want to get rid of these dicts as much as possible)

This revision now requires changes to proceed.Oct 29 2019, 4:39 PM

In D2191#51535, @vlorentz wrote:

Oh and also, it would be nice of these gen_ functions to return model objects instead of dicts (as we want to get rid of these dicts as much as possible)

Maybe, not sure we really want this (right now at least).