1174752 - Improve autoland testing

Status: RESOLVED FIXED

(Conduit Graveyard :: Transplant, defect)

Description

[Dan Minor [:dminor]]

Most of the Autoland tests were written prior to having reviewboard and mercurial in docker containers. We're now in a position to be able to write much better integration tests and we should do so before we start landing revisions to an inbound tree.

Comment 1

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: link autoland container to hgrb (bug 1174752) r?gps

testing: link autoland container to hgrb (bug 1174752) r?gps

Comment 2

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: create autoland ldap user for testing (bug 1174752) r?gps

testing: create autoland ldap user for testing (bug 1174752) r?gps

Comment 3

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: add poll option to mach autoland status commands (bug 1174752) r?mcote

testing: add poll option to mach autoland status commands (bug 1174752) r?mcote

Autoland services jobs asynchronously so the status commands are more useful if
we poll until the jobs either succeed or fail.

Comment 4

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: add autoland startup test (bug 1174752) r?mcote

testing: add autoland startup test (bug 1174752) r?mcote

Comment 5

[Dan Minor [:dminor]]

Attachment: MozReview Request: autoland: provide better default for missing repo path (bug 1174752) r=mdoglio

autoland: provide better default for missing repo path (bug 1174752) r?mdoglio

Comment 6

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: clone created repos in autoland container (bug 1174752) r?gps

testing: clone created repos in autoland container (bug 1174752) r?gps

Comment 7

[Dan Minor [:dminor]]

Attachment: MozReview Request: testing: Update autoland tests to use clone of hgrb repositories (bug 1174752) r?mdoglio

testing: Update autoland tests to use clone of hgrb repositories (bug 1174752) r?mdoglio

Update the autoland tests to make use of the repository cloned from hgrb rather
than the "fake" repository that was used previously.

Comment 8

[Mauro Doglio [:mdoglio]]

Comment on attachment 8630454 [details]
MozReview Request: autoland: provide better default for missing repo path (bug 1174752) r=mdoglio

https://reviewboard.mozilla.org/r/12731/#review11373

::: autoland/autoland/transplant.py:16
(Diff revision 1)
> +                                   os.path.join(os.path.sep + 'repos', tree))

I find os.path.join('', 'repos', tree) a bit more readable. Feel free to drop this comment if you don't agree.

Comment 9

[Mauro Doglio [:mdoglio]]

Comment on attachment 8630456 [details]
MozReview Request: testing: Update autoland tests to use clone of hgrb repositories (bug 1174752) r?mdoglio

https://reviewboard.mozilla.org/r/12735/#review11375

::: autoland/tests/test-post-autoland-job.t:20
(Diff revision 1)
> +  pushing to ssh://172.17.42.1:$HGPORT6/test-repo

Where does this ip come from?

::: autoland/tests/test-post-autoland-job.t:84
(Diff revision 1)
> -Get pullrequest job status
> +Posting a pullrequest job without a bugid should automatically file a bug for the user
>  
> -  $ ottoland pullrequest-job-status $AUTOLAND_URL 2
> -  (200, u'{\n  "bugid": 1, \n  "destination": "mozreview", \n  "error_msg": *, \n  "landed": *, \n  "pullrequest": 1, \n  "repo": "repo", \n  "result": *, \n  "user": "user"\n}') (glob)
> +  $ ottoland post-pullrequest-job $AUTOLAND_URL user repo 1 test-repo 1 cookie http://localhost:9898
> +  (200, u'{\n  "request_id": 4\n}')
> +  $ ottoland pullrequest-job-status $AUTOLAND_URL 4
> +  (200, u'{\n  "bugid": null, \n  "destination": "test-repo", \n  "error_msg": null, \n  "landed": null, \n  "pullrequest": 1, \n  "repo": "repo", \n  "result": "", \n  "user": "user"\n}')

Is there a way to verify that the bug was actually created?

Comment 10

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/12735/#review11375

> Is there a way to verify that the bug was actually created?

Right now this will always fail because I have a fake cookie. I'm planning to fix this when we have Bugzilla tokens. I'll add a TODO for now.

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8630450 [details]
MozReview Request: testing: link autoland container to hgrb (bug 1174752) r?gps

https://reviewboard.mozilla.org/r/12723/#review11429

::: testing/vcttesting/docker.py:699
(Diff revision 1)
>              start_autoland = True

You'll want to add `start_hgrb = True` here.

::: testing/vcttesting/docker.py:846
(Diff revision 1)
> +                bind_path = os.path.abspath(os.path.dirname(self._state_path))

bind_path is unused, so you might as well kill it.

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8630451 [details]
MozReview Request: testing: create autoland ldap user for testing (bug 1174752) r?gps

