Page MenuHomeSoftware Heritage
Differential D4057

Fix deposit atom test to actually check the response message
ClosedPublic
Actions

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

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDDEP45c7c35e471c: Fix deposit atom test to actually check the response message
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

ardumont created this revision.Sep 26 2020, 6:55 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 26 2020, 6:55 PM
ardumont mentioned this in D4013: Allow deposit metadata update on deposit already complete.Sep 26 2020, 6:57 PM
ardumont added a child revision: D4013: Allow deposit metadata update on deposit already complete.
swh-public-ci added a comment.Sep 26 2020, 6:57 PM

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.

Harbormaster completed remote builds in B15610: Diff 14312.Sep 26 2020, 6:57 PM
ardumont added a comment.Sep 26 2020, 7:01 PM

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

ardumont edited the summary of this revision. (Show Details)Sep 26 2020, 7:02 PM
ardumont added a parent revision: D4052: Fix transition deposit to deposited state with empty post on SE IRI.
ardumont updated this revision to Diff 14319.Sep 26 2020, 7:09 PM

Rebase

swh-public-ci added a comment.Sep 26 2020, 7:20 PM

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.

Harbormaster completed remote builds in B15617: Diff 14319.Sep 26 2020, 7:20 PM
vlorentz requested changes to this revision.Sep 28 2020, 12:11 PM
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
ardumont added inline comments.Sep 28 2020, 12:12 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.

vlorentz accepted this revision.Sep 28 2020, 1:04 PM
This revision is now accepted and ready to land.Sep 28 2020, 1:04 PM
Closed by commit rDDEP45c7c35e471c: Fix deposit atom test to actually check the response message (authored by ardumont). · Explain WhySep 28 2020, 1:07 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDDEP45c7c35e471c: Fix deposit atom test to actually check the response message.
ardumont mentioned this in D4069: tests: Explicit the bad request scenario error message.Sep 28 2020, 5:45 PM
ardumont mentioned this in rDDEP8d4f19a3f3cf: tests: Explicit the bad request scenario error messages.Sep 28 2020, 7:16 PM

Revision Contents
Changeset List

PathSize
swh/
deposit/
tests/
api/
2 lines
data/
atom/
2 lines

Diff 14328

View Options

swh/deposit/tests/api/test_deposit_atom.py

Loading...
View Options

swh/deposit/tests/data/atom/entry-data-empty-body.xml

Loading...
About · Developer information · Legal