allow additional bazel test flags to be passed in from project config

Summary: This uses the config value `bazel.additional-test-flags`.

Test Plan: unit tests

Reviewers: anupc

Reviewed By: anupc

Subscribers: changesbot, kylec, wwu

Differential Revision: https://tails.corp.dropbox.com/D231482

changes/config.py

@@ -448,6 +448,12 @@ def create_app(_read_config=True, **config):
         '--keep_going',
     ]
 
+    app.config['BAZEL_ADDITIONAL_TEST_FLAGS_WHITELIST_REGEX'] = [
+        r'^--test_env=[A-Za-z0-9=]+',
+        r'^--test_arg=[A-Za-z0-9=]+',
+        r'^--define=[A-Za-z0-9=]+',
+    ]
+
     # Jobsteps go from 'pending_allocation' to 'allocated' once an external scheduler claims them, and
     # once they begin running they're updated to 'in_progress'. If the scheduler somehow fails or drops
     # the task, this value is used to time out the 'allocated' status and revert back to 'pending_allocation'.

changes/models/jobplan.py

@@ -2,9 +2,10 @@
 
 import logging
 import os
+import re
 import uuid
 
-from collections import defaultdict
+from collections import defaultdict, OrderedDict
 from copy import deepcopy
 from datetime import datetime
 from flask import current_app
@@ -222,8 +223,14 @@ def get_build_step_for_job(cls, job_id):
                 logging.error('Project config for project %s requests invalid number of executors: constraint 1 <= %d <= %d', job.project.slug, bazel_max_executors, current_app.config['MAX_EXECUTORS'])
                 return jobplan, None
 
