Details
- Reviewers
anlambert - Group Reviewers
Reviewers - Maniphest Tasks
- T3266: Improve save code now failed/uneventful status reporting
- Commits
- rDWAPPS4a6d884a7449: origin_save: Add docstring and some test scenarios
tox
Diff Detail
- Repository
- rDWAPPS Web applications
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 20855 Build 32362: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 32361: arc lint + arc unit
Event Timeline
swh/web/tests/common/test_origin_save.py | ||
---|---|---|
240 | note that this is confusing when one wants to override the date [1] That means we can remove that line and let the default django behavior occur ;) [1] I tried locally at some point to try and push some date beyong the threshold. I did | |
307 | because that's what the code is doing, it overwrite the visit_date to none for that case. | |
338 | That test is confusing but so is the current behavior so status quo ;) |
Build is green
Patch application report for D5554 (id=19846)
Rebasing onto f3f1b7c84a...
Current branch diff-target is up to date.
Changes applied before test
commit d31ab3d188b567187f695dbb30d7513c77634657 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Mon Apr 19 17:56:06 2021 +0200 origin_save: Add docstring and some test scenarios Related to T3266
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/691/ for more details.
Build is green
Patch application report for D5554 (id=19852)
Rebasing onto c908051ca7...
Current branch diff-target is up to date.
Changes applied before test
commit 4a6d884a7449bf01c1ce8fda24ab6286fb887129 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Mon Apr 19 17:56:06 2021 +0200 origin_save: Add docstring and some test scenarios Related to T3266
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/695/ for more details.
Can you use exhaustive comparisons instead of len(sors) == 1 + comparing items?
swh/web/tests/common/test_origin_save.py | ||
---|---|---|
264 | I don't think this deserves a comment |
Can you use exhaustive comparisons instead of len(sors) == 1 + comparing items?
Ok, i'll check.
I just adapted according to how that was done.
But right, might as well improve along the way.
swh/web/tests/common/test_origin_save.py | ||
---|---|---|
264 | i'm pretty sure you might have asked me why the change without the comment ;) |
We could but in another diff.
Looks good to me otherwise.
swh/web/tests/common/test_origin_save.py | ||
---|---|---|
240 | Right, I guess you can remove that line in a separate commit. | |
264 | agreed | |
307 | Indeed, and it should not do it, that's a mistake in the save code now code. As a visit has been made, the date should not be discarded. To investigate later. | |
338 | Because the behavior prior exploiting visit statuses was to check scheduler task statuses (because only that info was available at the time). I guess we could mark a save request as succeeded only if a visit status has been found now (in another diff). |
We could but in another diff.
ok ;)
swh/web/tests/common/test_origin_save.py | ||
---|---|---|
338 | yes, quite. I planned to update the behavior and modify some more those tests accordingly |