Merge lp:~mabac/svammel/bug-749560-dont-hardcode-arch-tags into lp:svammel

[Mattias Backman]

Hi,

This branch add the possibility to specify bug tags and architectures on the command line.

If none of these are specified it will default to the values in the original requirements. That is checking for failures on 'armel' that have succeded on 'i386' or 'amd64' and to set the tags 'ftbfs' and 'arm-porting-queue'.

It is possible to just specify the option for target-arch and leave reference-arch to the defaults, or the other way around.

Thanks,

Mattias

[lp:~mabac/svammel/bug-749560-dont-hardcode-arch-tags]

86. By Mattias Backman on 2011-04-06

Fix long line.

[James Westby (james-w)]

Hi,

This looks good to me, thanks.

Two minor points. It's not possible to have no tags on the bugs with this
scheme, but I don't think that's a great loss.

Secondly, could you use a default argument to the Option creation, rather
than using the conditionals later?

Thanks,

James

review: Approve

[Mattias Backman (mabac)]

> Two minor points. It's not possible to have no tags on the bugs with this
> scheme, but I don't think that's a great loss.

Yeah, I thought that having the original tags as default would be less intrusive to Steve at the moment. To make the script really generic I think that there should be no default tags or architectures. Specifying no tags should mean to not set tags and the arch options should be required. As implemented now, it's still an arm bug filer with the option to override that.

> Secondly, could you use a default argument to the Option creation, rather
> than using the conditionals later?

I tried to do that. Having a default for an 'append' type option made the specified tag be appended to the default list. I would have wanted the default list only when no --bugtag was specified.

It would be possible for the --target-arch option though since that's not appending to a list, but then I'd have different handling of defaults for the two arch options.

Thanks,

Mattias

[James Westby (james-w)]

On Wed, 06 Apr 2011 12:46:37 -0000, Mattias Backman <email address hidden> wrote:
> Yeah, I thought that having the original tags as default would be less
> intrusive to Steve at the moment. To make the script really generic I
> think that there should be no default tags or
> architectures. Specifying no tags should mean to not set tags and the
> arch options should be required. As implemented now, it's still an arm
> bug filer with the option to override that.

Yeah, I think that's ok reasoning.

>
> > Secondly, could you use a default argument to the Option creation, rather
> > than using the conditionals later?
>
> I tried to do that. Having a default for an 'append' type option made
> the specified tag be appended to the default list. I would have wanted
> the default list only when no --bugtag was specified.

That's unexpected. I'm happy with what you've done then.

Thanks,

James

=== modified file 'bug_reporting.py'
--- bug_reporting.py	2011-03-22 10:17:01 +0000
+++ bug_reporting.py	2011-04-06 10:06:35 +0000
@@ -46,16 +46,9 @@
     return bug
 
 
-def bug_already_filed(package_name, lp):
-    """ Check to see if this project already has an active bug."""
-    project_url = get_project_url(package_name, lp)
-    return bug_already_filed_by_url(project_url, lp)
-
-
-def bug_already_filed_by_url(project_url, lp):
-    """ Check to see if this project already has an active bug."""
-
-    dupe_tags = ['ftbfs', 'arm-porting-queue']
+def bug_already_filed_by_url(project_url, dupe_tags, lp):
+    """ Check to see if this project already has an active bug."""
+
     project = lp.load(project_url)
 
     # Should tags_combinator be 'Any'? Assumption now is to determine

=== modified file 'config.py'
--- config.py	2011-03-22 10:17:01 +0000
+++ config.py	2011-04-06 10:06:35 +0000
@@ -120,6 +120,21 @@
     parser.add_argument(
         '--logfile', required=False,
         help="Path to file where reported bugs are logged.")
+    parser.add_argument(
+        '--bugtag', required=False, action='append',
+        help="Specify the tag or multiple tags to be set on all created " \
+            "bugs. If this is not specified tags default to 'ftbfs', " \
+            "'arm-porting-queue'.")
+    parser.add_argument(
+        '--target-arch', required=False, dest='targetarch',
+        help="The target architecture to find build failures for. " \
+            "Defaults to 'armel'.")
+    parser.add_argument(
+        '--reference-arch', required=False, action='append',
+        dest='referencearch',
+        help="One or more reference architectures. If successful builds " \
+            "for any of these are found for a package, a bug will not be " \
+            "filed against that package. Defaults to 'i386', 'amd64'.")
 
     return parser
 

