tox
Details
Details
- Reviewers
olasd vlorentz - Group Reviewers
Reviewers - Commits
- rDSTOc49890105e53: storage*: Allow origin-visit-get-latest to filter on type
Diff Detail
Diff Detail
- Repository
- rDSTO Storage manager
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
| swh/storage/tests/test_storage.py | ||
|---|---|---|
| 1797 | mmmph, wrong location, that should go in the test around origin-visit-get-latest (we are in the origin-visit-status-add test here ;) | |
Comment Actions
Build is green
Patch application report for D3311 (id=11720)
Rebasing onto 822d96b580...
Current branch diff-target is up to date.
Changes applied before test
commit f3a9b7ef18c878a5e161a7308aee9d305069c3d4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 10:11:47 2020 +0200
storage*: Allow origin-visit-get-latest to filter on typeSee https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/302/ for more details.
Comment Actions
- Move assertion on origin-visit-get-latest for type to the right test
- Add types sample in the interface docstring for the endpoint
- (Rework testt origin-visit-get-latest to deal with model object)
Comment Actions
Build was aborted
Patch application report for D3311 (id=11721)
Rebasing onto 822d96b580...
Current branch diff-target is up to date.
Changes applied before test
commit 7edbe486912faf5ddb8d5b543e768cd13cce1402
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 10:11:47 2020 +0200
storage*: Allow origin-visit-get-latest to filter on typeLink to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/303/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/303/console
Comment Actions
Build is green
Patch application report for D3311 (id=11722)
Rebasing onto 822d96b580...
Current branch diff-target is up to date.
Changes applied before test
commit 87912e041db5711dc95a596a91910004498bb717
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 10:11:47 2020 +0200
storage*: Allow origin-visit-get-latest to filter on typeSee https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/304/ for more details.
Comment Actions
I'm on the fence about accepting this with the mega test as is; The logic is sound, so I'm not too worried about the test being... a lot. But still :D
| swh/storage/db.py | ||
|---|---|---|
| 675 | = None to avoid an api break? | |
| swh/storage/tests/test_storage.py | ||
| 2073 | This test is now 180 lines long. Could we consider splitting it into more readable chunks? I think comparing the results for various filters should be split into separate tests, instead of done sequentially in a single huge test (that I have a hard time following). | |
| swh/storage/db.py | ||
|---|---|---|
| 676–677 | ah yes, we are in db, not in the api. So, that means, i can remove all defaults since it will be called from storage with them set (except for cur baybe). | |
| swh/storage/interface.py | ||
| 868 | well, i'm unsure about this. I see what you mean but also i see yagni... (maybe) and i'd like also to land
Like Alice Cooper said, "nothing is free" (as in paid-service)! :D I'll have to rewrite stuff in all backends again and tests for it and those Oh and also right now i already need to rewrite to extract tests in another | |
| swh/storage/tests/test_storage.py | ||
| 2073 | I'll start by getting the type filtering test in another test for that diff as that makes sense here. | |
Comment Actions
- Split "filter with type" tests into their own test method
- Drop unneeded defaults to db.origin-visit-get-latest
Note:
Regarding the filtering on type`S`, even though the suggestion sounds fair, I'm
tending towards not wanting to modify it (from type: Optional[str] to types:
Optional[Iterable[str]])
Because, right now i don't see a use for it. But also because we are only on
the origin_visit_get_latest endpoint here. If we want more type filtering, it
might make more sense on broader endpoints, origin_visit_get(_all?) or
something.
Comment Actions
Drop removal of assertions in wrong test (<- see the length of those tests
confuse me as well to the point where i took wrong instructions out ¯\_(ツ)_/¯)
Comment Actions
Build is green
Patch application report for D3311 (id=11724)
Rebasing onto 822d96b580...
Current branch diff-target is up to date.
Changes applied before test
commit 3797c1eda7b68958fe5f631fc0e29094ee662763
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 10:11:47 2020 +0200
storage*: Allow origin-visit-get-latest to filter on typeSee https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/305/ for more details.
Comment Actions
Build is green
Patch application report for D3311 (id=11726)
Rebasing onto 822d96b580...
Current branch diff-target is up to date.
Changes applied before test
commit c49890105e5314500ea0911021088883bf96f0ed
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 10:11:47 2020 +0200
storage*: Allow origin-visit-get-latest to filter on typeSee https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/307/ for more details.