628659 - Implementation of an automation wrapper script to simplify release testing

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Automation, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

With bug 528064 we have the download script, which needs to be automated now. Therefore we plan to have a script which supports INI files. It has to read those files accordingly to the test plan (update testing, and BFT testing) for the given release.

We have the following requirements:

1. The list of builds to download has to be listed in the ini file. As best it should cover all platforms and stores the downloaded builds in named sub-folders.

2. The download has to happen independently from the test-run itself. That would give us the chance to prepare everything in-front. But there should also be an demand testing, where builds gets downloaded right before the test-run.

Geo and Al, do we need anything else? A proposal for the ini file I will attach later today.

Comment 1

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

I envisioned the ini as something like:

[Platforms]
mac64
win32
linux

[Locales]
en-US
cs
es

[Builds]
3.6.16
3.6.15
3.6

...and from there the script would download the expansion. I imagine this is pretty close to what you have in mind. 

If we decide to do all locales as a point of process, that could either be handled with an .ini template that predefines them all, or with a comprehensive scrape. Former's probably significantly more flexible, and is likely what I'd go for, but admittedly requires maintenance to add locales as they're created.

Comment 2

[Al Billings [:abillings - ex-MoCo]]

I would add something to the ini about whether to do fallback or not, perhaps.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, everyone. I came up with a solution for our config file. In the following you will find the format for both our general and update tests. It can be executed with the platform as argument (updates also will accept the channel) and it will be executed. For the staging part platform=all will also work.

bft
~~~
[global]
application="firefox"
build=1

[mac]
platform=mac
3.6.14="en-US, de"

[win2000]
platform=win32
3.6.14="en-US, fr"

[winxp]
platform=win32
3.6.14="en-US, pl"

[testrun]
script=testrun_general.py
report=http://mozmill-xxx.brasstacks.mozilla.com/db/

update
~~~~~~
[global]
application="firefox"

[mac]
platform=mac
3.6.13="en-US, de"
3.6.12="en-US, ja-JP-mac"
3.6="en-US"

[win2000]
platform=win32
3.6.13="en-US, fr"
3.6.12="en-US, es-ES"

[winxp]
platform=win32
3.6.13="en-US, pl"
3.6.12="en-US"

[targets]
version="3.6.14"
mac="20110207130601"
win32="20110207130507"

[testrun]
script=testrun_update.py
fallback="true"
logfile="logs/%(platform)s-%(locale)s.log
report=http://mozmill-xxx.brasstacks.mozilla.com/db/

Please give me feedback what you think. I would like to start with the implementation tomorrow.

Comment 4

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Looks fine to me. I liked your idea of having separate .ini files for different levels of test run (i.e. tier1-only, all-locales, etc.)

Comment 5

[u279076]

What are your considerations for 64-bit builds and 32-bit builds on 64-bit OS?

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

That will be:

[win7_64]
platform=win64

Comment 7

[Al Billings [:abillings - ex-MoCo]]

This looks good to me.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: WIP v1 (staging part)

With this patch we are able to stage builds for update testing as specified by the cfg file.

Comment 9

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Not filing a separate bug, since this is still WIP:

For some update test plans (mostly ones where we already released something into beta, etc., then need to respin it) we need to be able to say something like:

3.6.17 build 1=en-US de etc

That is to say, download build 1 of 3.6.17 because we're going to make sure it updates to the current build (probably build 2)

Currently, the conf file lets you set a build number, but that's global, and meant only for BFTs. We do need the ability to specify the build number on each version as well. If you do that, we may not need the global option anymore.

Comment 10

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Note that we found a problem with the regex used in the original WIP. 

pattern = re.compile(r'\d+\.\d+(\.\d+)?')

...caused the beta designation to get clipped off the version.

We loosened it to \d+\.\d+ and things worked.

Given this and a couple of other improvements to the script to let it create directories, etc., I'm reuploading the current version to this bug.

Comment 11

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Attachment: Current version of staging script

Not requesting feedback, just don't want the current version to be lost.

Comment 12

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Ignore my comment re: the loosened regex in c10. The one that is in the current version of the script is what was used, and appears to work well.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: WIP v2 (staging part)

This WIP includes the changes made by Geo and also updates the structure of the config files to have only a single main section, now called testrun. A complete patch I target for tomorrow. It will contain:

* Handling of candidate builds
* Retrieval of the build id

Once that's done I have to think about how to handle daily builds.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1

First real patch now with candidate support built-in. That's all what we need for now.

The execution part will be a follow-up patch and handled later.

Comment 15

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 542990 [details] [diff] [review]
Patch v1

r+ on the script. Looks fine for download, which is my main focus.

Future plans: 

I'm not sold on the script_option stuff, especially this implementation. Technically-speaking, afaik it's a format error in an ini/cfg file to have more than one key with the same name in the same group (and I'm not at all sure that ConfigParser deals well with that situation). Plus, the way you're using the options could be accomplished more clearly with separate specific keys.

But there's a bigger problem, and it's a human one that requires stepping into someone else's head: I think it'll be hard for people to understand which options they could be using and how they work.

This is one of the problems I see with the "do everything in one script" idea, is that these configs are going to get so complex and implementation-specific they can't be defined by the leads anymore. These execution options won't make sense to them, and if we change the scripts we'll have to re-educate them.

Separating the staging process out into a separate thing lets them concentrate on what they know and care about so they can deliver the config we need themselves. Thus that config that they write cannot have execution options in it.

Here's the thing:

I'm confident that the right answer from a *process* point of view just isn't a do-everything script, and I'm going to continue to argue against that solution. It's technically tempting, but will fail in the real world because it's too complex to define the behavior for. 

The right answer is a do-everything web app front end that can guide people to the right options. And the web app just won't care whether it's calling one or two scripts to complete this process. So we should tilt the scripts for people, since they're the ones that will care.

The people are using command line, right now, today. So as long as we're in command line mode, I think we have to have the simple thing the leads can define and the more complicated thing the automation team knows how to run, and that's just the way it is.

So again, I'll urge you to at least talk this out with me. I think I'm seeing this solution from a much different perspective than you are, and I'm pretty sure of myself on it.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Those options have been added temporarily. And as we have spoken and made clear in a comment before this part of the code is not ready yet. Probably we should remove it for the first check-in, so that we have clean code without any distracting code. If you agree on it, I will do this change and land it. 

Anything else will follow later.

Comment 17

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Works for me.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v2 (staging) [checked-in]

Updated patch with not used config keys removed.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 543305 [details] [diff] [review]
Patch v2 (staging) [checked-in]

Landed as:
http://hg.mozilla.org/qa/mozmill-automation/rev/6402bfecb110

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

What we have so far is what was necessary to get us started. I don't think anything more is necessary here while we are working on a CI solution with Jenkins. I will close this bug and we will check later what to do with the code, once we don't need it anymore.