feat: wire in support for additional types of tests when pushing to Try

bot/code_review_bot/__init__.py

@@ -11,6 +11,9 @@
 
 import requests
 import structlog
+
+# Workaround https://github.com/taskcluster/taskcluster/issues/9172
+import taskcluster.download
 from libmozdata.phabricator import LintResult, UnitResult, UnitResultState
 from taskcluster.helper import TaskclusterConfig
 

bot/code_review_bot/analysis.py

@@ -1,3 +1,4 @@
+import enum
 from functools import cached_property
 
 import structlog
@@ -17,6 +18,10 @@
 )
 
 
+class AnalysisMode(enum.Enum):
+    Lint = 1
+
+
 class PhabricatorRevisionBuild(PhabricatorBuild):
     """
     Convert the bot revision into a libmozevent compatible build

bot/code_review_bot/cli.py

@@ -16,6 +16,7 @@
     UnitResult,
     UnitResultState,
 )
+from taskcluster.download import downloadArtifactToBuf
 
 from code_review_bot import (
     AnalysisException,
@@ -24,6 +25,7 @@
     stats,
     taskcluster,
 )
+from code_review_bot.analysis import AnalysisMode
 from code_review_bot.config import settings
 from code_review_bot.report import get_reporters
 from code_review_bot.revisions import PhabricatorRevision, Revision
@@ -171,13 +173,27 @@ def main():
     # We need Phabricator API to list black-listed users
     settings.load_user_blacklist(taskcluster.secrets["user_blacklist"], phabricator_api)
 
+    # Run workflow according to source
+    w = Workflow(
+        reporters,
+        index_service,
+        queue_service,
+        phabricator_api,
+        taskcluster.secrets["ZERO_COVERAGE_ENABLED"],
+        # Update build status only when phabricator reporting is enabled
+        update_build=phabricator_reporting_enabled,
+        task_failures_ignored=taskcluster.secrets["task_failures_ignored"],
+    )
+
+    revision = None
     # Load unique revision
     try:
         if settings.generic_group_id:
             # Only Phabricator revisions is supported from decision task
             revision = PhabricatorRevision.from_decision_task(
                 queue_service.task(settings.generic_group_id), phabricator_api
             )
+            w.ingest_revision(revision, settings.generic_group_id)
         elif settings.phabricator_build_target:
             # Only Phabricator revisions is supported from build target
             revision = PhabricatorRevision.from_phabricator_trigger(
@@ -186,13 +202,30 @@ def main():
             )
             if revision is None:
                 return 0
+            w.start_analysis(revision, settings.analysis_mode)
         else:
+            decision_task = queue_service.task(settings.try_group_id)
+            rawParams, _ = downloadArtifactToBuf(
+                taskId=settings.try_group_id,
+                name="public/parameters.yml",
+                queueService=queue_service,
+            )
+            parameters = yaml.safe_load(bytes(rawParams))
             revision = Revision.from_try_task(
-                queue_service.task(settings.try_task_id),
-                queue_service.task(settings.try_group_id),
+                decision_task,
+                parameters["phabricator_diff"],
                 phabricator_api,
             )
 
+            analysis_mode = None
+            if parameters["target_tasks_method"] == "codereview":
+                analysis_mode = AnalysisMode.Lint
+
+            if not analysis_mode:
+                raise Exception("Cannot detect analysis mode; cannot proceed!")
+
+            w.run(revision, analysis_mode)
+
     except InvalidTrigger as e:
         logger.info("Early stop analysis due to invalid trigger", error=str(e))
 
@@ -204,40 +237,22 @@ def main():
         # Stop cleanly as we just want to ignore that case, but report on sentry through warning
         return 0
     except Exception as e:
-        # Report revision loading failure on production only
-        # On testing or dev instances, we can use different Phabricator
-        # configuration that do not match all the pulse messages sent
-        if settings.on_production:
-            raise
-
-        else:
-            logger.info(
-                "Failed to load revision",
-                task=settings.try_task_id,
-                error=str(e),
-                phabricator=phabricator["url"],
-            )
-            return 1
+        if not revision:
+            # Report revision loading failure on production only
+            # On testing or dev instances, we can use different Phabricator
+            # configuration that do not match all the pulse messages sent
+            if settings.on_production:
+                raise
+
+            else:
+                logger.info(
+                    "Failed to load revision",
+                    task=settings.try_task_id,
+                    error=str(e),
+                    phabricator=phabricator["url"],
+                )
+                return 1
 
-    # Run workflow according to source
-    w = Workflow(
-        reporters,
-        index_service,
-        queue_service,
-        phabricator_api,
-        taskcluster.secrets["ZERO_COVERAGE_ENABLED"],
-        # Update build status only when phabricator reporting is enabled
-        update_build=phabricator_reporting_enabled,
-        task_failures_ignored=taskcluster.secrets["task_failures_ignored"],
-    )
-    try:
-        if settings.generic_group_id:
-            w.ingest_revision(revision, settings.generic_group_id)
-        elif settings.phabricator_build_target:
-            w.start_analysis(revision)
-        else:
-            w.run(revision)
-    except Exception as e:
         # Log errors to papertrail
         logger.error(
             "Static analysis failure", revision=revision, error=e, exc_info=True

bot/code_review_bot/config.py

@@ -14,6 +14,8 @@
 
 import structlog
 
+from code_review_bot.analysis import AnalysisMode
+
 REPO_MOZILLA_CENTRAL = "https://hg.mozilla.org/mozilla-central"
 REPO_AUTOLAND = "https://hg.mozilla.org/integration/autoland"
 
@@ -47,6 +49,7 @@ def __init__(self):
         self.try_group_id = None
         self.generic_group_id = None
         self.phabricator_build_target = None
+        self.analysis_mode = None
         self.repositories = []
         self.decision_env_prefixes = []
 
@@ -90,8 +93,9 @@ def setup(
         taskcluster_parallel_requests=None,
     ):
         # Detect source from env
-        if "TRY_TASK_ID" in os.environ and "TRY_TASK_GROUP_ID" in os.environ:
+        if "TRY_TASK_ID" in os.environ:
             self.try_task_id = os.environ["TRY_TASK_ID"]
+        if "TRY_TASK_GROUP_ID" in os.environ:
             self.try_group_id = os.environ["TRY_TASK_GROUP_ID"]
         elif "GENERIC_TASK_GROUP_ID" in os.environ:
             self.generic_group_id = os.environ["GENERIC_TASK_GROUP_ID"]
@@ -101,6 +105,10 @@ def setup(
             assert self.phabricator_build_target.startswith(
                 "PHID-HMBT"
             ), f"Not a phabrication build target PHID: {self.phabricator_build_target}"
+            # TODO: remove the default after we can be certain it will always be
+            # present
+            mode = os.environ.get("ANALYSIS_MODE", "Lint")
+            self.analysis_mode = AnalysisMode[mode]
         else:
             raise Exception("Only TRY mode is supported")
 

bot/code_review_bot/revisions/base.py

@@ -275,7 +275,9 @@ def serialize(self):
         raise NotImplementedError
 
     @staticmethod
-    def from_try_task(try_task: dict, decision_task: dict, phabricator: PhabricatorAPI):
+    def from_try_task(
+        decision_task: dict, build_target_phid: str, phabricator: PhabricatorAPI
+    ):
         """
         Load identifiers from Phabricator or Github, using the remote task description
         """
@@ -294,5 +296,5 @@ def from_try_task(try_task: dict, decision_task: dict, phabricator: PhabricatorA
             )
         else:
             return PhabricatorRevision.from_try_task(
-                try_task["extra"]["code-review"], decision_task, phabricator
+                decision_task, build_target_phid, phabricator
             )

bot/code_review_bot/revisions/phabricator.py

@@ -133,15 +133,11 @@ def __str__(self):
 
     @staticmethod
     def from_try_task(
-        code_review: dict, decision_task: dict, phabricator: PhabricatorAPI
+        decision_task: dict, build_target_phid: str, phabricator: PhabricatorAPI
     ):
         """
         Load identifiers from Phabricator, using the remote task description
         """
-        # Load build target phid from the task env
-        build_target_phid = code_review.get("phabricator-diff") or code_review.get(
-            "phabricator-build-target"
-        )
         assert (
             build_target_phid is not None
         ), "Missing phabricator-build-target or phabricator-diff declaration"

bot/code_review_bot/vcs.py

@@ -162,7 +162,7 @@ def apply_build(self, build):
             logger.info("Applying patch", phid=patch.phid, message=message)
             self.apply_patch(patch, message, commit)
 
-    def add_try_commit(self, build):
+    def add_try_commit(self, build, extra_parameters):
         """
         Build and commit the file configuring try
         with try_task_config.json and the code-review workflow parameters in JSON
