From 0d222614c5996b0b3bc3ca0dc30629c4199f50a4 Mon Sep 17 00:00:00 2001 From: Jared Grubb Date: Mon, 10 Feb 2014 12:53:43 -0800 Subject: [PATCH] Fix #928: Allow builder to be associated with multiple tags --- master/buildbot/config.py | 30 ++++++++--- master/buildbot/interfaces.py | 5 +- master/buildbot/process/builder.py | 10 ++-- master/buildbot/status/builder.py | 33 +++++++----- master/buildbot/status/mail.py | 33 +++++++----- master/buildbot/status/master.py | 22 ++++---- master/buildbot/status/words.py | 51 ++++++++++--------- master/buildbot/test/fake/fakebuild.py | 8 ++- master/buildbot/test/fake/fakemaster.py | 22 +++++--- master/buildbot/test/unit/test_config.py | 27 ++++++++-- .../buildbot/test/unit/test_process_build.py | 4 -- .../test/unit/test_process_builder.py | 8 +-- .../test/unit/test_process_properties.py | 18 +++---- .../test/unit/test_status_builder_cache.py | 4 +- .../test/unit/test_status_buildstep.py | 5 +- master/buildbot/test/unit/test_status_mail.py | 45 +++++++++++++--- .../buildbot/test/unit/test_status_words.py | 7 ++- master/buildbot/test/util/steps.py | 2 +- 18 files changed, 211 insertions(+), 123 deletions(-) diff --git a/master/buildbot/config.py b/master/buildbot/config.py index f4562ed247e..23f21ebddd2 100644 --- a/master/buildbot/config.py +++ b/master/buildbot/config.py @@ -57,6 +57,9 @@ def error(error): else: raise ConfigErrors([error]) +def warnDeprecated(version, msg): + # for now just log the deprecation + log.msg("NOTE: [%s and later] %s" % (version, msg)) class MasterConfig(util.ComparableMixin): @@ -382,7 +385,7 @@ def load_db(self, filename, config_dict): # db_poll_interval is deprecated if 'db_poll_interval' in self.db: - log.msg("NOTE: db_poll_interval is deprecated and will be ignored") + warnDeprecated("0.8.7", "db_poll_interval is deprecated and will be ignored") del self.db['db_poll_interval'] def load_mq(self, filename, config_dict): @@ -688,7 +691,8 @@ def check_ports(self): class BuilderConfig(util_config.ConfiguredMixin): def __init__(self, name=None, slavename=None, slavenames=None, - builddir=None, slavebuilddir=None, factory=None, category=None, + builddir=None, slavebuilddir=None, factory=None, + tags=None, category=None, nextSlave=None, nextBuild=None, locks=None, env=None, properties=None, mergeRequests=None, description=None, canStartBuild=None): @@ -743,10 +747,22 @@ def __init__(self, name=None, slavename=None, slavenames=None, self.slavebuilddir = slavebuilddir # remainder are optional - if category is not None and not isinstance(category, str): - error("builder '%s': category must be a string" % (name,)) - self.category = category or '' + if category and tags: + error("builder '%s': category is deprecated and replaced by tags; you should only specify tags" % (name,)) + if category: + warnDeprecated("0.9", "category is deprecated and should be replaced with 'tags=[cat]'") + if not isinstance(category, str): + error("builder '%s': category must be a string" % (name,)) + tags = [category] + if tags: + if not isinstance(tags, list): + error("builder '%s': tags must be a list" % (name,)) + bad_tags = any((tag for tag in tags if not isinstance(tag, str))) + if bad_tags: + error("builder '%s': tags list contains something that is not a string" % (name,)) + self.tags = tags + self.nextSlave = nextSlave if nextSlave and not callable(nextSlave): error('nextSlave must be a callable') @@ -776,8 +792,8 @@ def getConfigDict(self): 'builddir': self.builddir, 'slavebuilddir': self.slavebuilddir, } - if self.category: - rv['category'] = self.category + if self.tags: + rv['tags'] = self.tags if self.nextSlave: rv['nextSlave'] = self.nextSlave if self.nextBuild: diff --git a/master/buildbot/interfaces.py b/master/buildbot/interfaces.py index 39cdedec78d..126ee83f5b3 100644 --- a/master/buildbot/interfaces.py +++ b/master/buildbot/interfaces.py @@ -151,7 +151,7 @@ def getSchedulers(): """Return a list of ISchedulerStatus objects for all currently-registered Schedulers.""" - def getBuilderNames(categories=None): + def getBuilderNames(tags=None): """Return a list of the names of all current Builders.""" def getBuilder(name): """Return the IBuilderStatus object for a given named Builder. Raises @@ -328,9 +328,6 @@ class IBuilderStatus(Interface): def getName(): """Return the name of this Builder (a string).""" - def getCategory(): - """Return the category of this builder (a string).""" - def getDescription(): """Return the description of this builder (a string).""" diff --git a/master/buildbot/process/builder.py b/master/buildbot/process/builder.py index b6ba6331830..5380f7ee70d 100644 --- a/master/buildbot/process/builder.py +++ b/master/buildbot/process/builder.py @@ -102,10 +102,10 @@ def reconfigService(self, new_config): # set up a builder status object on the first reconfig if not self.builder_status: self.builder_status = self.master.status.builderAdded( - builder_config.name, - builder_config.builddir, - builder_config.category, - builder_config.description) + name=builder_config.name, + basedir=builder_config.builddir, + tags=builder_config.tags, + description=builder_config.description) self.config = builder_config @@ -115,7 +115,7 @@ def reconfigService(self, new_config): yield self.getBuilderId() self.builder_status.setDescription(builder_config.description) - self.builder_status.setCategory(builder_config.category) + self.builder_status.setTags(builder_config.tags) self.builder_status.setSlavenames(self.config.slavenames) self.builder_status.setCacheSize(new_config.caches['Builds']) diff --git a/master/buildbot/status/builder.py b/master/buildbot/status/builder.py index 4f5bc70a407..3cb3a88efd7 100644 --- a/master/buildbot/status/builder.py +++ b/master/buildbot/status/builder.py @@ -61,23 +61,23 @@ class BuilderStatus(styles.Versioned): I live in the buildbot.process.build.Builder object, in the .builder_status attribute. - @type category: string - @ivar category: user-defined category this builder belongs to; can be + @type tags: None or list of strings + @ivar tags: user-defined "tag" this builder has; can be used to filter on in status clients """ implements(interfaces.IBuilderStatus, interfaces.IEventSource) - persistenceVersion = 1 + persistenceVersion = 2 persistenceForgets = ('wasUpgraded', ) - category = None + tags = None currentBigState = "offline" # or idle/waiting/interlocked/building basedir = None # filled in by our parent - def __init__(self, buildername, category, master, description): + def __init__(self, buildername, tags, master, description): self.name = buildername - self.category = category + self.tags = tags self.description = description self.master = master @@ -133,6 +133,12 @@ def upgradeToVersion1(self): del self.nextBuildNumber # determineNextBuildNumber chooses this self.wasUpgraded = True + def upgradeToVersion2(self): + if hasattr(self, 'category'): + self.tags = self.category and [self.category] or None + del self.category + self.wasUpgraded = True + def determineNextBuildNumber(self): """Scan our directory of saved BuildStatus instances to determine what our self.nextBuildNumber should be. Set it one larger than the @@ -319,12 +325,15 @@ def getLastFinishedBuild(self): b = self.getBuild(-2) return b - def setCategory(self, category): + def getTags(self): + return self.tags + + def setTags(self, tags): # used during reconfig - self.category = category + self.tags = tags - def getCategory(self): - return self.category + def matchesAnyTag(self, tags): + return self.tags and any(tag for tag in self.tags if tag in tags) def getBuildByRevision(self, rev): number = self.nextBuildNumber - 1 @@ -435,7 +444,7 @@ def eventGenerator(self, branches=[], categories=[], committers=[], projects=[], # sourcestamps match, skip this build if branches and not branches & self._getBuildBranches(b): continue - if categories and not b.getBuilder().getCategory() in categories: + if categories and not b.getBuilder().matchesAnyTag(tags=categories): continue if committers and not [True for c in b.getChanges() if c.who in committers]: continue @@ -580,7 +589,7 @@ def asDict(self): # Constant # TODO(maruel): Fix me. We don't want to leak the full path. result['basedir'] = os.path.basename(self.basedir) - result['category'] = self.category + result['tags'] = self.getTags() result['slaves'] = self.slavenames result['schedulers'] = [s.name for s in self.status.master.allSchedulers() diff --git a/master/buildbot/status/mail.py b/master/buildbot/status/mail.py index 8876c51a7da..2bc899b5842 100644 --- a/master/buildbot/status/mail.py +++ b/master/buildbot/status/mail.py @@ -248,7 +248,7 @@ class MailNotifier(base.StatusReceiverMultiService, buildset.BuildSetSummaryNoti implements(interfaces.IEmailSender) compare_attrs = ["extraRecipients", "lookup", "fromaddr", "mode", - "categories", "builders", "addLogs", "relayhost", + "tags", "builders", "addLogs", "relayhost", "subject", "sendToInterestedUsers", "customMesg", "messageFormatter", "extraHeaders"] @@ -256,7 +256,7 @@ class MailNotifier(base.StatusReceiverMultiService, buildset.BuildSetSummaryNoti "exception", "cancelled") def __init__(self, fromaddr, mode=("failing", "passing", "warnings"), - categories=None, builders=None, addLogs=False, + tags=None, builders=None, addLogs=False, relayhost="localhost", buildSetSummary=False, subject="buildbot %(result)s in %(title)s on %(builder)s", lookup=None, extraRecipients=[], @@ -264,7 +264,9 @@ def __init__(self, fromaddr, mode=("failing", "passing", "warnings"), messageFormatter=defaultMessage, extraHeaders=None, addPatch=True, useTls=False, smtpUser=None, smtpPassword=None, smtpPort=25, - previousBuildGetter=defaultGetPreviousBuild): + previousBuildGetter=defaultGetPreviousBuild, + categories=None # deprecated, use tags + ): """ @type fromaddr: string @param fromaddr: the email address to be used in the 'From' header. @@ -305,14 +307,17 @@ def __init__(self, fromaddr, mode=("failing", "passing", "warnings"), @type builders: list of strings @param builders: a list of builder names for which mail should be sent. Defaults to None (send mail for all builds). - Use either builders or categories, but not both. + Use either builders or tags, but not both. - @type categories: list of strings - @param categories: a list of category names to serve status + @type tags: list of strings + @param tags: a list of tag names to serve status information for. Defaults to None (all - categories). Use either builders or categories, + tags). Use either builders or tags, but not both. + @type categories: list of strings + @param categories: (this attribute is deprecated; use 'tags' instead) + @type addLogs: boolean @param addLogs: if True, include all build logs as attachments to the messages. These can be quite large. This can also be @@ -419,7 +424,7 @@ def __init__(self, fromaddr, mode=("failing", "passing", "warnings"), config.error( "mode %s is not a valid mode" % (m,)) self.mode = mode - self.categories = categories + self.tags = tags or categories self.builders = builders self.addLogs = addLogs self.relayhost = relayhost @@ -449,10 +454,10 @@ def __init__(self, fromaddr, mode=("failing", "passing", "warnings"), self.watched = [] self.master_status = None - # you should either limit on builders or categories, not both - if self.builders is not None and self.categories is not None: + # you should either limit on builders or tags, not both + if self.builders is not None and self.tags is not None: config.error( - "Please specify only builders or categories to include - " + + "Please specify only builders or tags to include - " + "not both.") if customMesg: @@ -485,7 +490,7 @@ def disownServiceParent(self): def builderAdded(self, name, builder): # only subscribe to builders we are interested in - if self.categories is not None and builder.category not in self.categories: + if self.tags is not None and not builder.matchesAnyTag(self.tags): return None self.watched.append(builder) @@ -505,8 +510,8 @@ def isMailNeeded(self, build, results): builder = build.getBuilder() if self.builders is not None and builder.name not in self.builders: return False # ignore this build - if self.categories is not None and \ - builder.category not in self.categories: + if self.tags is not None and \ + not builder.matchesAnyTag(self.tags): return False # ignore this build prev = self.getPreviousBuild(build) diff --git a/master/buildbot/status/master.py b/master/buildbot/status/master.py index f9efc6e5362..2b485286b3b 100644 --- a/master/buildbot/status/master.py +++ b/master/buildbot/status/master.py @@ -221,15 +221,19 @@ def chdict2change(chdict): def getSchedulers(self): return self.master.allSchedulers() - def getBuilderNames(self, categories=None): - if categories is None: + def getBuilderNames(self, tags=None, categories=None): + if categories is not None: + # Categories is deprecated; pretend they said "tags". + tags = categories + + if tags is None: return util.naturalSort(self.botmaster.builderNames) # don't let them break it l = [] # respect addition order for name in self.botmaster.builderNames: - bldr = self.botmaster.builders[name] - if bldr.config.category in categories: + bldr = self.getBuilder(name) + if bldr.matchesAnyTag(tags): l.append(name) return util.naturalSort(l) @@ -328,7 +332,7 @@ def announceNewBuilder(self, target, name, builder_status): if t: builder_status.subscribe(t) - def builderAdded(self, name, basedir, category=None, description=None): + def builderAdded(self, name, basedir, tags=None, description=None): """ @rtype: L{BuilderStatus} """ @@ -358,13 +362,13 @@ def builderAdded(self, name, basedir, category=None, description=None): log.msg("error follows:") log.err() if not builder_status: - builder_status = builder.BuilderStatus(name, category, self.master, + builder_status = builder.BuilderStatus(name, tags, self.master, description) builder_status.addPointEvent(["builder", "created"]) - log.msg("added builder %s in category %s" % (name, category)) - # an unpickled object might not have category set from before, + log.msg("added builder %s with tags %r" % (name, tags)) + # an unpickled object might not have tags set from before, # so set it here to make sure - builder_status.category = category + builder_status.setTags(tags) builder_status.description = description builder_status.master = self.master builder_status.basedir = os.path.join(self.basedir, basedir) diff --git a/master/buildbot/status/words.py b/master/buildbot/status/words.py index bf99f58225b..7384216e1ea 100644 --- a/master/buildbot/status/words.py +++ b/master/buildbot/status/words.py @@ -207,7 +207,7 @@ def getAllBuilders(self): """ @rtype: list of L{buildbot.process.builder.Builder} """ - names = sorted(self.bot.status.getBuilderNames(categories=self.bot.categories)) + names = sorted(self.bot.status.getBuilderNames(tags=self.bot.tags)) builders = [self.bot.status.getBuilder(n) for n in names] return builders @@ -389,8 +389,8 @@ def command_WATCH(self, args, who): command_WATCH.usage = "watch - announce the completion of an active build" def builderAdded(self, builderName, builder): - if (self.bot.categories is not None and - builder.category not in self.bot.categories): + if (self.bot.tags is not None and + not builder.matchesAnyTag(tags=self.bot.tags)): return log.msg('[Contact] Builder %s added' % (builderName)) @@ -401,13 +401,13 @@ def builderRemoved(self, builderName): def buildStarted(self, builderName, build): builder = build.getBuilder() - log.msg('[Contact] Builder %r in category %s started' % (builder, builder.category)) + log.msg('[Contact] Builder %r started' % (builder,)) # only notify about builders we are interested in - if (self.bot.categories is not None and - builder.category not in self.bot.categories): - log.msg('Not notifying for a build in the wrong category') + if (self.bot.tags is not None and + not builder.matchesAnyTag(tags=self.bot.tags)): + log.msg('Not notifying for a build that does not match any tags') return if not self.notify_for('started'): @@ -453,8 +453,8 @@ def getResultsDescriptionAndColor(self, results): def buildFinished(self, builderName, build, results): builder = build.getBuilder() - if (self.bot.categories is not None and - builder.category not in self.bot.categories): + if (self.bot.tags is not None and + not builder.matchesAnyTag(tags=self.bot.tags)): return if not self.notify_for_finished(build): @@ -510,8 +510,8 @@ def watchedBuildFinished(self, b): # only notify about builders we are interested in builder = b.getBuilder() - if (self.bot.categories is not None and - builder.category not in self.bot.categories): + if (self.bot.tags is not None and + not builder.matchesAnyTag(tags=self.bot.tags)): return builder_name = builder.getName() @@ -884,15 +884,17 @@ class IrcStatusBot(irc.IRCClient): contactClass = IRCContact def __init__(self, nickname, password, channels, pm_to_nicks, status, - categories, notify_events, noticeOnChannel=False, - useRevisions=False, showBlameList=False, useColors=True): + tags, notify_events, noticeOnChannel=False, + useRevisions=False, showBlameList=False, useColors=True, + categories=None # deprecated + ): self.nickname = nickname self.channels = channels self.pm_to_nicks = pm_to_nicks self.password = password self.status = status self.master = status.master - self.categories = categories + self.tags = tags or categories self.notify_events = notify_events self.hasQuit = 0 self.contacts = {} @@ -1004,9 +1006,11 @@ class IrcStatusFactory(ThrottledClientFactory): shuttingDown = False p = None - def __init__(self, nickname, password, channels, pm_to_nicks, categories, notify_events, + def __init__(self, nickname, password, channels, pm_to_nicks, tags, notify_events, noticeOnChannel=False, useRevisions=False, showBlameList=False, - lostDelay=None, failedDelay=None, useColors=True, allowShutdown=False): + lostDelay=None, failedDelay=None, useColors=True, allowShutdown=False, + categories=None # deprecated + ): ThrottledClientFactory.__init__(self, lostDelay=lostDelay, failedDelay=failedDelay) self.status = None @@ -1014,7 +1018,7 @@ def __init__(self, nickname, password, channels, pm_to_nicks, categories, notify self.password = password self.channels = channels self.pm_to_nicks = pm_to_nicks - self.categories = categories + self.tags = tags or categories self.notify_events = notify_events self.noticeOnChannel = noticeOnChannel self.useRevisions = useRevisions @@ -1035,7 +1039,7 @@ def shutdown(self): def buildProtocol(self, address): p = self.protocol(self.nickname, self.password, self.channels, self.pm_to_nicks, self.status, - self.categories, self.notify_events, + self.tags, self.notify_events, noticeOnChannel=self.noticeOnChannel, useColors=self.useColors, useRevisions=self.useRevisions, @@ -1069,14 +1073,15 @@ class IRC(base.StatusReceiverMultiService): compare_attrs = ["host", "port", "nick", "password", "channels", "pm_to_nicks", "allowForce", "useSSL", - "useRevisions", "categories", "useColors", + "useRevisions", "tags", "useColors", "lostDelay", "failedDelay", "allowShutdown"] def __init__(self, host, nick, channels, pm_to_nicks=[], port=6667, - allowForce=False, categories=None, password=None, notify_events={}, + allowForce=False, tags=None, password=None, notify_events={}, noticeOnChannel=False, showBlameList=True, useRevisions=False, useSSL=False, lostDelay=None, failedDelay=None, useColors=True, - allowShutdown=False): + allowShutdown=False, categories=None # categories is deprecated + ): base.StatusReceiverMultiService.__init__(self) if allowForce not in (True, False): @@ -1093,13 +1098,13 @@ def __init__(self, host, nick, channels, pm_to_nicks=[], port=6667, self.password = password self.allowForce = allowForce self.useRevisions = useRevisions - self.categories = categories + self.tags = tags or categories self.notify_events = notify_events self.allowShutdown = allowShutdown self.f = IrcStatusFactory(self.nick, self.password, self.channels, self.pm_to_nicks, - self.categories, self.notify_events, + self.tags, self.notify_events, noticeOnChannel=noticeOnChannel, useRevisions=useRevisions, showBlameList=showBlameList, diff --git a/master/buildbot/test/fake/fakebuild.py b/master/buildbot/test/fake/fakebuild.py index 3f5d1d6dc69..70f2bc567ed 100644 --- a/master/buildbot/test/fake/fakebuild.py +++ b/master/buildbot/test/fake/fakebuild.py @@ -20,6 +20,7 @@ from buildbot import interfaces from buildbot.process import factory from buildbot.process import properties +from buildbot.test.fake import fakemaster from twisted.python import components @@ -40,9 +41,9 @@ def getInterestedUsers(self): class FakeBuild(properties.PropertiesMixin): - def __init__(self, props=None): + def __init__(self, props=None, master=None): self.build_status = FakeBuildStatus() - self.builder = mock.Mock(name='build.builder') + self.builder = fakemaster.FakeBuilderStatus(master) self.builder.config = config.BuilderConfig( name='bldr', slavenames=['a'], @@ -66,6 +67,9 @@ def getSourceStamp(self, codebase): def allFiles(self): return [] + def getBuilder(self): + return self.builder + components.registerAdapter( lambda build: build.build_status.properties, diff --git a/master/buildbot/test/fake/fakemaster.py b/master/buildbot/test/fake/fakemaster.py index db147bdec79..78f4bc1d051 100644 --- a/master/buildbot/test/fake/fakemaster.py +++ b/master/buildbot/test/fake/fakemaster.py @@ -62,7 +62,7 @@ def __init__(self, master): self.master = master self.lastBuilderStatus = None - def builderAdded(self, name, basedir, category=None, description=None): + def builderAdded(self, name, basedir, tags=None, description=None): bs = FakeBuilderStatus(self.master) self.lastBuilderStatus = bs return bs @@ -90,10 +90,13 @@ class FakeBuilderStatus(object): implements(interfaces.IBuilderStatus) - def __init__(self, master): - self.master = master - self.basedir = os.path.join(master.basedir, 'bldr') + def __init__(self, master=None, buildername="Builder"): + if master: + self.master = master + self.basedir = os.path.join(master.basedir, 'bldr') self.lastBuildStatus = None + self._tags = None + self.name = buildername def setDescription(self, description): self._description = description @@ -101,11 +104,14 @@ def setDescription(self, description): def getDescription(self): return self._description - def setCategory(self, category): - self._category = category + def getTags(self): + return self._tags + + def setTags(self, tags): + self._tags = tags - def getCategory(self): - return self._category + def matchesAnyTag(self, tags): + return set(self._tags) & set(tags) def setSlavenames(self, names): pass diff --git a/master/buildbot/test/unit/test_config.py b/master/buildbot/test/unit/test_config.py index 348b19339b5..e5ee0df365c 100644 --- a/master/buildbot/test/unit/test_config.py +++ b/master/buildbot/test/unit/test_config.py @@ -1131,6 +1131,25 @@ def test_bogus_category(self): lambda: config.BuilderConfig(category=13, name='a', slavenames=['a'], factory=self.factory)) + def test_tags_must_be_list(self): + self.assertRaisesConfigError( + "tags must be a list", + lambda: config.BuilderConfig(tags='abc', + name='a', slavenames=['a'], factory=self.factory)) + + def test_tags_must_be_list_of_str(self): + self.assertRaisesConfigError( + "tags list contains something that is not a string", + lambda: config.BuilderConfig(tags=['abc', 13], + name='a', slavenames=['a'], factory=self.factory)) + + def test_tags_no_categories_too(self): + self.assertRaisesConfigError( + "category is deprecated and replaced by tags; you should only specify tags", + lambda: config.BuilderConfig(tags=['abc'], + category='def', + name='a', slavenames=['a'], factory=self.factory)) + def test_inv_nextSlave(self): self.assertRaisesConfigError( "nextSlave must be a callable", @@ -1164,7 +1183,7 @@ def test_defaults(self): slavenames=['a'], builddir='a_b_c', slavebuilddir='a_b_c', - category='', + tags=None, nextSlave=None, locks=[], env={}, @@ -1192,7 +1211,7 @@ def test_args(self): slavenames=['s2', 's1'], builddir='bd', slavebuilddir='sbd', - category='c', + tags=['c'], locks=['l'], env={'x': 10}, properties={'y': 20}, @@ -1204,12 +1223,12 @@ def test_getConfigDict(self): nb = lambda: 'nb' cfg = config.BuilderConfig( name='b', slavename='s1', slavenames='s2', builddir='bd', - slavebuilddir='sbd', factory=self.factory, category='c', + slavebuilddir='sbd', factory=self.factory, tags=['c'], nextSlave=ns, nextBuild=nb, locks=['l'], env=dict(x=10), properties=dict(y=20), mergeRequests='mr', description='buzz') self.assertEqual(cfg.getConfigDict(), {'builddir': 'bd', - 'category': 'c', + 'tags': ['c'], 'description': 'buzz', 'env': {'x': 10}, 'factory': self.factory, diff --git a/master/buildbot/test/unit/test_process_build.py b/master/buildbot/test/unit/test_process_build.py index f4eed40c48e..130e399123e 100644 --- a/master/buildbot/test/unit/test_process_build.py +++ b/master/buildbot/test/unit/test_process_build.py @@ -111,10 +111,6 @@ def setExpectations(self, progress): pass -class FakeBuilderStatus: - implements(interfaces.IBuilderStatus) - - class FakeStepFactory(object): """Fake step factory that just returns a fixed step object.""" diff --git a/master/buildbot/test/unit/test_process_builder.py b/master/buildbot/test/unit/test_process_builder.py index c9c6fc7c0e9..97dbc9d727d 100644 --- a/master/buildbot/test/unit/test_process_builder.py +++ b/master/buildbot/test/unit/test_process_builder.py @@ -388,18 +388,18 @@ class TestReconfig(BuilderMixin, unittest.TestCase): @defer.inlineCallbacks def test_reconfig(self): - yield self.makeBuilder(description="Old", category="OldCat") + yield self.makeBuilder(description="Old", tags=["OldTag"]) self.builder_config.description = "New" - self.builder_config.category = "NewCat" + self.builder_config.tags = ["NewTag"] mastercfg = config.MasterConfig() mastercfg.builders = [self.builder_config] yield self.bldr.reconfigService(mastercfg) self.assertEqual( dict(description=self.bldr.builder_status.getDescription(), - category=self.bldr.builder_status.getCategory()), + tags=self.bldr.builder_status.getTags()), dict(description="New", - category="NewCat")) + tags=["NewTag"])) # check that the reconfig grabbed a buliderid self.assertNotEqual(self.bldr._builderid, None) diff --git a/master/buildbot/test/unit/test_process_properties.py b/master/buildbot/test/unit/test_process_properties.py index 59a219f571a..daa4f286ff1 100644 --- a/master/buildbot/test/unit/test_process_properties.py +++ b/master/buildbot/test/unit/test_process_properties.py @@ -87,7 +87,7 @@ def setUp(self): prop_true=True, prop_empty='', ) - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) def doTestSimpleWithProperties(self, fmtstring, expect, **kwargs): d = self.build.render(WithProperties(fmtstring, **kwargs)) @@ -337,7 +337,7 @@ class TestInterpolatePositional(unittest.TestCase): def setUp(self): self.props = Properties() - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) def test_string(self): command = Interpolate("test %s", "one fish") @@ -373,7 +373,7 @@ class TestInterpolateProperties(unittest.TestCase): def setUp(self): self.props = Properties() - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) def test_properties(self): self.props.setProperty("buildername", "winbld", "test") @@ -519,7 +519,7 @@ class TestInterpolateSrc(unittest.TestCase): def setUp(self): self.props = Properties() - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) sa = FakeSource() sb = FakeSource() sc = FakeSource() @@ -642,7 +642,7 @@ class TestInterpolateKwargs(unittest.TestCase): def setUp(self): self.props = Properties() - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) sa = FakeSource() sa.repository = 'cvs://A..' @@ -775,7 +775,7 @@ class TestWithProperties(unittest.TestCase): def setUp(self): self.props = Properties() - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) def testInvalidParams(self): self.assertRaises(ValueError, lambda: @@ -1111,7 +1111,7 @@ class TestProperty(unittest.TestCase): def setUp(self): self.props = Properties() - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) def testIntProperty(self): self.props.setProperty("do-tests", 1, "scheduler") @@ -1245,7 +1245,7 @@ class TestRenderalbeAdapters(unittest.TestCase): def setUp(self): self.props = Properties() - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) def test_list_deferred(self): r1 = DeferredRenderable() @@ -1286,7 +1286,7 @@ class Renderer(unittest.TestCase): def setUp(self): self.props = Properties() - self.build = FakeBuild(self.props) + self.build = FakeBuild(props=self.props) def test_renderer(self): self.props.setProperty("x", "X", "test") diff --git a/master/buildbot/test/unit/test_status_builder_cache.py b/master/buildbot/test/unit/test_status_builder_cache.py index 5ec1885fb3c..6549e42d8a0 100644 --- a/master/buildbot/test/unit/test_status_builder_cache.py +++ b/master/buildbot/test/unit/test_status_builder_cache.py @@ -27,9 +27,9 @@ class TestBuildStatus(unittest.TestCase): # that buildstep.BuildStepStatus is never instantiated here should tell you # that these classes are not well isolated! - def setupBuilder(self, buildername, category=None, description=None): + def setupBuilder(self, buildername, description=None): m = fakemaster.make_master() - b = builder.BuilderStatus(buildername=buildername, category=category, + b = builder.BuilderStatus(buildername=buildername, tags=None, master=m, description=description) # Awkwardly, Status sets this member variable. b.basedir = os.path.abspath(self.mktemp()) diff --git a/master/buildbot/test/unit/test_status_buildstep.py b/master/buildbot/test/unit/test_status_buildstep.py index bf25f1b38f4..4db06521aff 100644 --- a/master/buildbot/test/unit/test_status_buildstep.py +++ b/master/buildbot/test/unit/test_status_buildstep.py @@ -26,12 +26,11 @@ class TestBuildStepStatus(unittest.TestCase): # that buildstep.BuildStepStatus is never instantiated here should tell you # that these classes are not well isolated! - def setupBuilder(self, buildername, category=None, description=None): + def setupBuilder(self, buildername, tags=None, description=None): self.master = fakemaster.make_master() self.master.basedir = '/basedir' - b = builder.BuilderStatus(buildername, self.master, category, description) - b.master = self.master + b = builder.BuilderStatus(buildername, tags, self.master, description) # Ackwardly, Status sets this member variable. b.basedir = os.path.abspath(self.mktemp()) os.mkdir(b.basedir) diff --git a/master/buildbot/test/unit/test_status_mail.py b/master/buildbot/test/unit/test_status_mail.py index 46e45caaa71..448b7936ba1 100644 --- a/master/buildbot/test/unit/test_status_mail.py +++ b/master/buildbot/test/unit/test_status_mail.py @@ -182,7 +182,13 @@ def callback(m): self.assertIn('application/octet-stream', txt) return d + def test_init_enforces_tags_and_builders_are_mutually_exclusive(self): + self.assertRaises(config.ConfigErrors, + MailNotifier, 'from@example.org', + tags=['fast', 'slow'], builders=['a', 'b']) + def test_init_enforces_categories_and_builders_are_mutually_exclusive(self): + # categories are deprecated, but allow them until they're removed. self.assertRaises(config.ConfigErrors, MailNotifier, 'from@example.org', categories=['fast', 'slow'], builders=['a', 'b']) @@ -192,11 +198,21 @@ def test_init_warns_notifier_mode_all_in_iter(self): "mode 'all' is not valid in an iterator and must be passed in as a separate string", lambda: MailNotifier('from@example.org', mode=['all'])) + def test_builderAdded_ignores_unspecified_tags(self): + mn = MailNotifier('from@example.org', tags=['fast']) + + builder = fakemaster.FakeBuilderStatus(self.master) + builder.setTags(['slow']) + + self.assertEqual(None, mn.builderAdded('dummyBuilder', builder)) + self.assert_(builder not in mn.watched) + def test_builderAdded_ignores_unspecified_categories(self): + # categories are deprecated, but leave a test for it until we remove it mn = MailNotifier('from@example.org', categories=['fast']) - builder = Mock() - builder.category = 'slow' + builder = fakemaster.FakeBuilderStatus(self.master) + builder.setTags(['slow']) self.assertEqual(None, mn.builderAdded('dummyBuilder', builder)) self.assert_(builder not in mn.watched) @@ -204,10 +220,11 @@ def test_builderAdded_ignores_unspecified_categories(self): def test_builderAdded_subscribes_to_all_builders_by_default(self): mn = MailNotifier('from@example.org') - builder = Mock() - builder.category = 'slow' - builder2 = Mock() - builder2.category = None + builder = fakemaster.FakeBuilderStatus(self.master) + builder.setTags(['slow']) + + builder2 = fakemaster.FakeBuilderStatus(self.master) + # No tags set. self.assertEqual(mn, mn.builderAdded('dummyBuilder', builder)) self.assertEqual(mn, mn.builderAdded('dummyBuilder2', builder2)) @@ -495,12 +512,24 @@ def check(_): self.assertEqual(self.passedAttrs['revision'], '111222') return d + def test_buildFinished_ignores_unspecified_tags(self): + mn = MailNotifier('from@example.org', tags=['fast']) + + build = FakeBuildStatus(name="build") + build.builder = fakemaster.FakeBuilderStatus(self.master) + build.builder.setTags(['slow']) + build.getBuilder = lambda: build.builder + + self.assertEqual(None, mn.buildFinished('dummyBuilder', build, SUCCESS)) + def test_buildFinished_ignores_unspecified_categories(self): + # categories are deprecated, but test them until they're removed mn = MailNotifier('from@example.org', categories=['fast']) build = FakeBuildStatus(name="build") - build.builder = Mock() - build.builder.category = 'slow' + build.builder = fakemaster.FakeBuilderStatus(self.master) + build.builder.setTags(['slow']) + build.getBuilder = lambda: build.builder self.assertEqual( None, mn.buildFinished('dummyBuilder', build, SUCCESS)) diff --git a/master/buildbot/test/unit/test_status_words.py b/master/buildbot/test/unit/test_status_words.py index c013994f573..9b58e330e97 100644 --- a/master/buildbot/test/unit/test_status_words.py +++ b/master/buildbot/test/unit/test_status_words.py @@ -391,13 +391,12 @@ def get_name(): build = mock.Mock() build.getNumber = lambda: 42 build.getName = get_name - build.category = lambda: "" builder = mock.Mock() builder.getName = get_name build.getBuilder = lambda: builder - self.bot.categories = None + self.bot.tags = None self.contact.notify_for = lambda _: True self.contact.useRevisions = False @@ -643,7 +642,7 @@ def test_constr_args(self): pm_to_nicks=['pm', 'to', 'nicks'], port=1234, allowForce=True, - categories=['categories'], + tags=['tags'], password='pass', notify_events={'successToFailure': 1, }, noticeOnChannel=True, @@ -665,7 +664,7 @@ def test_constr_args(self): self.assertIdentical(p, proto_obj) factory.protocol.assert_called_with( 'nick', 'pass', ['channels'], ['pm', 'to', 'nicks'], - factory.status, ['categories'], {'successToFailure': 1}, + factory.status, ['tags'], {'successToFailure': 1}, noticeOnChannel=True, useColors=False, useRevisions=True, diff --git a/master/buildbot/test/util/steps.py b/master/buildbot/test/util/steps.py index 5862b4e420c..ff468fc9e73 100644 --- a/master/buildbot/test/util/steps.py +++ b/master/buildbot/test/util/steps.py @@ -87,7 +87,7 @@ def setupStep(self, step, slave_version={'*': "99.99"}, slave_env={}, # step.build - b = self.build = fakebuild.FakeBuild() + b = self.build = fakebuild.FakeBuild(master=self.master) b.allFiles = lambda: buildFiles b.master = self.master