Merge lp:~mabac/svammel/get-success-logs into lp:svammel

[Mattias Backman]

Hi,

This branch adds cross referencing successful builds before filing bugs. The action is added to filter_spph which did get quite messy. :/

Well, what it does for each package not already rejected is to iterate over all archives and ask for successful builds. We then get the successful build of the most recent package version and compare that to the version of the failed build.

If there are any successful builds in any archive the only way the failed build survives is if it is of a higher version than any of the successful builds. My reasoning is that if the fail is of the most recent version, it should be reported. And if there is any successful build for a more recent package there is nothing to report.

The interesting case is when we find a successful build of the same version of the package that we have already found a failure for. I think that this would mean that there isn't a porting issue then since the version can be built on armel so no bug will be filed.

Thanks,

Mattias

[lp:~mabac/svammel/get-success-logs]

74. By Mattias Backman on 2011-03-18

Merge changes from add-version-and-tests.

75. By Mattias Backman on 2011-03-18

Docstring indentation and whitespace fix.

76. By Mattias Backman on 2011-03-21

Add basic filtering tests.

77. By Mattias Backman on 2011-03-21

Break long lines and add get_build_list tests.

78. By Mattias Backman on 2011-03-21

Add more filtering tests.

79. By Mattias Backman on 2011-03-21

remove double import.

[James Westby (james-w)]

9 + help='Disable bug filing. Combine with -v to enable useful output.')

Should -v just be implied?

61 + if spph_queue is None:
62 + raise NoVersionError("None supplied as list.")

I tend to not code defensively in cases like this. The developer
will get an error that is almost as obvious when it tries to iterate the
list, so I lean towards brevity. I reserve the checks for when the error
would be obtuse, or the code will do the wrong thing.

That's just my preference though, if you prefer this then leave it as is.

126 + if spph is None:
127 + continue

...

150 + spph = None

Could that just be a "break" where you set spph to None?

This looks good otherwise.

Thanks,

James

review: Approve

[lp:~mabac/svammel/get-success-logs]

80. By Mattias Backman on 2011-03-23

Make --dryrun enable -v output.

81. By Mattias Backman on 2011-03-23

Do not raise error for null queue.

82. By Mattias Backman on 2011-03-23

Improve test.

[Mattias Backman (mabac)]

> 9 + help='Disable bug filing. Combine with -v to enable useful
> output.')
>
> Should -v just be implied?

Yeah, that's a good idea. Fixed.

>
> 61 + if spph_queue is None:
> 62 + raise NoVersionError("None supplied as list.")
>
> I tend to not code defensively in cases like this. The developer
> will get an error that is almost as obvious when it tries to iterate the
> list, so I lean towards brevity. I reserve the checks for when the error
> would be obtuse, or the code will do the wrong thing.
>
> That's just my preference though, if you prefer this then leave it as is.

Mostly this is about fear of Coverity Prevent which flags any remotely theoretical possibility of an unchecked null pointer as a defect. I can probably take it more easy with this in svammel and get cleaner code. :)

>
> 126 + if spph is None:
> 127 + continue
>
> ...
>
> 150 + spph = None
>
> Could that just be a "break" where you set spph to None?

Yes, that's true.

[lp:~mabac/svammel/get-success-logs]

83. By Mattias Backman on 2011-03-23

Remove unused Error.

84. By Mattias Backman on 2011-03-24

Merged changes from add-version-check-and-tests.

85. By Mattias Backman on 2011-03-29

Merge changes from trunk.

86. By Mattias Backman on 2011-03-30

Merge James' fix for not shadowing logging.

87. By Mattias Backman on 2011-03-30

Fix build_build_record defect.

=== renamed file 'logging.py' => 'bug_logging.py'
=== modified file 'config.py'
--- config.py	2011-03-22 10:17:01 +0000
+++ config.py	2011-03-30 07:39:33 +0000
@@ -104,6 +104,9 @@
         '-v', '--verbose', action='store_true', dest='verbose',
         help='Enable verbose messages.')
     parser.add_argument(
+        '--dryrun', action='store_true', dest='dryrun',
+        help='Disable bug filing and enable verbose messages.')
+    parser.add_argument(
         '--serviceroot', default='production',
         choices=['production', 'staging', 'qastaging'],
         help='The Launchpad environment to connect to.')