https://reviewboard.mozilla.org/r/12725/#review11435

::: testing/docker/builder-autoland/autoland_id_rsa:1
(Diff revision 1)
> +-----BEGIN RSA PRIVATE KEY-----

I'm not crazy about checking in pre-generated SSH keys. But since this is testing only, I don't think it matters too much. It certainly makes things simpler.

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8630455 [details]
MozReview Request: testing: clone created repos in autoland container (bug 1174752) r?gps

https://reviewboard.mozilla.org/r/12733/#review11437

::: testing/docker/builder-autoland/Dockerfile:20
(Diff revision 1)
> -  python2.7-dev libffi6 libffi-dev libpq-dev libldap2-dev libsasl2-dev \
> +  python2.7-dev mercurial libffi6 libffi-dev libpq-dev ca-certificates

You may find it easier to `pip install Mercurial`. That's actually preferred, as distros tend to not have the latest/greatest Mercurial version. And, you can explicitly control which version gets installed via pinning. It will require python-dev and possibly other support packages to compile, however.

::: testing/docker/builder-autoland/Dockerfile:32
(Diff revision 1)
> +ADD extra/vct/hgext/bootstrap.py ${VCT_HOME}/hgext/bootstrap.py
> +ADD extra/vct/hgext/reviewboard ${VCT_HOME}/hgext/reviewboard
> +ADD extra/vct/pylib/ ${VCT_HOME}/pylib/

This feels brittle. But it's a lesser evil than copying all of v-c-t over.

::: testing/docker/builder-autoland/Dockerfile:48
(Diff revision 1)
> -RUN mkdir /repos && hg init /repos/mozilla-central && hg init /repos/try
> +RUN cd /home/ubuntu && hg clone https://bitbucket.org/marmoute/mutable-history 

mutable-history is very aggressive about dropping compatibility with older versions. You'll almost certainly want to pin the Mercurial package version and the checked out mutable-history revision.

Comment 14

[Mark Côté [:mcote]]

Comment on attachment 8630452 [details]
MozReview Request: testing: add poll option to mach autoland status commands (bug 1174752) r?mcote

https://reviewboard.mozilla.org/r/12727/#review11525

::: testing/vcttesting/autoland_mach_commands.py:136
(Diff revision 1)
> +    def pullrequest_job_status(self, host, requestid, poll):
> +        url = host.rstrip('/') + '/pullrequest/mozreview/status/' + requestid
> +        if not poll:
> +            r = requests.get(url)
> +            print(r.status_code, r.text)
> +        else:
> +            import json
> +            import time
> +            attempts = 0
> +            while attempts < MAX_POLL_ATTEMPTS:
> +                attempts += 1
> +                r = requests.get(url)
> +                if r.status_code != 200 or json.loads(r.text)['landed'] is not None:
> -        print(r.status_code, r.text)
> +                    print(r.status_code, r.text)
> +                    break
> +                time.sleep(POLL_INTERVAL)
> +            else:
> +                print('timed out')

This code is duplicated exactly aside from the url. Screams to me that this should be put into its own function.

Comment 15

[Mark Côté [:mcote]]

Comment on attachment 8630453 [details]
MozReview Request: testing: add autoland startup test (bug 1174752) r?mcote

https://reviewboard.mozilla.org/r/12729/#review11527

::: autoland/tests/test-post-autoland-job.t:8
(Diff revision 1)
> +  $ mozreview exec autoland tail -n 20 /home/ubuntu/autoland.log
> +   0:00.00 LOG: MainThread INFO starting autoland

Is there any reason you are tailing the last 20 lines but verifying there's only one?  Just to make sure no other messages are logged at startup?

Comment 16

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/12729/#review11527

> Is there any reason you are tailing the last 20 lines but verifying there's only one?  Just to make sure no other messages are logged at startup?

I'm just trying to provide enough context to look at a caught exception in the log without having to output the entire log file, which can grow big quickly if there's a fundamental problem with the code.

Comment 17

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/12725/#review11539

::: hgext/reviewboard/tests/helpers.sh:69
(Diff revision 1)
> +  mozreview create-ldap-user [e-mail] password 2000 'Otto Land' \
> +    --key-file $TESTDIR/testing/docker/builder-autoland/autoland_id_rsa \
> +    --scm-level 3 > /dev/null

the `mozreview` bind user is created as part of the container, so that the ldap container always mimics production in the accounts it has etc.

Should this be moved and created there as well? That way other tests which don't use helpers.sh have it.

Comment 18

[Dan Minor [:dminor]]

Comment on attachment 8630450 [details]
MozReview Request: testing: link autoland container to hgrb (bug 1174752) r?gps

