handle missing artifacts for bazel targets

Summary: If artifacts are not sent back from a bazel test run, then a failure during the build phase is very likely. This diff marks such jobsteps as failing.

Test Plan: unit tests

Reviewers: anupc

Reviewed By: anupc

Subscribers: changesbot, anupc, kylec

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

changes/buildsteps/default.py

@@ -15,9 +15,10 @@
 from changes.artifacts.xunit import XunitHandler
 from changes.buildsteps.base import BuildStep, LXCConfig
 from changes.config import db
-from changes.constants import Cause, Status, DEFAULT_CPUS, DEFAULT_MEMORY_MB
+from changes.constants import Cause, Result, Status, DEFAULT_CPUS, DEFAULT_MEMORY_MB
 from changes.db.utils import get_or_create
 from changes.jobs.sync_job_step import sync_job_step
+from changes.models.bazeltarget import BazelTarget
 from changes.models.command import CommandType, FutureCommand
 from changes.models.jobphase import JobPhase
 from changes.models.jobstep import JobStep, FutureJobStep
@@ -45,7 +46,7 @@
 # have an explicit list of attributes we'll copy
 JOBSTEP_DATA_COPY_WHITELIST = (
     'release', 'cpus', 'memory', 'weight', 'tests', 'shard_count',
-    'artifact_search_path',
+    'artifact_search_path', 'targets',
 )
 
 
@@ -473,6 +474,18 @@ def create_expanded_jobstep(self, base_jobstep, new_jobphase, future_jobstep, sk
             new_command = future_command.as_command(new_jobstep, index)
             db.session.add(new_command)
 
+        # create bazel targets if necessary
+        if 'targets' in new_jobstep.data:
+            for target_name in new_jobstep.data['targets']:
+                target = BazelTarget(
+                    step=new_jobstep,
+                    job_id=new_jobstep.job_id,
+                    name=target_name,
+                    status=Status.in_progress,
+                    result=Result.unknown,
+                )
+                db.session.add(target)
+
         return new_jobstep
 
     def get_client_adapter(self):

changes/jobs/sync_job_step.py

@@ -18,6 +18,7 @@
 from changes.jobs.sync_artifact import sync_artifact
 from changes.lib.artifact_store_lib import ArtifactStoreClient
 from changes.models.artifact import Artifact
+from changes.models.bazeltarget import BazelTarget
 from changes.models.failurereason import FailureReason
 from changes.models.filecoverage import FileCoverage
 from changes.models.itemstat import ItemStat
@@ -133,6 +134,13 @@ def has_test_failures(step):
     ).exists()).scalar()
 
 
+def has_missing_targets(step):
+    return db.session.query(BazelTarget.query.filter(
+        BazelTarget.step_id == step.id,
+        BazelTarget.status == Status.in_progress,
+    ).exists()).scalar()
+
+
 # Extra time to allot snapshot builds, as they are expected to take longer.
 _SNAPSHOT_TIMEOUT_BONUS_MINUTES = 40
 
@@ -490,6 +498,29 @@ def sync_job_step(step_id):
             'reason': 'test_failures'
         })
         db.session.commit()
+
+    if has_missing_targets(step):
+        if step.result != Result.failed:
+            step.result = Result.failed
+            db.session.add(step)
+
+        BazelTarget.query.filter(
+            BazelTarget.step_id == step.id,
+            BazelTarget.status == Status.in_progress,
+        ).update({
+            'status': Status.finished,
+            'result': Result.aborted,
+        })
+
+        try_create(FailureReason, {
+            'step_id': step.id,
+            'job_id': step.job_id,
+            'build_id': step.job.build_id,
+            'project_id': step.project_id,
+            'reason': 'missing_targets',
+        })
+        db.session.commit()
+
     _report_jobstep_result(step)
 
 

tests/changes/buildsteps/test_default.py