=== modified file 'data_parsing.py'
--- data_parsing.py	2011-03-30 07:39:33 +0000
+++ data_parsing.py	2011-03-30 07:39:33 +0000
@@ -85,14 +85,48 @@
                                        self.version) < 0)
 
 
+def get_build_list(archive, state, source=None):
+
+    buildlist = archive.getBuildRecords(build_state=state, source_name=source)
+
+    # XXX mabac 2011-03-16: getBuildRecords filters on substrings
+    # so buildlist probably contains non exact matches.
+    # https://bugs.launchpad.net/launchpad/+bug/231862
+    if source is not None:
+        filter_list = []
+        for build in buildlist:
+            csp = build.current_source_publication
+            if csp is not None and csp.source_package_name == source:
+                filter_list.append(build)
+        return filter_list
+
+    return buildlist
+
+
+def get_highest_version(spph_queue):
+    """Returns the spph of the highest version in the list.
+
+    If more than one spph with the same version is in the list,
+    the first of the identical versions in the list is returned.
+    """
+    candidate_spph = None
+    while len(spph_queue) > 0:
+        new_spph = spph_queue.pop(0)
+        if new_spph is None:
+            continue
+        if new_spph.is_higher_version_than(candidate_spph):
+            candidate_spph = new_spph
+
+    return candidate_spph
+
+
 def fetch_pkg_list(archives):
     all_spph = dict()
     for archive, archive_name in archives:
         print_message("The following packages have failed to build " \
                       "in '%s'." % archive_name)
 
-        buildlist = archive.getBuildRecords(build_state=FAILED_BUILD_STATE)
-
+        buildlist = get_build_list(archive, FAILED_BUILD_STATE)
         for build in buildlist:
             csp = build.current_source_publication
             if csp is None:
@@ -112,13 +146,14 @@
     return all_spph
 
 
-def filter_spph(all_spph_list, fail_arch, ok_archs):
+def filter_spph(all_spph_list, fail_arch, ok_archs, archives):
     """Filter out the build failures that are unique to the target arch.
 
     Arguments:
     all_spph_list -- List of all spphs to investigate.
     fail_arch -- The arch on which the builds should have failed.
     ok_archs -- A list of archs on which the builds have not failed.
+    archives -- A list of archives to cross check for successful builds.
 
     Returns:
     A subset of all_spph_list which represents the interesting packages.
@@ -126,13 +161,45 @@
     filtered_list = []
     for spph in all_spph_list.values():
         if not spph.is_ftbfs(fail_arch):
-            # Drop this spph since it did not fail on target arch.
-            continue
-        if next((arch for arch in ok_archs if spph.is_ftbfs(arch)),
-                None) is not None:
-            # Drop this spph since it did fail on other arch.
-            continue
-        filtered_list.append(spph)
+            print_message("Ignoring package '%s' version '%s' since it did " \
+                              "not fail on architecture '%s'." % \
+                              (spph.package_name, spph.version, fail_arch))
+            continue
+        also_failed = next((arch for arch in ok_archs
+                                 if spph.is_ftbfs(arch)), None)
+        if also_failed is not None:
+            print_message("Ignoring package '%s' version '%s' since it also " \
+                              "failed to build on architecture '%s'." % \
+                              (spph.package_name, spph.version, also_failed))
+            continue
+
+        for archive, archive_name in archives:
+            # Get list of successful builds for the same package
+            success_list = get_build_list(archive, SUCCESS_BUILD_STATE,
+                                          spph.package_name)
+            success_spph_list = []
+            for success_build in success_list:
+                success_spph = SPPH(success_build.current_source_publication)
+                if success_build.arch_tag == fail_arch:
+                    success_spph_list.append(success_spph)
+
+            candidate_success_spph = get_highest_version(success_spph_list)
+            if spph.is_higher_version_than(candidate_success_spph):
+                # Only care about the failed build if it is of a higher
+                # version than any successful build of the same package.
+                pass
+            else:
+                print_message("Failed build for package '%s' version '%s' " \
+                                  "overridden by successful build of " \
+                                  "version '%s' in archive '%s'." % \
+                                  (spph.package_name, spph.version,
+                                   candidate_success_spph.version,
+                                   archive_name))
+                spph = None
+                break
+
+        if spph is not None:
+            filtered_list.append(spph)
 
     return filtered_list
 
@@ -179,4 +246,4 @@
     print_message("Will be getting builds from archives " + ", ".join(
             name for (archive, name) in archives))
     spph_list = fetch_pkg_list(archives)
-    return filter_spph(spph_list, fail_platform, ok_platforms)
+    return filter_spph(spph_list, fail_platform, ok_platforms, archives)

=== modified file 'file-failures.py'
--- file-failures.py	2011-03-30 07:39:33 +0000
+++ file-failures.py	2011-03-30 07:39:33 +0000
@@ -13,7 +13,6 @@
 from data_parsing import (
     get_tags_for_fail,
     get_build_status,
-    filter_spph,
     )
 from bug_reporting import (
     file_bug_by_url,
@@ -30,7 +29,7 @@
     get_file_contents,
     get_interesting_part,
 )
-from logging import (
+from bug_logging import (
     Log,
     LogEntry,
 )
@@ -46,7 +45,7 @@
 
 parser = get_args_parser()
 args = parser.parse_args()
-set_verbose_messages(args.verbose)
+set_verbose_messages(args.verbose or args.dryrun)
 
 init(args.serviceroot, 'testing', '~/.launchpadlib/cache/')
 
@@ -80,7 +79,7 @@
 
     build_record = spph.get_arch(fail_platform)
     if build_record is not None:
-        build_build_record = get_file_contents(log.build_log_url)
+        build_log = get_file_contents(build_record.build_log_url)
     else:
         print "%s build record not available for %s version %s." % \
             (fail_platform, spph.package_name, spph.version)
@@ -99,12 +98,14 @@
 """ % (bug_title, build_record.web_link, build_record.build_log_url,
        interesting_log_part)
 
