Page MenuHomeSoftware Heritage

Fix deposit atom test to actually check the response message
ClosedPublic

Authored by ardumont on Sep 26 2020, 6:55 PM.

Details

Summary

Prior to this commit, the actual test was wrongfully ok because the slug was
missing. This resulted in a 400 response which passed because we missed
checking the actual response content. This now fixes it.

Depends on D4052

Test Plan

tox

Diff Detail

Repository
rDDEP Push deposit
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 D4057 (id=14312)

Could not rebase; Attempt merge onto b820c61d67...

Merge made by the 'recursive' strategy.
 swh/deposit/api/deposit_update.py                  |  38 +++--
 swh/deposit/tests/api/test_deposit_atom.py         |   2 +
 swh/deposit/tests/api/test_deposit_update.py       | 171 ++++++++++++++++++++-
 .../tests/data/atom/entry-data-empty-body.xml      |   2 +-
 4 files changed, 191 insertions(+), 22 deletions(-)
Changes applied before test
commit 05506c1c47851159d237bb81c42cb50badae0fbc
Merge: b820c61d 7271e750
Author: Jenkins user <jenkins@localhost>
Date:   Sat Sep 26 16:55:32 2020 +0000

    Merge branch 'diff-target' into HEAD

commit 7271e750bca43692fb88b4fbd9126e88462460ac
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Sep 26 18:37:50 2020 +0200

    Fix deposit atom test to actually check the response message
    
    Prior to this commit, the actual test was wrongfully ok because the slug was
    missing. This resulted in a 400 response which passed because we missed
    checking the actual response content. This now fixes it.

commit bb0167a8945c12a22e8d63e42b291dc414b59525
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Sep 25 15:15:37 2020 +0200

    Fix transition deposit to deposited state with empty post on SE IRI
    
    Prior to this the content-length could be None instead of 0, failing the
    transition.
    
    As the coverage was missing, that peculiar case was missed. This fixes it.

commit c6debbd53a1c055be30abbf738fb81aed382da49
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Sep 25 14:28:10 2020 +0200

    test: Update missing scenario on update metadata with multipart (post)

commit 8c40c1e093913734549590a5893d4d610e331de3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Sep 25 14:14:20 2020 +0200

    test: Update missing scenario on update metadata with multipart (put)

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

There should be another diff in regards to that test.
At least to add a file similar to the original one (with the xmlns namespace...) which should pass the same way... (heads up, it does not...)
Currently, the check on emptiness fails with that xmlns, why i removed it here (as i'm being lazy right now).
Like i said, I'll adapt in another diff. I don't want to be sidetracked too much right now.

Note that there should be other follow-up on similar other "bad request scenario" false positive (because missing the check on response.content).

Build is green

Patch application report for D4057 (id=14319)

Could not rebase; Attempt merge onto b820c61d67...

Updating b820c61d..45c7c35e
Fast-forward
 swh/deposit/api/deposit_update.py                  |  38 +++--
 swh/deposit/tests/api/test_deposit_atom.py         |   2 +
 swh/deposit/tests/api/test_deposit_update.py       | 171 ++++++++++++++++++++-
 .../tests/data/atom/entry-data-empty-body.xml      |   2 +-
 4 files changed, 191 insertions(+), 22 deletions(-)
Changes applied before test
commit 45c7c35e471ce28529bc495a076d10e2c67b9cf7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Sep 26 18:37:50 2020 +0200

    Fix deposit atom test to actually check the response message
    
    Prior to this commit, the actual test was wrongfully ok because the slug was
    missing. This resulted in a 400 response which passed because we missed
    checking the actual response content. This now fixes it.

commit b04c457d65689ae5e278edac7faee34bb3a2908f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Sep 25 15:15:37 2020 +0200

    Fix transition deposit to deposited state with empty post on SE IRI
    
    Prior to this the content-length could be None instead of 0, failing the
    transition.
    
    As the coverage was missing, that peculiar case was missed. This fixes it.

commit f26ea4492695238164e8ac1bcec984feed68ed9d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Sep 25 14:28:10 2020 +0200

    test: Update missing scenario on update metadata with multipart (post)

commit 251a0031eada21011d1683e6d755c1813830b97b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Sep 25 14:14:20 2020 +0200

    test: Update missing scenario on update metadata with multipart (put)

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

vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/deposit/tests/data/atom/entry-data-empty-body.xml
2

I'm guessing you didn't mean to put it in this diff either?

This revision now requires changes to proceed.Sep 28 2020, 12:11 PM
swh/deposit/tests/data/atom/entry-data-empty-body.xml
2

yes, i want it.
It fails otherwise.

I need to upgrade the code then.

This revision is now accepted and ready to land.Sep 28 2020, 1:04 PM