568980 - Better error messages for add-ons testrun script for missing builds or wrong add-on id's

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Automation, defect)

Description

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

Right now the error output is not really helpful or doesn't exist at all.

* If you don't specify a build we should immediately fail

* If you specify an unknown add-on id we should throw an error and proceed with the next extension.

Comment 1

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

Attachment: Patch v1

With that patch we give an useful message if no binary has been specified for all test-run scripts.

For the add-ons test-run I have updated the optparse help for the --addons option. It caused a lot of confusion today. I'm sure calling out the limitation to specific add-ons from the test repository should make things more clear.

Comment 2

[Jeff Hammel]

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

># HG changeset patch
># Parent 5c6b6f32b18e228190d5ff2cfafad008bcbf54fe
>Bug 568980 - Better error messages for add-ons testrun script for missing builds or wrong add-on id's, r=jhammel
>
>diff --git a/libs/testrun.py b/libs/testrun.py
>--- a/libs/testrun.py
>+++ b/libs/testrun.py
>@@ -153,16 +153,21 @@ class TestRun(object):
>         """ Start the execution of the tests. """
> 
>         self.prepare_tests()
>         self._mozmill.run()
> 
>     def run(self, *args, **kwargs):
>         """ Run software update tests for all specified builds. """
> 
>+        # If no binaries have been specified we cancel the test-run
>+        if not self.binaries:
>+            print "*** No builds have been specified. Use --help to see all options."
>+            return
>+

I'd check this in the constructor instead, but this is fine.

>         self.clone_repository()
> 
>         try:
>             # Run tests for each binary
>             for binary in self.binaries:
>                 try:
>                     self.prepare_binary(binary)
>                     self.prepare_repository()
>diff --git a/testrun_addons.py b/testrun_addons.py
>--- a/testrun_addons.py
>+++ b/testrun_addons.py
>@@ -43,29 +43,30 @@ import sys
> base_path = os.path.dirname(os.path.abspath(__file__))
> sys.path.append(os.path.join(base_path, 'libs'))
> 
> from testrun import AddonsTestRun
> 
> def main():
>     usage = "usage: %prog [options] (binary|folder)"
>     parser = optparse.OptionParser(usage=usage, version="%prog 0.1")
>-    parser.add_option("-a", "--addon",
>+    parser.add_option("-a", "--addons",

You can have both if you'd rather: "-a", "--addon", "--addons"
But either way is fine.

>                       action="append",
>                       dest="addons",
>                       metavar="ID",
>-                      help = "List of add-ons to use")
>+                      help="Only test those listed add-ons from the " +
>+                           "mozmill-tests repository, e.g. [e-mail]")
>     parser.add_option("-l", "--logfile",
>-                      dest = "logfile",
>-                      metavar = "PATH",
>-                      help = "Path to the log file")
>+                      dest="logfile",
>+                      metavar="PATH",
>+                      help="Path to the log file")
>     parser.add_option("-r", "--report",
>-                      dest = "report",
>-                      metavar = "URL",
>-                      help = "Send results to the report server")
>+                      dest="report",
>+                      metavar="URL",
>+                      help="Send results to the report server")
>     parser.add_option("--with-untrusted",
>                       action="store_true",
>                       dest="with_untrusted",
>                       default=False,
>                       help="Also run tests for add-ons which are not stored on AMO")
>     (options, args) = parser.parse_args()
> 
>     addons = AddonsTestRun(binaries=args, addons=options.addons,

Looks fine!

Comment 3

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

(In reply to comment #2)
> >+        # If no binaries have been specified we cancel the test-run
> >+        if not self.binaries:
> >+            print "*** No builds have been specified. Use --help to see all options."
> >+            return

There is still the attribute and I don't know if consumers of this class will modify the list at a later stage before run gets called. So I can't do that in the constructor.

> >-    parser.add_option("-a", "--addon",
> >+    parser.add_option("-a", "--addons",
> 
> You can have both if you'd rather: "-a", "--addon", "--addons"
> But either way is fine.

--addons gives more the feeling that there is a list behind and you can use this option multiple times. Personally I would be happier with a comma separated list but lets see how it goes.

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

Comment 4

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

Move of Mozmill related project bugs to newly created components. You can
filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.