-    print_message(
-        "Will file bug for %s\n desc: %s\n title:  %s\n tags: %s" %
-        (spph.package_name, bug_description[0:220], bug_title, bug_tags))
-    bug = file_bug_by_url(spph.url, bug_description, bug_title,
-                          bug_tags, get_launchpad())
-    if bug is not None and args.logfile is not None:
-        bug_log.write_entry(
-            LogEntry(spph.package_name, spph.version, fail_platform,
-                     bug.self_link).to_string())
+    print_message("Will file bug for %s version %s with tags: %s" % \
+                  (spph.package_name, spph.version, bug_tags))
+    if not args.dryrun:
+        bug = file_bug_by_url(spph.url, bug_description, bug_title,
+                              bug_tags, get_launchpad())
+        if bug is not None and args.logfile is not None:
+            bug_log.write_entry(
+                LogEntry(spph.package_name, spph.version, fail_platform,
+                         bug.self_link).to_string())
+    else:
+        print_message("  ... but didn't since dryrun is specified.")

=== modified file 'tests/test_bug_logging.py'
--- tests/test_bug_logging.py	2011-03-23 11:12:12 +0000
+++ tests/test_bug_logging.py	2011-03-30 07:39:33 +0000
@@ -15,7 +15,7 @@
     TestCaseWithFixtures,
     MockSomethingFixture,
     )
-from logging import (
+from bug_logging import (
     LOG_ITEM_SEPARATOR,
     Log,
     LogEntry,

=== modified file 'tests/test_fail_filer.py'
--- tests/test_fail_filer.py	2011-03-30 07:39:33 +0000
+++ tests/test_fail_filer.py	2011-03-30 07:39:33 +0000
@@ -9,8 +9,6 @@
 #     Linaro Infrastructure Team - initial implementation
 ###############################################################################
 from testtools import TestCase
-
-from testtools import TestCase
 from datetime import (
     datetime,
     )
@@ -25,13 +23,13 @@
     SUCCESS_BUILD_STATE,
     SPPH,
     get_tags_for_fail,
+    get_highest_version,
+    filter_spph,
+    get_build_list,
     )
 from bug_reporting import (
     bug_already_filed_by_url,
-#    get_project_url,
-#    file_bug,
     )
-import config
 from config import (
     get_service_root,
     get_base_url,
@@ -42,9 +40,14 @@
     inited,
     ConfigInitError,
     LaunchpadError,
+    set_verbose_messages,
     )
 
 
+TEST_FAIL_ARCH = 'armel'
+TEST_OK_ARCHS = ['i386', 'amd64']
+
+
 class TestBugTags(TestCase):
 
     def setUp(self):
@@ -69,6 +72,206 @@
                         msg='Got more than default tags without match.')
 
 