=== modified file 'data_parsing.py'
--- data_parsing.py	2011-03-18 10:14:36 +0000
+++ data_parsing.py	2011-04-06 10:06:35 +0000
@@ -43,13 +43,10 @@
 def get_tags_for_fail(fail_data=''):
     """ Determine cause of failure and tag accordingly. """
 
-    # These are default tags to be set on all bugs.
-    tags = ['ftbfs', 'arm-porting-queue']
-
+    tags = []
     if fail_data != '':
         # Process build log contents to determine extra tags
         pass
-
     return tags
 
 

=== modified file 'file-failures.py'
--- file-failures.py	2011-03-23 08:25:30 +0000
+++ file-failures.py	2011-04-06 10:06:35 +0000
@@ -47,6 +47,9 @@
 parser = get_args_parser()
 args = parser.parse_args()
 set_verbose_messages(args.verbose)
+standard_bug_tags = ['ftbfs', 'arm-porting-queue']
+if args.bugtag is not None:
+    standard_bug_tags = args.bugtag
 
 init(args.serviceroot, 'testing', '~/.launchpadlib/cache/')
 
@@ -54,6 +57,10 @@
 fail_platform = 'armel'
 # unless there's a failed build for any of these:
 ok_platforms = ['i386', 'amd64']
+if args.targetarch is not None:
+    fail_platform = args.targetarch
+if args.referencearch is not None:
+    ok_platforms = args.referencearch
 
 try:
     interesting_spph = get_build_status(args.archive, args.series,
@@ -66,7 +73,7 @@
 bug_log = Log(args.logfile)
 for spph in interesting_spph:
 
-    if bug_already_filed_by_url(spph.url, get_launchpad()):
+    if bug_already_filed_by_url(spph.url, standard_bug_tags, get_launchpad()):
         print_message("There's already a bug report for %s version %s." % \
             (spph.package_name, spph.version))
         continue
@@ -85,7 +92,8 @@
         print "%s build log not available for %s version %s." % \
             (fail_platform, spph.package_name, spph.version)
         build_log = ''
-    bug_tags = get_tags_for_fail(build_log)
+    bug_tags = standard_bug_tags
+    bug_tags.extend(get_tags_for_fail(build_log))
     interesting_log_part = get_interesting_part(build_log)
 
     bug_title = '%s version %s failed to build on %s' % \

=== modified file 'tests/test_fail_filer.py'
--- tests/test_fail_filer.py	2011-03-01 11:50:47 +0000
+++ tests/test_fail_filer.py	2011-04-06 10:06:35 +0000
@@ -43,23 +43,10 @@
     def setUp(self):
         super(TestBugTags, self).setUp()
 
-    def test_default_tag_ftbfs(self):
-        """ All bugs should get this tag. """
-        tags = get_tags_for_fail("Dummy tag data")
-        self.assertIn('ftbfs', tags)
-
-    def test_default_tag_arm_porting_queue(self):
-        """ All bugs should get this tag. """
-        tags = get_tags_for_fail("Dummy tag data")
-        self.assertIn('arm-porting-queue', tags)
-
     def test_no_match_no_extra_tag(self):
         """ This will not match, so should only get default tags. """
         tags = get_tags_for_fail("This will never trigger matcher.")
-        self.assertIn('ftbfs', tags)
-        self.assertIn('arm-porting-queue', tags)
-        self.assertFalse(len(tags) > 2,
-                        msg='Got more than default tags without match.')
+        self.assertEquals([], tags)
 
 
 #class TestLaunchpadProject(TestCaseWithFixtures):
@@ -91,7 +78,7 @@
                         return ['something here']
                 return MockProject()
 
-        self.assertTrue(bug_already_filed_by_url('https://dummy',
+        self.assertTrue(bug_already_filed_by_url('https://dummy', [],
                                             MockLaunchpad()))
 
     def test_duplicate_does_not_exist(self):
@@ -103,7 +90,7 @@
                         return []
                 return MockProject()
 
-        self.assertFalse(bug_already_filed_by_url('https://dummy',
+        self.assertFalse(bug_already_filed_by_url('https://dummy', [],
                                             MockLaunchpad()))
 
 #    def test_file_bug(self):