@@ -406,6 +406,93 @@ def test_create_expanded_jobstep(self, get_vcs):
         assert tuple(commands[idx].artifacts) == tuple(DEFAULT_ARTIFACTS)
         assert commands[idx].env == DEFAULT_ENV
 
+    @mock.patch.object(Repository, 'get_vcs')
+    def test_create_expanded_jobstep_with_targets(self, get_vcs):
+        build = self.create_build(self.create_project())
+        job = self.create_job(build)
+        jobphase = self.create_jobphase(job, label='foo')
+        jobstep = self.create_jobstep(jobphase)
+
+        new_jobphase = self.create_jobphase(job, label='bar')
+
+        vcs = mock.Mock(spec=Vcs)
+        vcs.get_buildstep_clone.return_value = 'git clone https://example.com'
+        get_vcs.return_value = vcs
+
+        future_jobstep = FutureJobStep(
+            label='test',
+            commands=[
+                FutureCommand('echo 1'),
+                FutureCommand('echo "foo"\necho "bar"', path='subdir'),
+            ],
+            data={
+                'targets': [
+                    '//a/b:b_test',
+                    '//a:a_test',
+                ],
+            },
+        )
+
+        buildstep = self.get_buildstep(cluster='foo')
+        new_jobstep = buildstep.create_expanded_jobstep(
+            jobstep, new_jobphase, future_jobstep)
+
+        db.session.flush()
+
+        assert new_jobstep.data['expanded'] is True
+        assert new_jobstep.cluster == 'foo'
+
+        commands = new_jobstep.commands
+
+        assert len(commands) == 5
+
+        idx = 0
+        assert commands[idx].script == 'git clone https://example.com'
+        assert commands[idx].cwd == ''
+        assert commands[idx].type == CommandType.infra_setup
+        assert commands[idx].artifacts == []
+        assert commands[idx].env == DEFAULT_ENV
+        assert commands[idx].order == idx
+
+        # skip blacklist removal command
+        idx += 1
+
+        idx += 1
+        assert commands[idx].script == 'echo "hello world 2"'
+        assert commands[idx].cwd == '/usr/test/1'
+        assert commands[idx].type == CommandType.setup
+        assert tuple(commands[idx].artifacts) == ('artifact1.txt', 'artifact2.txt')
+        assert commands[idx].env['PATH'] == '/usr/test/1'
+        for k, v in DEFAULT_ENV.items():
+            if k != 'PATH':
+                assert commands[idx].env[k] == v
+        assert commands[idx].order == idx
+
+        idx += 1
+        assert commands[idx].label == 'echo 1'
+        assert commands[idx].script == 'echo 1'
+        assert commands[idx].order == idx
+        assert commands[idx].cwd == DEFAULT_PATH
+        assert commands[idx].type == CommandType.default
+        assert tuple(commands[idx].artifacts) == tuple(DEFAULT_ARTIFACTS)
+        assert commands[idx].env == DEFAULT_ENV
+
+        idx += 1
+        assert commands[idx].label == 'echo "foo"'
+        assert commands[idx].script == 'echo "foo"\necho "bar"'
+        assert commands[idx].order == idx
+        assert commands[idx].cwd == './source/subdir'
+        assert commands[idx].type == CommandType.default
+        assert tuple(commands[idx].artifacts) == tuple(DEFAULT_ARTIFACTS)
+        assert commands[idx].env == DEFAULT_ENV
+
+        assert len(new_jobstep.targets) == 2
+        assert set(t.name for t in new_jobstep.targets) == set(['//a/b:b_test', '//a:a_test'])
+        for target in new_jobstep.targets:
+            assert target.job == new_jobstep.job
+            assert target.status is Status.in_progress
+            assert target.result is Result.unknown
+
     @mock.patch.object(Repository, 'get_vcs')
     def test_create_replacement_jobstep_expanded(self, get_vcs):
         build = self.create_build(self.create_project())