+class TestFilter(TestCaseWithFixtures):
+
+    class MockPackage(object):
+        def __init__(self, name, version):
+            self.source_package_version = version
+            self.source_package_name = name
+
+    class MockRecord(object):
+        class CSP:
+            def __init__(self, name, version):
+                self.source_package_name = name
+                self.source_package_version = version
+
+        def __init__(self, arch, datecreated, state=FAILED_BUILD_STATE,
+                     name='packagename', version='1'):
+            self.arch_tag = arch
+            self.datecreated = datecreated
+            self.buildstate = state
+            self.current_source_publication = self.CSP(name, version)
+
+    class MockArchive(object):
+        def __init__(self, name, records_to_return=[]):
+            self.name = name
+            self.records_to_return = records_to_return
+
+        def getBuildRecords(self, build_state, source_name):
+            return self.records_to_return
+
+    def setUp(self):
+        super(TestFilter, self).setUp()
+
+    def init_dummy_config(self):
+        def mock_func(obj, appid, root, cachedir):
+            return 'this is a launchpad instance'
+
+        self.useFixture(MockSomethingFixture(
+                Launchpad, 'login_with', classmethod(mock_func)))
+        init('staging', 'dummyapp', 'dummycache')
+
+    def test_filter_fail_record_not_filtered(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('packagefail', '1'))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE)
+        spph.add_build_record(failrecord)
+        spph_list = dict()
+        spph_list[spph.package_name] = spph
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [])
+        self.assertIn(spph, filtered_list)
+
+    def test_filter_success_record_filtered(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('packagesuccess', '1'))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     SUCCESS_BUILD_STATE)
+        spph.add_build_record(failrecord)
+        spph_list = dict()
+        spph_list[spph.package_name] = spph
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [])
+        self.assertNotIn(spph, filtered_list)
+
+    def test_filter_only_other_arch_filtered(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('packageotherarch', '1'))
+        failrecord = self.MockRecord(TEST_OK_ARCHS[0], datetime.now(),
+                                     FAILED_BUILD_STATE)
+        spph.add_build_record(failrecord)
+        spph_list = dict()
+        spph_list[spph.package_name] = spph
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [])
+        self.assertNotIn(spph, filtered_list)
+
+    def test_filter_no_arch_filtered(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('packagenobuilds', '1'))
+        spph_list = dict()
+        spph_list[spph.package_name] = spph
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [])
+        self.assertNotIn(spph, filtered_list)
+
+    def test_get_build_list_only_returns_exact_matches(self):
+        packagename = 'packagesubstring'
+        exactrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                      SUCCESS_BUILD_STATE, packagename)
+        nonexactrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                         SUCCESS_BUILD_STATE,
+                                         'pre' + packagename + 'post')
+        archive = self.MockArchive('archivename',
+                                   [exactrecord, nonexactrecord])
+        build_records = get_build_list(archive, SUCCESS_BUILD_STATE,
+                                       packagename)
+        self.assertIn(exactrecord, build_records)
+        self.assertNotIn(nonexactrecord, build_records)
+
+    def test_filter_success_on_same_arch_same_version_filtered(self):
+        self.init_dummy_config()
+        packagename = 'packagefailandok'
+        version = '1'
+        fail_spph = SPPH(self.MockPackage(packagename, version))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE, packagename, version)
+        successrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                        SUCCESS_BUILD_STATE, packagename,
+                                        version)
+        fail_spph.add_build_record(failrecord)
+        spph_list = dict()
+        spph_list[fail_spph.package_name] = fail_spph
+        archive = self.MockArchive('archivename', [successrecord])
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [(archive, 'archivename')])
+        self.assertNotIn(fail_spph, filtered_list)
+
+    def test_filter_success_on_other_arch_not_filtered(self):
+        self.init_dummy_config()
+        packagename = 'packagename'
+        version = '1'
+        fail_spph = SPPH(self.MockPackage(packagename, version))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE, packagename, version)
+        successrecord = self.MockRecord(TEST_OK_ARCHS[0], datetime.now(),
+                                        SUCCESS_BUILD_STATE, packagename,
+                                        version)
+        fail_spph.add_build_record(failrecord)
+        spph_list = dict()
+        spph_list[fail_spph.package_name] = fail_spph
+        archive = self.MockArchive('archivename', [successrecord])
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [(archive, 'archivename')])
+        self.assertIn(fail_spph, filtered_list)
+
+    def test_filter_success_on_higher_version_filtered(self):
+        self.init_dummy_config()
+        packagename = 'packagename'
+        low_version = '1'
+        high_version = '1.2.3'
+        fail_spph = SPPH(self.MockPackage(packagename, low_version))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE, packagename, low_version)
+        successrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                        SUCCESS_BUILD_STATE, packagename,
+                                        high_version)
+        fail_spph.add_build_record(failrecord)
+        spph_list = dict()
+        spph_list[fail_spph.package_name] = fail_spph
+        archive = self.MockArchive('archivename', [successrecord])
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [(archive, 'archivename')])
+        self.assertNotIn(fail_spph, filtered_list)
+
+    def test_filter_success_on_lower_version_not_filtered(self):
+        self.init_dummy_config()
+        packagename = 'packagename'
+        low_version = '1'
+        high_version = '1.2.3'
+        fail_spph = SPPH(self.MockPackage(packagename, high_version))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE, packagename, high_version)
+        successrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                        SUCCESS_BUILD_STATE, packagename,
+                                        low_version)
+        fail_spph.add_build_record(failrecord)
+        spph_list = dict()
+        spph_list[fail_spph.package_name] = fail_spph
+        archive = self.MockArchive('archivename', [successrecord])
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [(archive, 'archivename')])
+        self.assertIn(fail_spph, filtered_list)
+
+    def test_filter_success_on_lower_and_higher_version_filtered(self):
+        self.init_dummy_config()
+        packagename = 'midpackage'
+        low_version = '1'
+        mid_version = '2.1.3-1'
+        high_version = '3.2.3'
+        fail_spph = SPPH(self.MockPackage(packagename, mid_version))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE, packagename, mid_version)
+        low_successrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                            SUCCESS_BUILD_STATE, packagename,
+                                            low_version)
+        high_successrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                             SUCCESS_BUILD_STATE, packagename,
+                                             high_version)
+        fail_spph.add_build_record(failrecord)
+        spph_list = dict()
+        spph_list[fail_spph.package_name] = fail_spph
+        low_archive = self.MockArchive('archive_with_lower_version',
+                                       [low_successrecord])
+        high_archive = self.MockArchive('archive_with_higher_version',
+                                        [high_successrecord])
+        filtered_list = filter_spph(spph_list, TEST_FAIL_ARCH, TEST_OK_ARCHS,
+                                    [(low_archive, 'lo_archivename'),
+                                     (high_archive, 'hi_archivename')])
+        self.assertNotIn(fail_spph, filtered_list)
+
+
 class TestSPPH(TestCaseWithFixtures):
 
     class MockPackage(object):