-            bazel_test_flags = current_app.config['BAZEL_MANDATORY_TEST_FLAGS']
-            # TODO(anupc): Allow additional flags to be passed in via project config.
+            additional_test_flags = project_config['bazel.additional-test-flags']
+            for f in additional_test_flags:
+                patterns = current_app.config['BAZEL_ADDITIONAL_TEST_FLAGS_WHITELIST_REGEX']
+                if not any([re.match(p, f) for p in patterns]):
+                    logging.error('Project config for project %s contains invalid additional-test-flags %s. Allowed patterns are %s.', job.project.slug, f, patterns)
+                    return jobplan, None
+            bazel_test_flags = current_app.config['BAZEL_MANDATORY_TEST_FLAGS'] + additional_test_flags
+            bazel_test_flags = list(OrderedDict([(b, None) for b in bazel_test_flags]))  # ensure uniqueness, preserve order
 
             vcs = job.project.repository.get_vcs()
             implementation = LXCBuildStep(

changes/models/project.py

@@ -62,6 +62,7 @@ def get(cls, id):
 
     _default_config = {
         'build.file-blacklist': [],
+        'bazel.additional-test-flags': [],
         'bazel.exclude-tags': ['manual'],  # Ignore tests with manual tag
         'bazel.cpus': DEFAULT_CPUS,
         'bazel.mem': DEFAULT_MEMORY_MB,

tests/changes/models/test_job.py

@@ -32,6 +32,7 @@ def _create_job_and_jobplan(self):
     @mock.patch('changes.models.project.Project.get_config')
     def test_autogenerated_commands(self, get_config):
         get_config.return_value = {
+            'bazel.additional-test-flags': [],
             'bazel.targets': [
                 '//aa/bb/cc/...',
                 '//aa/abc/...',
@@ -111,6 +112,7 @@ def test_autogenerated_commands(self, get_config):
     @mock.patch('changes.models.project.Project.get_config')
     def test_autogenerated_commands_with_exclusions(self, get_config):
         get_config.return_value = {
+            'bazel.additional-test-flags': [],
             'bazel.targets': [
                 '//foo/bar/baz/...',
                 '//bar/bax/...',
@@ -161,9 +163,78 @@ def test_autogenerated_commands_with_exclusions(self, get_config):
         assert implementation.commands[2].type == CommandType.collect_bazel_targets
         assert implementation.commands[2].script == collect_tests_expected
 
+    @mock.patch('changes.models.project.Project.get_config')
+    def test_autogenerated_commands_with_additional_test_flags(self, get_config):
+        get_config.return_value = {
+            'bazel.additional-test-flags': ['--test_env=testing=123', '--test_env=testing=123'],
+            'bazel.targets': [
+                '//foo/bar/baz/...',
+                '//bar/bax/...',
+            ],
+            'bazel.exclude-tags': [],
+            'bazel.cpus': 2,
+            'bazel.mem': 1234,
+            'bazel.max-executors': 3,
+        }
+
+        collect_tests_expected = """#!/bin/bash -eu
+# Clean up any existing apt sources
+sudo rm -rf /etc/apt/sources.list.d >/dev/null 2>&1
+# Overwrite apt sources
+(echo "deb http://example.com/debian distribution component1" | sudo tee /etc/apt/sources.list) >/dev/null 2>&1
+
+# apt-get update, and try again if it fails first time
+(sudo apt-get -y update || sudo apt-get -y update) >/dev/null 2>&1
+sudo apt-get install -y --force-yes bazel python >/dev/null 2>&1
+
+"/var/changes/input/collect-targets" --output-user-root="/bazel/root/path" --target-patterns=//foo/bar/baz/... --target-patterns=//bar/bax/...  --test-flags=--spawn_strategy=sandboxed --test-flags=--genrule_strategy=sandboxed --test-flags=--keep_going --test-flags=--test_env=testing=123 --jobs="4" 2> /dev/null
+""".strip()
+
+        mock_vcs = mock.Mock(spec=Vcs)
+        mock_vcs.get_buildstep_checkout_revision.return_value = 'git checkout master'
+        mock_vcs.get_buildstep_checkout_parent_revision.return_value = 'git checkout master^'
+        mock_vcs.get_buildstep_changed_files.return_value = 'git diff --name-only master^..master'
+        job = self._create_job_and_jobplan()
+        with mock.patch.object(job.project.repository, "get_vcs") as mock_get_vcs:
+            mock_get_vcs.return_value = mock_vcs
+            _, implementation = JobPlan.get_build_step_for_job(job.id)
+
+        assert implementation.max_executors == 3
+
+        assert implementation.artifacts == []
+        assert implementation.artifact_suffix == '.bazel'
+
+        assert implementation.resources['cpus'] == 2
+        assert implementation.resources['mem'] == 1234
+
+        assert len(implementation.commands) == 3
+
+        assert implementation.commands[0].type == CommandType.setup
+        assert implementation.commands[1].type == CommandType.setup
+        assert implementation.commands[2].type == CommandType.collect_bazel_targets
+        assert implementation.commands[2].script == collect_tests_expected
+
+    @mock.patch('changes.models.project.Project.get_config')
+    def test_autogenerated_commands_with_additional_test_flags_invalid(self, get_config):
+        get_config.return_value = {
+            'bazel.additional-test-flags': ['--keep_going'],  # not in whitelist
+            'bazel.targets': [
+                '//foo/bar/baz/...',
+                '//bar/bax/...',
+            ],
+            'bazel.exclude-tags': [],
+            'bazel.cpus': 2,
+            'bazel.mem': 1234,
+            'bazel.max-executors': 3,
+        }
+
+        _, implementation = JobPlan.get_build_step_for_job(self._create_job_and_jobplan().id)
+        assert implementation is None
+
     @mock.patch('changes.models.project.Project.get_config')
     def test_invalid_cpus(self, get_config):
         get_config.return_value = {
+            'bazel.additional-test-flags': [],
             'bazel.targets': [
                 '//aa/bb/cc/...',
                 '//aa/abc/...',
@@ -195,6 +266,7 @@ def test_invalid_cpus(self, get_config):
     @mock.patch('changes.models.project.Project.get_config')
     def test_invalid_mems(self, get_config):
         get_config.return_value = {
+            'bazel.additional-test-flags': [],
             'bazel.targets': [
                 '//aa/bb/cc/...',
                 '//aa/abc/...',
@@ -231,6 +303,7 @@ def test_invalid_mems(self, get_config):
     @mock.patch('changes.models.project.Project.get_config')
     def test_invalid_num_executors(self, get_config):
         get_config.return_value = {
+            'bazel.additional-test-flags': [],
             'bazel.targets': [
                 '//aa/bb/cc/...',
                 '//aa/abc/...',