testing: link autoland container to hgrb (bug 1174752) r?gps

Comment 19

[Dan Minor [:dminor]]

Comment on attachment 8630451 [details]
MozReview Request: testing: create autoland ldap user for testing (bug 1174752) r?gps

testing: create autoland ldap user for testing (bug 1174752) r?gps

Comment 20

[Dan Minor [:dminor]]

Comment on attachment 8630452 [details]
MozReview Request: testing: add poll option to mach autoland status commands (bug 1174752) r?mcote

testing: add poll option to mach autoland status commands (bug 1174752) r?mcote

Autoland services jobs asynchronously so the status commands are more useful if
we poll until the jobs either succeed or fail.

Comment 21

[Dan Minor [:dminor]]

Comment on attachment 8630453 [details]
MozReview Request: testing: add autoland startup test (bug 1174752) r?mcote

testing: add autoland startup test (bug 1174752) r?mcote

Comment 22

[Dan Minor [:dminor]]

Comment on attachment 8630454 [details]
MozReview Request: autoland: provide better default for missing repo path (bug 1174752) r=mdoglio

autoland: provide better default for missing repo path (bug 1174752) r=mdoglio

This default helps with testing by allowing us to use repositories at paths
we do not know in advance.

Comment 23

[Dan Minor [:dminor]]

Comment on attachment 8630455 [details]
MozReview Request: testing: clone created repos in autoland container (bug 1174752) r?gps

testing: clone created repos in autoland container (bug 1174752) r?gps

Comment 24

[Dan Minor [:dminor]]

Comment on attachment 8630456 [details]
MozReview Request: testing: Update autoland tests to use clone of hgrb repositories (bug 1174752) r?mdoglio

testing: Update autoland tests to use clone of hgrb repositories (bug 1174752) r?mdoglio

Update the autoland tests to make use of the repository cloned from hgrb rather
than the "fake" repository that was used previously.

Comment 25

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/12725/#review11593

::: testing/docker/builder-ldap/mozilla.ldif:86
(Diff revision 2)
> +homeDirectory: /home/password
> +sn: Land
> +uid: password

the uid password thing confuses me? why isn't this something like `uid: autoland`?

Comment 26

[Gregory Szorc [:gps]]

Comment on attachment 8630450 [details]
MozReview Request: testing: link autoland container to hgrb (bug 1174752) r?gps

https://reviewboard.mozilla.org/r/12723/#review11595

Ship It!

Comment 27

[Dan Minor [:dminor]]

https://reviewboard.mozilla.org/r/12725/#review11593

> the uid password thing confuses me? why isn't this something like `uid: autoland`?

I used the slapcat utility to dump what was being generated by the create-ldap-user command I was using before. I didn't have a close look at the output until now.

Comment 28

[Gregory Szorc [:gps]]

Comment on attachment 8630451 [details]
MozReview Request: testing: create autoland ldap user for testing (bug 1174752) r?gps

https://reviewboard.mozilla.org/r/12725/#review11619

The "password" value for uid is the only thing that bothers me. Else, this is fine.

Comment 29

[Gregory Szorc [:gps]]

Comment on attachment 8630455 [details]
MozReview Request: testing: clone created repos in autoland container (bug 1174752) r?gps

https://reviewboard.mozilla.org/r/12733/#review11623

::: testing/docker/builder-autoland/Dockerfile:15
(Diff revision 2)
> +RUN apt-get -y install software-properties-common
> +RUN add-apt-repository ppa:mercurial-ppa/releases

I don't believe this isn't needed now that you pip install Mercurial.

Comment 30

[Mark Côté [:mcote]]

Comment on attachment 8630452 [details]
MozReview Request: testing: add poll option to mach autoland status commands (bug 1174752) r?mcote

https://reviewboard.mozilla.org/r/12727/#review11659

Ship It!

Comment 31

[Mark Côté [:mcote]]

Comment on attachment 8630453 [details]
MozReview Request: testing: add autoland startup test (bug 1174752) r?mcote

https://reviewboard.mozilla.org/r/12729/#review11661

Ship It!

Comment 32

[Mauro Doglio [:mdoglio]]

Comment on attachment 8630454 [details]
MozReview Request: autoland: provide better default for missing repo path (bug 1174752) r=mdoglio

https://reviewboard.mozilla.org/r/12731/#review11699

Ship It!

Comment 33

[Mauro Doglio [:mdoglio]]

Comment on attachment 8630456 [details]
MozReview Request: testing: Update autoland tests to use clone of hgrb repositories (bug 1174752) r?mdoglio

https://reviewboard.mozilla.org/r/12735/#review11701

Ship It!