Merge lp://staging/~jml/launchpad/searchTasks-modified-since-590535 into lp://staging/launchpad
| Status: | Merged |
|---|---|
| Approved by: | Deryck Hodge |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 11055 |
| Proposed branch: | lp://staging/~jml/launchpad/searchTasks-modified-since-590535 |
| Merge into: | lp://staging/launchpad |
| Diff against target: |
302 lines (+114/-22) 6 files modified
lib/lp/bugs/interfaces/bugtarget.py (+9/-3) lib/lp/bugs/interfaces/bugtask.py (+5/-3) lib/lp/bugs/model/bugtarget.py (+2/-1) lib/lp/bugs/model/bugtask.py (+5/-0) lib/lp/bugs/stories/webservice/xx-bug.txt (+9/-0) lib/lp/bugs/tests/test_bugtask.py (+84/-15) |
| To merge this branch: | bzr merge lp://staging/~jml/launchpad/searchTasks-modified-since-590535 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Deryck Hodge (community) | code | Approve | |
|
Review via email:
|
|||
Description of the change
This branch fixes bug 590535 by adding a new parameter to searchTasks to allow people to search for bugs that have been modified since a certain date.
I put the tests in test_bugtask.py, since it seemed the most sensible place. I may have duplicated tests elsewhere, since I wanted to be extra confident with these, since I didn't know what I was doing.
The test_suite method change simply makes the test runner find all of the tests in the module, without them having to be picked out and named individually.
The change to xx-bug.txt is kind of a cheat, in that it doesn't test much of the behaviour. However, since I know the method works, all I'm really testing is that passing in modified_since actually has an effect on the results. I think the test works for that.
I didn't really know whether the query should be for date_last_updated >= modified_since or date_last_updated > modified_since. I didn't think hard about it, but I couldn't really see the difference, so I opted for the one needing the fewest key presses.
I'm beside myself with scrumptious anticipation as I wait dotingly on your review.
jml
Hi, Jono.
This branch looks good to me. I was going to suggest changing the test name to be less generic about search and more about the specific case you were covering, but realizing we have no unit test coverage for searchTasks, I'm fine with this now, even if there is some tiny duplication. I would like to see us eventually move tests out of doc/bugtask-
Also, limiting to tasks after but not including modified_since feels right to me, given the naming. The way you've done it here is what I would expect when using the parameter.
Finally, I agree with your choice to simply demonstrate the webservice and rely on unit test coverage. I have taken this approach myself recently.
Thanks for the work!
Cheers,
deryck