tests/changes/jobs/test_sync_job_step.py

@@ -584,6 +584,110 @@ def mark_finished(step):
             FailureReason.reason == 'missing_tests',
         )
 
+    @mock.patch('changes.config.queue.delay')
+    @mock.patch.object(HistoricalImmutableStep, 'get_implementation')
+    @responses.activate
+    def test_missing_targets(self, get_implementation, queue_delay):
+        # Simulate test type which doesn't interact with artifacts store.
+        responses.add(responses.GET, SyncJobStepTest.ARTIFACTSTORE_REQUEST_RE, body='', status=404)
+
+        implementation = mock.Mock()
+        get_implementation.return_value = implementation
+
+        def mark_finished(step):
+            step.status = Status.finished
+            step.result = Result.passed
+
+        implementation.update_step.side_effect = mark_finished
+
+        project = self.create_project()
+        build = self.create_build(project=project)
+        job = self.create_job(build=build)
+
+        plan = self.create_plan(project)
+        self.create_step(plan, implementation='test', order=0)
+
+        self.create_job_plan(job, plan)
+
+        phase = self.create_jobphase(
+            job=job,
+            date_started=datetime(2013, 9, 19, 22, 15, 24),
+        )
+        step = self.create_jobstep(phase)
+
+        self.create_target(job, step, status=Status.finished, result=Result.passed)
+        target = self.create_target(job, step, status=Status.in_progress, result=Result.unknown)
+
+        with mock.patch.object(sync_job_step, 'allow_absent_from_db', True):
+            sync_job_step(
+                step_id=step.id.hex,
+                task_id=step.id.hex,
+                parent_task_id=job.id.hex,
+            )
+
+        db.session.expire(step)
+        db.session.expire(target)
+
+        assert target.status is Status.finished
+        assert target.result is Result.aborted
+
+        step = JobStep.query.get(step.id)
+
+        assert step.status == Status.finished
+        assert step.result == Result.failed
+
+        assert db.session.query(FailureReason.query.filter(
+            FailureReason.step_id == step.id,
+            FailureReason.reason == 'missing_targets',
+        ).exists()).scalar()
+
+    @mock.patch('changes.config.queue.delay')
+    @mock.patch.object(HistoricalImmutableStep, 'get_implementation')
+    @responses.activate
+    def test_no_missing_targets(self, get_implementation, queue_delay):
+        # Simulate test type which doesn't interact with artifacts store.
+        responses.add(responses.GET, SyncJobStepTest.ARTIFACTSTORE_REQUEST_RE, body='', status=404)
+
+        implementation = mock.Mock()
+        get_implementation.return_value = implementation
+
+        def mark_finished(step):
+            step.status = Status.finished
+            step.result = Result.passed
+
+        implementation.update_step.side_effect = mark_finished
+
+        project = self.create_project()
+        build = self.create_build(project=project)
+        job = self.create_job(build=build)
+
+        plan = self.create_plan(project)
+        self.create_step(plan, implementation='test', order=0)
+
+        self.create_job_plan(job, plan)
+
+        phase = self.create_jobphase(
+            job=job,
+            date_started=datetime(2013, 9, 19, 22, 15, 24),
+        )
+        step = self.create_jobstep(phase)
+
+        self.create_target(job, step, status=Status.finished, result=Result.passed)
+
+        with mock.patch.object(sync_job_step, 'allow_absent_from_db', True):
+            sync_job_step(
+                step_id=step.id.hex,
+                task_id=step.id.hex,
+                parent_task_id=job.id.hex,
+            )
+
+        db.session.expire(step)
+
+        step = JobStep.query.get(step.id)
+
+        assert step.status == Status.finished
+        assert step.result == Result.passed
+
     @mock.patch('changes.jobs.sync_job_step.has_timed_out')
     @mock.patch.object(HistoricalImmutableStep, 'get_implementation')
     @responses.activate