@@ -76,7 +279,7 @@
             self.source_package_version = version
             self.source_package_name = name
 
-    class MockLog(object):
+    class MockRecord(object):
         def __init__(self, arch, datecreated, state=FAILED_BUILD_STATE):
             self.arch_tag = arch
             self.datecreated = datecreated
@@ -103,90 +306,96 @@
         spph = SPPH(self.MockPackage('dummy', '1.1.2-3'))
         self.assertEquals(spph.version, '1.1.2-3')
 
-    def test_new_sppg_has_no_logs(self):
+    def test_new_spph_has_no_records(self):
         self.init_dummy_config()
         spph = SPPH(self.MockPackage('dummy', '1.1.2-3'))
-        self.assertTrue(len(spph.logs) == 0)
+        self.assertEqual({}, spph.records)
 
-    def test_add_build_log_does_so(self):
+    def test_add_build_record_does_so(self):
         self.init_dummy_config()
         spph = SPPH(self.MockPackage('dummy', '1'))
-        log = self.MockLog('armel', datetime.now())
-        spph.add_build_log(log)
-        self.assertEquals(spph.logs['armel'], log)
+        record = self.MockRecord(TEST_FAIL_ARCH, datetime.now())
+        spph.add_build_record(record)
+        self.assertEquals(spph.records[TEST_FAIL_ARCH], record)
 
     def test_get_arch_does_so(self):
         self.init_dummy_config()
         spph = SPPH(self.MockPackage('dummy', '1'))
