Merge pull request #7618 from mokibit/remove-deprecated-GerritStatusP…

…ush-arguments

reporters: Remove deprecated usage of GerritStatusPush arguments

master/buildbot/reporters/gerrit.py

@@ -26,7 +26,6 @@
 from twisted.python import log
 from zope.interface import implementer
 
-from buildbot import config
 from buildbot import interfaces
 from buildbot.process.results import EXCEPTION
 from buildbot.process.results import FAILURE
@@ -37,7 +36,6 @@
 from buildbot.reporters import utils
 from buildbot.reporters.base import ReporterBase
 from buildbot.util import bytes2unicode
-from buildbot.warnings import warn_deprecated
 
 # Cache the version that the gerrit server is running for this many seconds
 GERRIT_VERSION_CACHE_TIMEOUT = 600
@@ -331,59 +329,22 @@ def checkConfig(
  self,
  server,
  username,
- reviewCB=DEFAULT_REVIEW,
- startCB=None,
  port=29418,
- reviewArg=None,
- startArg=None,
- summaryCB=DEFAULT_SUMMARY,
- summaryArg=None,
  identity_file=None,
- builders=None,
  notify=None,
- wantSteps=False,
- wantLogs=False,
  generators=None,
  **kwargs,
  ):
- old_arg_names = {
- "reviewCB": reviewCB is not DEFAULT_REVIEW,
- "startCB": startCB is not None,
- "reviewArg": reviewArg is not None,
- "startArg": startArg is not None,
- "summaryCB": summaryCB is not DEFAULT_SUMMARY,
- "summaryArg": summaryArg is not None,
- "builders": builders is not None,
- "wantSteps": wantSteps is not False,
- "wantLogs": wantLogs is not False,
- }
-
- passed_old_arg_names = [k for k, v in old_arg_names.items() if v]
-
- if passed_old_arg_names:
- old_arg_names_msg = ', '.join(passed_old_arg_names)
- if generators is not None:
- config.error(
- "can't specify generators and deprecated GerritStatusPush "
- f"arguments ({old_arg_names_msg}) at the same time"
- )
- warn_deprecated(
- "3.11.0",
- f"The arguments {old_arg_names_msg} passed to {self.__class__.__name__} "
- "have been deprecated. Use generators instead",
- )
-
  if generators is None:
- generators = self._create_generators_from_old_args(
- reviewCB,
- startCB,
- reviewArg,
- startArg,
- summaryCB,
- summaryArg,
- builders,
- wantSteps,
- wantLogs,
+ generators = []
+ generators.append(
+ GerritBuildSetStatusGenerator(
+ callback=defaultSummaryCB,
+ callback_arg=None,
+ builders=None,
+ want_steps=False,
+ want_logs=False,
+ )
  )
 
  super().checkConfig(generators=generators, **kwargs)
@@ -392,18 +353,9 @@ def reconfigService(
  self,
  server,
  username,
- reviewCB=DEFAULT_REVIEW,
- startCB=None,
  port=29418,
- reviewArg=None,
- startArg=None,
- summaryCB=DEFAULT_SUMMARY,
- summaryArg=None,
  identity_file=None,
- builders=None,
  notify=None,
- wantSteps=False,
- wantLogs=False,
  generators=None,
  **kwargs,
  ):
@@ -416,80 +368,18 @@ def reconfigService(
  self._gerrit_notify = notify
 
  if generators is None:
- generators = self._create_generators_from_old_args(
- reviewCB,
- startCB,
- reviewArg,
- startArg,
- summaryCB,
- summaryArg,
- builders,
- wantSteps,
- wantLogs,
- )
-
- super().reconfigService(generators=generators, **kwargs)
-
- def _create_generators_from_old_args(
- self,
- reviewCB,
- startCB,
- reviewArg,
- startArg,
- summaryCB,
- summaryArg,
- builders,
- wantSteps,
- wantLogs,
- ):
- # If neither reviewCB nor summaryCB were specified, default to sending
- # out "summary" reviews. But if we were given a reviewCB and only a
- # reviewCB, disable the "summary" reviews, so we don't send out both
- # by default.
- if reviewCB is DEFAULT_REVIEW and summaryCB is DEFAULT_SUMMARY:
- reviewCB = None
- summaryCB = defaultSummaryCB
- if reviewCB is DEFAULT_REVIEW:
- reviewCB = None
- if summaryCB is DEFAULT_SUMMARY:
- summaryCB = None
-
- generators = []
-
- if startCB is not None:
- generators.append(
- GerritBuildStartStatusGenerator(
- callback=startCB,
- callback_arg=startArg,
- builders=builders,
- want_steps=wantSteps,
- want_logs=wantLogs,
- )
- )
-
- if reviewCB is not None:
- generators.append(
- GerritBuildEndStatusGenerator(
- callback=reviewCB,
- callback_arg=reviewArg,
- builders=builders,
- want_steps=wantSteps,
- want_logs=wantLogs,
- )
- )
-
- if summaryCB is not None:
+ generators = []
  generators.append(
  GerritBuildSetStatusGenerator(
- callback=summaryCB,
- callback_arg=summaryArg,
- builders=builders,
- want_steps=wantSteps,
- want_logs=wantLogs,
+ callback=defaultSummaryCB,
+ callback_arg=None,
+ builders=None,
+ want_steps=False,
+ want_logs=False,
  )
  )
 
- return generators
+ super().reconfigService(generators=generators, **kwargs)
 
  def _gerritCmd(self, *args):
  """Construct a command as a list of strings suitable for

master/buildbot/test/unit/reporters/test_gerrit.py

@@ -33,6 +33,9 @@
 from buildbot.reporters.generators.buildset import BuildSetStatusGenerator
 from buildbot.reporters.gerrit import GERRIT_LABEL_REVIEWED
 from buildbot.reporters.gerrit import GERRIT_LABEL_VERIFIED
+from buildbot.reporters.gerrit import GerritBuildEndStatusGenerator
+from buildbot.reporters.gerrit import GerritBuildSetStatusGenerator
+from buildbot.reporters.gerrit import GerritBuildStartStatusGenerator
 from buildbot.reporters.gerrit import GerritStatusPush
 from buildbot.reporters.gerrit import defaultReviewCB
 from buildbot.reporters.gerrit import defaultSummaryCB
@@ -44,8 +47,6 @@
 from buildbot.test.fake import fakemaster
 from buildbot.test.reactor import TestReactorMixin
 from buildbot.test.util.reporter import ReporterTestMixin
-from buildbot.test.util.warnings import assertProducesWarnings
-from buildbot.warnings import DeprecatedApiWarning
 
 warnings.filterwarnings('error', message='.*Gerrit status')
 
@@ -187,8 +188,17 @@ def run_fake_summary_build(self, gsp, buildResults, finalResult, resultText, exp
 
  @defer.inlineCallbacks
  def check_summary_build_deferred(self, buildResults, finalResult, resultText, verifiedScore):
- with assertProducesWarnings(DeprecatedApiWarning, message_pattern="Use generators instead"):
- gsp = yield self.setupGerritStatusPush(summaryCB=sampleSummaryCBDeferred)
+ gsp = yield self.setupGerritStatusPush(
+ generators=[
+ GerritBuildSetStatusGenerator(
+ callback=sampleSummaryCBDeferred,
+ callback_arg=None,
+ builders=None,
+ want_steps=False,
+ want_logs=False,
+ )
+ ]
+ )
 
  msg = yield self.run_fake_summary_build(gsp, buildResults, finalResult, resultText)
 
@@ -201,8 +211,17 @@ def check_summary_build_deferred(self, buildResults, finalResult, resultText, ve
 
  @defer.inlineCallbacks
  def check_summary_build(self, buildResults, finalResult, resultText, verifiedScore):
- with assertProducesWarnings(DeprecatedApiWarning, message_pattern="Use generators instead"):
- gsp = yield self.setupGerritStatusPush(summaryCB=sampleSummaryCB)
+ gsp = yield self.setupGerritStatusPush(
+ generators=[
+ GerritBuildSetStatusGenerator(
+ callback=sampleSummaryCB,
+ callback_arg=None,
+ builders=None,
+ want_steps=False,
+ want_logs=False,
+ )
+ ]
+ )
 
  msg = yield self.run_fake_summary_build(gsp, buildResults, finalResult, resultText)
 
@@ -290,9 +309,17 @@ def test_buildsetComplete_mixed_sends_summary_review(self):
  ])
  @defer.inlineCallbacks
  def test_buildset_complete_filtered_builder(self, name, builders, should_call):
- with assertProducesWarnings(DeprecatedApiWarning, message_pattern="Use generators instead"):
- gsp = yield self.setupGerritStatusPush(summaryCB=sampleSummaryCB, builders=builders)
-
+ gsp = yield self.setupGerritStatusPush(
+ generators=[
+ GerritBuildSetStatusGenerator(
+ callback=sampleSummaryCB,
+ callback_arg=None,
+ builders=builders,
+ want_steps=False,
+ want_logs=False,
+ )
+ ]
+ )
  yield self.run_fake_summary_build(gsp, [FAILURE, FAILURE], FAILURE, ["failed", "failed"])
 
  self.assertEqual(gsp.send_code_review.called, should_call)
@@ -323,36 +350,27 @@ def run_fake_single_build(self, gsp, buildResult, expWarning=False):
 
  return str({'name': 'Builder0', 'result': buildResult})
 
- # same goes for check_single_build and check_single_build_legacy
+ # same goes for check_single_build
  @defer.inlineCallbacks
  def check_single_build(self, buildResult, verifiedScore):
- with assertProducesWarnings(DeprecatedApiWarning, message_pattern="Use generators instead"):
- gsp = yield self.setupGerritStatusPush(reviewCB=sampleReviewCB, startCB=sampleStartCB)
-
- msg = yield self.run_fake_single_build(gsp, buildResult)
- calls = [
- call(
- self.reporter_test_project,
- self.reporter_test_revision,
- str({'name': self.reporter_test_builder_name}),
- {GERRIT_LABEL_REVIEWED: 0},
- ),
- call(
- self.reporter_test_project,
- self.reporter_test_revision,
- msg,
- {GERRIT_LABEL_VERIFIED: verifiedScore},
- ),
- ]
- gsp.send_code_review.assert_has_calls(calls)
-
- # same goes for check_single_build and check_single_build_legacy
- @defer.inlineCallbacks
- def check_single_build_deferred(self, buildResult, verifiedScore):
- with assertProducesWarnings(DeprecatedApiWarning, message_pattern="Use generators instead"):
- gsp = yield self.setupGerritStatusPush(
- reviewCB=sampleReviewCBDeferred, startCB=sampleStartCBDeferred
- )
+ gsp = yield self.setupGerritStatusPush(
+ generators=[
+ GerritBuildEndStatusGenerator(
+ callback=sampleReviewCB,
+ callback_arg=None,
+ builders=None,
+ want_steps=False,
+ want_logs=False,
+ ),
+ GerritBuildStartStatusGenerator(
+ callback=sampleStartCB,
+ callback_arg=None,
+ builders=None,
+ want_steps=False,
+ want_logs=False,
+ ),
+ ]
+ )
 
  msg = yield self.run_fake_single_build(gsp, buildResult)
  calls = [
@@ -377,17 +395,31 @@ def test_buildComplete_success_sends_review(self):
  def test_buildComplete_failure_sends_review(self):
  return self.check_single_build(FAILURE, -1)
 
- # same goes for check_single_build and check_single_build_legacy
+ # same goes for check_single_build
  @parameterized.expand([
  ("matched", ["Builder0"], True),
  ("not_matched", ["foo"], False),
  ])
  @defer.inlineCallbacks
  def test_single_build_filtered(self, name, builders, should_call):
- with assertProducesWarnings(DeprecatedApiWarning, message_pattern="Use generators instead"):
- gsp = yield self.setupGerritStatusPush(
- reviewCB=sampleReviewCB, startCB=sampleStartCB, builders=builders
- )
+ gsp = yield self.setupGerritStatusPush(
+ generators=[
+ GerritBuildEndStatusGenerator(
+ callback=sampleReviewCB,
+ callback_arg=None,
+ builders=builders,
+ want_steps=False,
+ want_logs=False,
+ ),
+ GerritBuildStartStatusGenerator(
+ callback=sampleStartCB,
+ callback_arg=None,
+ builders=builders,
+ want_steps=False,
+ want_logs=False,
+ ),
+ ]
+ )
 
  yield self.run_fake_single_build(gsp, SUCCESS)
  self.assertEqual(gsp.send_code_review.called, should_call)