@@ -171,11 +171,11 @@ def add_try_commit(self, build):
         config = {
             "version": 2,
             "parameters": {
-                "target_tasks_method": "codereview",
-                "optimize_target_tasks": True,
                 "phabricator_diff": build.target_phid,
             },
         }
+        config["parameters"].update(extra_parameters)
+
         diff_phid = build.stack[-1].phid
 
         if build.revision_url:
@@ -213,7 +213,7 @@ def __init__(
     ):
         self.skippable_files = skippable_files
 
-    def run(self, repository, build):
+    def run(self, repository, build, extra_parameters):
         """
         Apply the stack of patches from the build, handling retries
         in case of try server errors
@@ -228,7 +228,7 @@ def run(self, repository, build):
                 )
 
             try:
-                return self.handle_build(repository, build)
+                return self.handle_build(repository, build, extra_parameters)
             except RetryNeeded:
                 build.retries += 1
 
@@ -289,7 +289,7 @@ def format_error(self, error):
         """Extract a readable error log from a VCS exception"""
         raise NotImplementedError
 
-    def handle_build(self, repository, build):
+    def handle_build(self, repository, build, extra_parameters):
         """
         Try to load and apply a diff on local clone
         If successful, push to try and send a treeherder link
@@ -321,7 +321,7 @@ def handle_build(self, repository, build):
                 )
 
             # Configure the try task
