Page MenuHomeSoftware Heritage

Fix swh_model_data hardcoded id values
ClosedPublic

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

Details

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

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.

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)

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.
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).

swh/model/tests/test_swh_model_data.py
24

for some reason, it seems to not run according to the report thus that suggestion ^

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?

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.

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...

if you are convinced it's a report covering issue, ok then ;)

This revision is now accepted and ready to land.Tue, Apr 27, 10:13 AM

if you are convinced it's a report covering issue, ok then ;)

well the print statements I've added at some point in this test were definitely printed out, so somebody is doing something...

This revision was automatically updated to reflect the committed changes.