Fix swh_model_data hardcoded id values
ClosedPublic
Actions

Authored by douardda on Apr 23 2021, 5:25 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Commits

rDMOD446bd2b167c3: Fix swh_model_data hardcoded id values

Summary

and add a test to keep them correct.

Diff Detail

Repository

rDMOD Data model

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

douardda created this revision.Apr 23 2021, 5:25 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptApr 23 2021, 5:25 PM

Build is green

Patch application report for D5589 (id=19981)

Rebasing onto 1f6b3b9d5b...

Current branch diff-target is up to date.

Changes applied before test

commit 446bd2b167c340899c4d81b1e071173648f0f4e2
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Apr 23 17:24:08 2021 +0200

    Fix swh_model_data hardcoded id values
    
    and add a test to keep them correct.

See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/320/ for more details.

Harbormaster completed remote builds in B20982: Diff 19981.Apr 23 2021, 5:27 PM

douardda requested review of this revision.Apr 23 2021, 5:27 PM

ardumont added a subscriber: ardumont.Apr 23 2021, 5:30 PM

ardumont added inline comments.

swh/model/tests/swh_model_data.py
123	what about removing those. AFAIR, it should be computed by itself? (just a thought)

douardda added inline comments.Apr 26 2021, 2:29 PM

swh/model/tests/swh_model_data.py
123	as I said in D5587, I prefer keeping them: it makes debugging failed tests easier it makes obvious if for some reason the computed id changes (e.g. update test data) that may have unforseen side effects.

ardumont added inline comments.Apr 26 2021, 2:30 PM

swh/model/tests/swh_model_data.py
123	ok, fair points. (I did not see the other diff as it's still in draft, thx for the heads up).

ardumont added inline comments.Apr 26 2021, 2:33 PM

swh/model/tests/test_swh_model_data.py
24	for some reason, it seems to not run according to the report thus that suggestion ^

douardda added inline comments.Apr 26 2021, 4:06 PM

swh/model/tests/test_swh_model_data.py
24	don't think this is needed, this assertion is made in `test_swh_model_data` above. What makes you think this is needed?

ardumont added inline comments.Apr 26 2021, 4:08 PM

swh/model/tests/test_swh_model_data.py
24	because the coverage report is saying that it does not run (it's orange and not blue on the right here, in the forge ui diff view) So if it does not run, modifying like so should raise (as far as i could tell without running anything ;) Otherwise, it's simply the coverage report thingy which is buggy.

douardda added inline comments.Apr 27 2021, 10:12 AM

swh/model/tests/test_swh_model_data.py
24	Don't know why it is reported as not covered, makes no sense to me... Note that it also reports the whole body of the previous test function (test_swh_model_data) as not covered, so...