-            repository.add_try_commit(build)
+            repository.add_try_commit(build, extra_parameters)
 
             # Then push that stack on try
             tip = repository.push_to_try()

bot/code_review_bot/workflow.py

@@ -16,6 +16,7 @@
 
 from code_review_bot import Level, stats
 from code_review_bot.analysis import (
+    AnalysisMode,
     PhabricatorRevisionBuild,
     publish_analysis_lando,
     publish_analysis_phabricator,
@@ -104,10 +105,14 @@ def __init__(
         # Is local clone already setup ?
         self.clone_available = False
 
-    def run(self, revision):
+    def run(self, revision, analysis_mode: AnalysisMode):
         """
         Find all issues on remote tasks and publish them
         """
+        if analysis_mode == AnalysisMode.Lint:
+            return self._run_lint(revision)
+
+    def _run_lint(self, revision):
         # Index ASAP Taskcluster task for this revision
         self.index(revision, state="started")
 
@@ -273,7 +278,9 @@ def _build_tasks(tasks):
         # Publish issues in the backend
         self.backend_api.publish_issues(issues, revision)
 
-    def start_analysis(self, revision: PhabricatorRevision) -> None:
+    def start_analysis(
+        self, revision: PhabricatorRevision, analysis_mode: AnalysisMode
+    ):
         """
         Apply a patch on a local clone and push to try to trigger a new Code review analysis
         """
@@ -365,9 +372,18 @@ def start_analysis(self, revision: PhabricatorRevision) -> None:
         # We'll clone the required repository
         repository.clone()
 
+        parameters = {}
+        if analysis_mode == AnalysisMode.Lint:
+            parameters.update(
+                {
+                    "target_tasks_method": "codereview",
+                    "optimize_target_tasks": True,
+                }
+            )
+
         # Apply the stack of patches and push to try
         worker = MercurialWorker()
-        output = worker.run(repository, build)
+        output = worker.run(repository, build, parameters)
 
         # Cancel any in-progress tasks from an earlier update
         # This is done after pushing to try to avoid delaying runs of the

bot/tests/conftest.py

@@ -381,14 +381,6 @@ def mock_github(mock_config):
     )
 
 
-@pytest.fixture
-def mock_try_task():
-    """
-    Mock a remote Try task definition
-    """
-    return {"extra": {"code-review": {"phabricator-diff": "PHID-HMBT-test"}}}
-
-
 @pytest.fixture
 def mock_github_decision_task():
     """
@@ -446,14 +438,14 @@ def mock_autoland_task():
 
 
 @pytest.fixture
-def mock_revision(mock_phabricator, mock_try_task, mock_decision_task, mock_config):
+def mock_revision(mock_phabricator, mock_decision_task, mock_config):
     """
     Mock a mercurial revision
     """
     from code_review_bot.revisions import PhabricatorRevision, Revision
 
     with mock_phabricator as api:
-        revision = Revision.from_try_task(mock_try_task, mock_decision_task, api)
+        revision = Revision.from_try_task(mock_decision_task, "PHID-HMBT-test", api)
         assert isinstance(revision, PhabricatorRevision)
         return revision
 
@@ -470,15 +462,13 @@ def mock_revision_autoland(mock_phabricator, mock_autoland_task):
 
 
 @pytest.fixture
-def mock_github_revision(
-    mock_github, mock_try_task, mock_github_decision_task, mock_config
-):
+def mock_github_revision(mock_github, mock_github_decision_task, mock_config):
     """
     Mock a github revision
     """
     from code_review_bot.revisions import GithubRevision, Revision
 
-    revision = Revision.from_try_task(mock_try_task, mock_github_decision_task, None)
+    revision = Revision.from_try_task(mock_github_decision_task, "PHID-HMBT-test", None)
     assert isinstance(revision, GithubRevision)
     return revision