1350407 - Staging a commit creates a set of hard-coded diff attachments on a hard-coded bug in BMO

Status: RESOLVED FIXED

(Conduit :: General, enhancement)

Comment 1

[David Walsh :davidwalsh]

Attachment: commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

Review commit: https://reviewboard.mozilla.org/r/123512/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/123512/

Comment 2

[David Walsh :davidwalsh]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

https://reviewboard.mozilla.org/r/123512/#review125980

https://trello.com/c/6OtBECna/200-2-staging-a-commit-creates-a-set-of-hard-coded-diff-attachments-on-a-hard-coded-bug-in-bmo

Demonstrate that POSTing to /iterations/ generates attachments in BMO using hard-coded data.

POSTing to the /iterations/ endpoint calls trigger_review() with a hard-coded diff.  The review code then uses a hard-coded pretend hg response (hard-coded Python dict?) to pretend it has spoken to hg, and then passes the fake hard-coded response data on to the bmo attachment creation code.

Test with mocks at the boundaries?
- calling trigger_review() should call bmo_update_attachments()

**AC:**

- POST to /iterations/ appends a diff attachment to a pre-determined bug in BMO.  (always the same diff, always posted to the same bug #)

Comment 3

[David Walsh :davidwalsh]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

https://reviewboard.mozilla.org/r/123512/#review125984

Please let me know if any/none/what makes sense here.  I feel I'm close but missing a few minor things to make this work.

::: commitindex/commitindex/reviews/bugzilla.py:71
(Diff revision 1)
>              )
>  
>          try:
>              data = json.loads(response.content.decode('utf-8'))
>          except:
> -            raise BugzillaError(400, "Error decoding JSON data")
> +            raise BugzillaError(400, "Error decoding JSON data %s" % response.content.decode('utf-8'))

Temporary, just trying to figure out what the invalid response content is.

::: commitindex/commitindex/reviews/triggers.py:12
(Diff revision 1)
> -def trigger_review(*args, **kwargs):
> +
> +def trigger_review(commits):
>      """Trigger review creation for an Iteration."""
> -    pass
> +
> +    # TODO: Is there a way we don't have to hardcode this address?
> +    bz = Bugzilla('http://mountebank:2525')

What's the best way to get to bmo for the sake of this card?

::: commitindex/tests/test_bmo_attachments.py:153
(Diff revision 1)
> +    """Tests that a new bugzilla attachment is created for each commit"""
> +    commits = [{"id":"1"}, {"id":"1"}, {"id":"1"}]
> +
> +    bugzilla.create_attachment = MagicMock(1, commits)
> +    try:
> +        trigger_review(commits)

Using a `try` here because I get an unholy error back from the bugzilla service:

https://gist.github.com/darkwing/e3c13cb77b8c46ec831c3554dbb5e32f

It now occurs to me that I'm using the "official" mountebank in `triggers.py` and the tests use a fake/different mountebank.

Comment 4

[Māris Fogels [:mars] (please needinfo)]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

https://reviewboard.mozilla.org/r/123512/#review126014

::: commitindex/tests/test_bmo_attachments.py:146
(Diff revision 1)
> [e-mail]
> +def test_trigger(bugzilla):

You can remove the `@pytest` decorator.

Replace the `(bugzilla)` fixture with the `(monkeypatch)` fixture.

::: commitindex/tests/test_bmo_attachments.py:151
(Diff revision 1)
> [e-mail]
> +def test_trigger(bugzilla):
> +    """Tests that a new bugzilla attachment is created for each commit"""
> +    commits = [{"id":"1"}, {"id":"1"}, {"id":"1"}]
> +
> +    bugzilla.create_attachment = MagicMock(1, commits)

You need to get a mock bugzilla.create_attachment object first.  I think this should do it:

```
bugzilla = MagicMock()
monkeypatch.setattr("commitindex.reviews.triggers.Bugzilla", bugzilla)
```

::: commitindex/tests/test_bmo_attachments.py:151
(Diff revision 1)
> [e-mail]
> +def test_trigger(bugzilla):
> +    """Tests that a new bugzilla attachment is created for each commit"""
> +    commits = [{"id":"1"}, {"id":"1"}, {"id":"1"}]
> +
> +    bugzilla.create_attachment = MagicMock(1, commits)

You need to get a mock bugzilla.create_attachment object first.  I think this should do it:

```
bugzilla = MagicMock()
monkeypatch.setattr("commitindex.reviews.triggers.Bugzilla", bugzilla)
```

Comment 5

[David Walsh :davidwalsh]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123512/diff/1-2/

Comment 6

[David Walsh :davidwalsh]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

https://reviewboard.mozilla.org/r/123512/#review126482

::: commitindex/commitindex/reviews/triggers.py:12
(Diff revision 2)
> -def trigger_review(*args, **kwargs):
> +
> +def trigger_review(commits):
>      """Trigger review creation for an Iteration."""
> -    pass
> +
> +    # TODO: Is there a way we don't have to hardcode this address?
> +    bz = Bugzilla('http://mountebank:2525')

We'll need to figure out how to turn this  address from a hardcoded string to an environment var -> app config -> usage here.

::: commitindex/commitindex/reviews/triggers.py:23
(Diff revision 2)
> ++++ b/dirs/source.py
> +@@ -1,8 +1,17 @@
> +
> ++from commitindex.reviews.bugzilla import Bugzilla"""
> +
> +        commit['data'] = diff

dkl: is this needed?

Comment 7

[David Lawrence [:dkl]]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

https://reviewboard.mozilla.org/r/123512/#review126482

> dkl: is this needed?

You do need to set commit['data'] to the diff content, yes. You could shortcut and do:

        commit['data'] = """diff --git a/dirs/source.py b/dirs/source.py

--- a/dirs/source.py

+++ b/dirs/source.py

@@ -1,8 +1,17 @@

+from commitindex.reviews.bugzilla import Bugzilla"""

Comment 8

[David Walsh :davidwalsh]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123512/diff/2-3/

Comment 9

[David Walsh :davidwalsh]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123512/diff/3-4/

Comment 10

[David Walsh :davidwalsh]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123512/diff/4-5/

Comment 11

[David Walsh :davidwalsh]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/123512/diff/5-6/

Comment 12

[Māris Fogels [:mars] (please needinfo)]

Comment on attachment 8851074 [details]
commitindex: trigger_review creates diffs on bugzilla when called (Bug 1350407).

https://reviewboard.mozilla.org/r/123512/#review127316

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/automation/conduit/rev/2ec31f98ed9c
commitindex: trigger_review creates diffs on bugzilla when called . r=mars