Add ability to download unit test prerequisites and to run from a specific cached build directory

RESOLVED FIXED

Status

Testing
Autophone
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bc, Assigned: bc)

Tracking

Trunk
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 654627 [details] [diff] [review]
patch

In order to run mochitests and robocop tests from Autophone we will need to download the test zip file, the robocop apk and the fennec_ids.txt files for the build we are testing. Since this is a huge download hit, this should be optional.

In addition, it is helpful to specify a specific build cache directory when testing so that we can use a set of files not necessarily a part of the formal build cache directory.

I changed the jobs to pass around the path to the build cache directory rather than the path to the build.apk as it was simpler when dealing with the additional files.
Attachment #654627 - Flags: review?(mcote)
(Assignee)

Updated

6 years ago
Assignee: nobody → bclary
Status: NEW → ASSIGNED
(Assignee)

Updated

6 years ago
Blocks: 785129
(Assignee)

Updated

6 years ago
Blocks: 785130
(Assignee)

Comment 1

6 years ago
Created attachment 654827 [details] [diff] [review]
patch v2

I should have also changed the actual job data structure to pass the build cache directory rather than keeping the apkpath. This allows me to get the tests, fennec_ids.txt and robocop.apk files from the job. I'll go ahead and tweak the patch here instead of in a follow up. I haven't tested this yet though.

This assumes build.apk is always the name of the file for the downloaded build which I don't think is a problem.
Attachment #654627 - Attachment is obsolete: true
Attachment #654627 - Flags: review?(mcote)
Attachment #654827 - Flags: review?(mcote)
is this ready to land?  Can we get a review on the above patch as I need to use this for a burnin script on the panda boards.

Comment 3

6 years ago
Comment on attachment 654827 [details] [diff] [review]
patch v2

Review of attachment 654827 [details] [diff] [review]:
-----------------------------------------------------------------

So this is a good start, but I think it warrants two changes, if I understand this correctly:

- rename cached_build_dir to something that indicates that it overrides all builds, that is, that it will be used regardless of what build is requested in the trigger command. I find that, when skimming the code, it's a bit confusing to have both build_cache and cached_build_dir variables that do pretty different things (the former being a true cache, the latter being an override).
- move all the build-cache/overriding stuff to builds.py. I tried to put as much of the build-fetching logic as possible into the BuildCache object, so I think this new stuff should go there too, to avoid having build logic in multiple places.

That make sense?

::: trigger_runs.py
@@ +11,5 @@
> +try:
> +    import json
> +except ImportError:
> +    import simplejson
> +

This doesn't appear to be used anywhere.

@@ +48,4 @@
>              start_time = start_time.replace(tzinfo=pytz.timezone('US/Pacific'))
>          if not end_time.tzinfo:
>              end_time = end_time.replace(tzinfo=pytz.timezone('US/Pacific'))
> +        cache_build_dir_list = builds.BuildCache().find_builds(start_time, end_time, 

May as well get rid of the trailing space. :)
Attachment #654827 - Flags: review?(mcote) → review-
(Assignee)

Comment 4

6 years ago
Created attachment 658980 [details] [diff] [review]
patch v3
Attachment #658980 - Flags: review?(mcote)
(Assignee)

Updated

6 years ago
Attachment #658980 - Attachment description: patch v2 → patch v3
Attachment #658980 - Attachment filename: bug-785086-v2.patch → bug-785086-v3.patch

Comment 5

6 years ago
Comment on attachment 658980 [details] [diff] [review]
patch v3

Looks good. My only comment is that a bad build dir results in a traceback being displayed. Since this isn't an unexpected error, the AutoPhone object should probably catch it and just display a helpful error. Might need a simple Exception class for this.
Attachment #658980 - Flags: review?(mcote) → review+

Updated

6 years ago
Attachment #654827 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Created attachment 660981 [details] [diff] [review]
patch v4

with builds.BuildCacheException and a helpful message.

Mark, can you land this for me?
(Assignee)

Comment 7

6 years ago
Comment on attachment 660981 [details] [diff] [review]
patch v4

(In reply to Bob Clary [:bc:] from comment #6)
> Created attachment 660981 [details] [diff] [review]
> patch v4
> 
> with builds.BuildCacheException and a helpful message.
> 
> Mark, can you land this for me?

Wait a second. I think I have git merge issue here.
Attachment #660981 - Attachment is obsolete: true

Comment 8

6 years ago
I just added you to the mozilla automation team on github, which has write access to autophone, so feel free to land it yourself whenever you are ready.
(Assignee)

Comment 9

6 years ago
Comment on attachment 660981 [details] [diff] [review]
patch v4

doh. This is ok actually. I got confused with interdiff patches that were against different masters.

Mark, can you land this one for real now?
Attachment #660981 - Attachment is obsolete: false
(Assignee)

Comment 10

6 years ago
commit d1e02c520b4685c4432f6f660a64a2122a66032c
Author: Bob Clary <bob@bclary.com>
Date:   Thu Sep 13 14:16:03 2012 -0700
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.