add a test for the already inserted case

this removes a TODO
buildbot · Feb 16, 2015 · af1c53c · af1c53c
1 parent 443f9b9
commit af1c53c
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 2 deletions.
diff --git a/master/buildbot/db/buildslaves.py b/master/buildbot/db/buildslaves.py
@@ -75,9 +75,19 @@ def thd(conn):
                              [{'buildslaveid': buildslaveid, 'buildermasterid': buildermasterid}
                               for buildermasterid in buildermasterids])
             except (sa.exc.IntegrityError, sa.exc.ProgrammingError):
-                # TODO
-                # if the row is already present, silently fail..
+                # if some rows are already present, insert one by one.
                 pass
+            else:
+                return
+
+            for buildermasterid in buildermasterids:
+                # insert them one by one
+                q = cfg_tbl.insert()
+                try:
+                    conn.execute(q, {'buildslaveid': buildslaveid, 'buildermasterid': buildermasterid})
+                except (sa.exc.IntegrityError, sa.exc.ProgrammingError):
+                    # if the row is already present, silently fail..
+                    pass
 
         return self.db.pool.do(thd)
 

diff --git a/master/buildbot/test/fake/fakedb.py b/master/buildbot/test/fake/fakedb.py
@@ -1343,6 +1343,7 @@ def insertTestData(self, rows):
                     name=row.name,
                     info=row.info)
             elif isinstance(row, ConfiguredBuildslave):
+                row.id = row.buildermasterid * 10000 + row.buildslaveid
                 self.configured[row.id] = dict(
                     buildermasterid=row.buildermasterid,
                     buildslaveid=row.buildslaveid)

diff --git a/master/buildbot/test/unit/test_db_buildslaves.py b/master/buildbot/test/unit/test_db_buildslaves.py
@@ -458,6 +458,27 @@ def test_buildslaveConfigured(self):
             {'builderid': 20, 'masterid': 10},
             {'builderid': 22, 'masterid': 10}]))
 
+    @defer.inlineCallbacks
+    def test_buildslaveConfiguredTwice(self):
+        yield self.insertTestData(self.baseRows + self.multipleMasters)
+
+        # should remove builder 21, and add 22
+        yield self.db.buildslaves.deconfigureAllBuidslavesForMaster(masterid=10)
+
+        yield self.db.buildslaves.buildslaveConfigured(
+            buildslaveid=30, masterid=10, builderids=[20, 22])
+
+        # configure again (should eat the duplicate insertion errors)
+        yield self.db.buildslaves.buildslaveConfigured(
+            buildslaveid=30, masterid=10, builderids=[20, 21, 22])
+
+        bs = yield self.db.buildslaves.getBuildslave(30)
+        self.assertEqual(sorted(bs['configured_on']), sorted([
+            {'builderid': 20, 'masterid': 11},
+            {'builderid': 20, 'masterid': 10},
+            {'builderid': 21, 'masterid': 10},
+            {'builderid': 22, 'masterid': 10}]))
+
     @defer.inlineCallbacks
     def test_nothingConfigured(self):
         yield self.insertTestData(self.baseRows + self.multipleMasters)