Closed Bug 1085698 Opened 7 years ago Closed 7 years ago

Fix and test review interaction with Bugzilla

Categories

(MozReview Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gps, Unassigned)

References

Details

Attachments

(1 file)

42 bytes, text/x-review-board-request
smacleod
: review+
Details
We have some gaps in test coverage for review board. I'll be writing some tests to round out the test coverage.
Attached file bz://1085698/gps
/r/238 - reviewboard: add a test for publishing against an invalid bug

Pull down this commit:

hg pull review -r 77aa12223b49a42d0bc30282f45449631f14c4ce
Changing purpose of bug a little bit because as part of writing tests, I found a bug causing review flags to not get set in Bugzilla.
Blocks: 1021929
Status: NEW → ASSIGNED
Summary: Write moar tests → Fix and test review interaction with Bugzilla
/r/239 - bugzilla: add flags to dump-bugs output
/r/238 - reviewboard: add a test for publishing against an invalid bug
/r/240 - reviewboard: add a create-review command
/r/241 - reviewboard: add a test that verifies Bugzilla review interaction
/r/242 - reviewboard: add an add-reviewer mach command
/r/243 - reviewboard: properly set review flag requestee (bug 1085698)

Pull down these commits:

hg pull review -r 0cf28a587b574a1121bd20cfcd69813f01a88590
Comment on attachment 8508312 [details]
bz://1085698/gps

The good news is this patch series fixes the bug where RB doesn't add the f? flag to Bugzilla :)
Attachment #8508312 - Flags: review?(smacleod)
Attachment #8508312 - Flags: review?(mcote)
https://reviewboard-dev.allizom.org/r/240/#review199

Ship-It!

::: testing/vcttesting/reviewboard/mach_commands.py
(Diff revision 1)
> +    @CommandArgument('--public', action='store_true',
> +            help='Whether to make this review public')

allowing public=False is mostly useless since we can't add comments to the review after, and we can't publish it.

We'll probably end up implementing methods so that we can craft reviews with comments in the tests, so I guess that's fine for now.
https://reviewboard-dev.allizom.org/r/243/#review198

Fix it, then Ship-It!

::: pylib/rbbz/rbbz/extension.py
(Diff revision 1)
> +from rbbz.models import (
> +    BugzillaUserMap,
> +    get_or_create_bugzilla_users,
> +)

Can we stick with the style convention used above (it's what RB core uses)
``` python
from rbbz.models import (BugzillaUserMap,
                         get_or_create_bugzilla_users)
```

::: pylib/rbbz/rbbz/extension.py
(Diff revision 1)
> +    for u in review_request_draft.target_people.all():
> +        if not using_bugzilla:
> +            reviewers.append(u.get_username())
> +
> +        bum = BugzillaUserMap.objects.get(user=u)
> +
> +        user_data = b.get_user_from_userid(bum.bugzilla_user_id)

I wonder how slow this is going to be for a large number of reviewers...
https://reviewboard-dev.allizom.org/r/237/#review204

Fix the one commit, then Ship-It all.
Attachment #8508312 - Flags: review+
Attachment #8508312 - Flags: review?(smacleod)
https://reviewboard-dev.allizom.org/r/240/#review205

> allowing public=False is mostly useless since we can't add comments to the review after, and we can't publish it.
> 
> We'll probably end up implementing methods so that we can craft reviews with comments in the tests, so I guess that's fine for now.

Yeah, this confused me a little as I was developing the patch. I had to look and see what the HTML was doing on reviewboard-dev to grasp at how reviews are stored and incrementally updated.

We can always improve this later.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Attachment #8508312 - Flags: review?(mcote)
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.