-        log = self.MockLog('armel', datetime.now())
-        spph.add_build_log(log)
-        self.assertEquals(spph.get_arch('armel'), log)
-        self.assertEquals(spph.get_arch('i386'), None)
-
-    def test_add_newer_build_log_replaces(self):
-        self.init_dummy_config()
-        spph = SPPH(self.MockPackage('dummy', '1'))
-        oldlog = self.MockLog('armel', datetime.now())
-        newlog = self.MockLog('armel', datetime.now())
-        spph.add_build_log(oldlog)
-        spph.add_build_log(newlog)
-        self.assertEquals(spph.logs['armel'], newlog)
-
-    def test_add_older_build_log_dont_replace(self):
-        self.init_dummy_config()
-        spph = SPPH(self.MockPackage('dummy', '1'))
-        oldlog = self.MockLog('armel', datetime.now())
-        newlog = self.MockLog('armel', datetime.now())
-        spph.add_build_log(newlog)
-        spph.add_build_log(oldlog)
-        self.assertEquals(spph.logs['armel'], newlog)
-
-    def test_add_other_arch_build_log_dont_replace(self):
-        self.init_dummy_config()
-        spph = SPPH(self.MockPackage('dummy', '1'))
-        armlog = self.MockLog('armel', datetime.now())
-        i386log = self.MockLog('i386', datetime.now())
-        spph.add_build_log(armlog)
-        spph.add_build_log(i386log)
-        self.assertEquals(spph.logs['armel'], armlog)
-        self.assertEquals(spph.logs['i386'], i386log)
-
-    def test_is_not_ftbfs_when_no_log(self):
-        self.init_dummy_config()
-        spph = SPPH(self.MockPackage('dummy', '1'))
-        self.assertFalse(spph.is_ftbfs('armel'))
-
-    def test_is_ftbfs_when_failed_log(self):
-        self.init_dummy_config()
-        spph = SPPH(self.MockPackage('dummy', '1'))
-        faillog = self.MockLog('armel', datetime.now(), FAILED_BUILD_STATE)
-        spph.add_build_log(faillog)
-        self.assertTrue(spph.is_ftbfs('armel'))
-
-    def test_is_not_ftbfs_when_success_log(self):
-        self.init_dummy_config()
-        spph = SPPH(self.MockPackage('dummy', '1'))
-        successlog = self.MockLog('armel', datetime.now(), SUCCESS_BUILD_STATE)
-        spph.add_build_log(successlog)
-        self.assertFalse(spph.is_ftbfs('armel'))
-
-    def test_is_not_ftbfs_when_replaced_log(self):
-        self.init_dummy_config()
-        spph = SPPH(self.MockPackage('dummy', '1'))
-        faillog = self.MockLog('armel', datetime.now(), FAILED_BUILD_STATE)
-        successlog = self.MockLog('armel', datetime.now(), SUCCESS_BUILD_STATE)
-        spph.add_build_log(faillog)
-        spph.add_build_log(successlog)
-        self.assertFalse(spph.is_ftbfs('armel'))
-
-    def test_is_ftbfs_when_replaced_log(self):
-        self.init_dummy_config()
-        spph = SPPH(self.MockPackage('dummy', '1'))
-        successlog = self.MockLog('armel', datetime.now(), SUCCESS_BUILD_STATE)
-        faillog = self.MockLog('armel', datetime.now(), FAILED_BUILD_STATE)
-        spph.add_build_log(successlog)
-        spph.add_build_log(faillog)
-        self.assertTrue(spph.is_ftbfs('armel'))
+        record = self.MockRecord(TEST_FAIL_ARCH, datetime.now())
+        spph.add_build_record(record)
+        self.assertEquals(spph.get_arch(TEST_FAIL_ARCH), record)
+        self.assertEquals(spph.get_arch(TEST_OK_ARCHS[0]), None)
+
+    def test_add_newer_build_record_replaces(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('dummy', '1'))
+        oldrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now())
+        newrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now())
+        spph.add_build_record(oldrecord)
+        spph.add_build_record(newrecord)
+        self.assertEquals(spph.records[TEST_FAIL_ARCH], newrecord)
+
+    def test_add_older_build_record_dont_replace(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('dummy', '1'))
+        oldrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now())
+        newrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now())
+        spph.add_build_record(newrecord)
+        spph.add_build_record(oldrecord)
+        self.assertEquals(spph.records[TEST_FAIL_ARCH], newrecord)
+
+    def test_add_other_arch_build_record_dont_replace(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('dummy', '1'))
+        armrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now())
+        i386record = self.MockRecord(TEST_OK_ARCHS[0], datetime.now())
+        spph.add_build_record(armrecord)
+        spph.add_build_record(i386record)
+        self.assertEquals(spph.records[TEST_FAIL_ARCH], armrecord)
+        self.assertEquals(spph.records[TEST_OK_ARCHS[0]], i386record)
+
+    def test_is_not_ftbfs_when_no_record(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('dummy', '1'))
+        self.assertFalse(spph.is_ftbfs(TEST_FAIL_ARCH))
+
+    def test_is_ftbfs_when_failed_record(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('dummy', '1'))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE)
+        spph.add_build_record(failrecord)
+        self.assertTrue(spph.is_ftbfs(TEST_FAIL_ARCH))
+
+    def test_is_not_ftbfs_when_success_record(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('dummy', '1'))
+        successrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                        SUCCESS_BUILD_STATE)
+        spph.add_build_record(successrecord)
+        self.assertFalse(spph.is_ftbfs(TEST_FAIL_ARCH))
+
+    def test_is_not_ftbfs_when_replaced_record(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('dummy', '1'))
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE)
+        successrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                        SUCCESS_BUILD_STATE)
+        spph.add_build_record(failrecord)
+        spph.add_build_record(successrecord)
+        self.assertFalse(spph.is_ftbfs(TEST_FAIL_ARCH))
+
+    def test_is_ftbfs_when_replaced_record(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('dummy', '1'))
+        successrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                        SUCCESS_BUILD_STATE)
+        failrecord = self.MockRecord(TEST_FAIL_ARCH, datetime.now(),
+                                     FAILED_BUILD_STATE)
+        spph.add_build_record(successrecord)
+        spph.add_build_record(failrecord)
+        self.assertTrue(spph.is_ftbfs(TEST_FAIL_ARCH))
 
     def test_spph_is_higher_version(self):
         self.init_dummy_config()
@@ -212,6 +421,48 @@
         newspph = SPPH(self.MockPackage('packageversion', '0.5.32-0ubuntu2'))
         self.assertTrue(newspph.is_higher_version_than(oldspph))
 
+    def test_get_highest_version_finds_single_version(self):
+        self.init_dummy_config()
+        spph = SPPH(self.MockPackage('packageversion', '0.5.32-0ubuntu2'))
+        spphlist = [spph]
+        self.assertEquals(spph, get_highest_version(spphlist))
+
+    def test_get_highest_version_returns_first_of_same_version(self):
+        self.init_dummy_config()
+        spph_good = SPPH(self.MockPackage('packageversion', '0.5.32-0ubuntu2'))
+        spph_bad = SPPH(self.MockPackage('packageversion', '0.5.32-0ubuntu2'))
+        spphlist = [spph_good, spph_bad]
+        self.assertEquals(spph_good, get_highest_version(spphlist))
+
+    def test_get_highest_version_returns_highest_version_in_order(self):
+        self.init_dummy_config()
+        spph_good = SPPH(self.MockPackage('packageversion', '0.5.32-0ubuntu3'))
+        spph_bad = SPPH(self.MockPackage('packageversion', '0.5.32-0ubuntu2'))
+        spphlist = [spph_good, spph_bad]
+        self.assertEquals(spph_good, get_highest_version(spphlist))
+
+    def test_get_highest_version_returns_highest_version_out_of_order(self):
+        self.init_dummy_config()
+        spph_good = SPPH(self.MockPackage('packageversion', '0.5.32-0ubuntu3'))
+        spph_bad = SPPH(self.MockPackage('packageversion', '0.5.32-0ubuntu2'))
+        spphlist = [spph_bad, spph_good]
+        self.assertEquals(spph_good, get_highest_version(spphlist))
+
+    def test_get_highest_version_returns_highest_version_mixed_order(self):
+        self.init_dummy_config()
+        spph_good = SPPH(self.MockPackage('packageversion', '99'))
+        spph_bad1 = SPPH(self.MockPackage('packageversion', '1'))
+        spph_bad2 = SPPH(self.MockPackage('packageversion', '2'))
+        spph_bad3 = SPPH(self.MockPackage('packageversion', '3'))
+        spphlist = [spph_bad1, spph_bad2, spph_bad1, spph_good, spph_bad3]
+        self.assertEquals(spph_good, get_highest_version(spphlist))
+
+    def test_get_highest_version_returns_null(self):
+        self.init_dummy_config()
+        self.assertTrue(get_highest_version([]) is None)
+        self.assertTrue(get_highest_version([None]) is None)
+        self.assertTrue(get_highest_version([None, None]) is None)
+
 
 class TestFileBug(TestCase):