Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(134)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: appengine/findit/waterfall/test/identify_try_job_culprit_pipeline_test.py

Issue 2230103002: [Findit] Pipeline change to save suspected cls to data store. (Closed) Base URL: https://chromium.googlesource.com/infra/infra.git@0808-resubmit-suspected_cl_model
Patch Set: Self review. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« appengine/findit/waterfall/suspected_cl_util.py ('K') | « appengine/findit/waterfall/test/build_failure_analysis_test.py ('k') | appengine/findit/waterfall/test/suspected_cl_util_test.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: appengine/findit/waterfall/test/identify_try_job_culprit_pipeline_test.py
diff --git a/appengine/findit/waterfall/test/identify_try_job_culprit_pipeline_test.py b/appengine/findit/waterfall/test/identify_try_job_culprit_pipeline_test.py
index 3a87c8d2ddd19dcbbf33ad553475d7c332464fd1..963e4620db3ca7b95a1db368fbdfe3445dd98055 100644
--- a/appengine/findit/waterfall/test/identify_try_job_culprit_pipeline_test.py
+++ b/appengine/findit/waterfall/test/identify_try_job_culprit_pipeline_test.py
@@ -6,9 +6,11 @@ from testing_utils import testing
from common.git_repository import GitRepository
from common.waterfall import failure_type
+from model import analysis_approach_type
from model import analysis_status
from model import result_status
from model.wf_analysis import WfAnalysis
+from model.wf_suspected_cl import WfSuspectedCL
from model.wf_try_job import WfTryJob
from model.wf_try_job_data import WfTryJobData
from waterfall import identify_try_job_culprit_pipeline
@@ -26,8 +28,8 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
self.code_review_url = code_review_url
mock_change_logs = {}
- mock_change_logs['rev1'] = MockedChangeLog('1', 'url_1')
- mock_change_logs['rev2'] = MockedChangeLog('2', 'url_2')
+ mock_change_logs['rev1'] = MockedChangeLog(1, 'url_1')
+ mock_change_logs['rev2'] = MockedChangeLog(2, 'url_2')
return mock_change_logs.get(revision)
def setUp(self):
@@ -101,7 +103,7 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'culprit': {
'compile': {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
@@ -124,7 +126,7 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'culprit': {
'compile': {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
@@ -148,7 +150,7 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'culprit': {
'compile': {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
@@ -171,7 +173,7 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'culprit': {
'compile': {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
@@ -206,7 +208,7 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'culprit': {
'compile': {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
@@ -248,14 +250,14 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
def testGetSuspectedCLsForCompileTryJob(self):
heuristic_suspected_cl = {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
compile_suspected_cl = {
'revision': 'rev2',
- 'commit_position': '2',
+ 'commit_position': 2,
'url': 'url_2',
'repo_name': 'chromium'
}
@@ -264,20 +266,19 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
analysis.suspected_cls = [heuristic_suspected_cl]
analysis.put()
- result = {
- 'culprit': {
- 'compile': compile_suspected_cl
- }
+ try_job_suspected_cls = {
+ 'rev2': compile_suspected_cl
}
self.assertEqual(
- identify_try_job_culprit_pipeline._GetSuspectedCLs(analysis, result),
+ identify_try_job_culprit_pipeline._GetSuspectedCLs(
+ analysis, try_job_suspected_cls),
[heuristic_suspected_cl, compile_suspected_cl])
def testGetSuspectedCLsForTestTryJobAndHeuristicResultsSame(self):
suspected_cl = {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
@@ -286,71 +287,52 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
analysis.suspected_cls = [suspected_cl]
analysis.put()
- result = {
- 'culprit': {
- 'compile': suspected_cl
- }
+ try_job_suspected_cls = {
+ 'rev1': suspected_cl
}
- self.assertEqual(
- identify_try_job_culprit_pipeline._GetSuspectedCLs(analysis, result),
- [suspected_cl])
+ updated_cls = identify_try_job_culprit_pipeline._GetSuspectedCLs(
+ analysis, try_job_suspected_cls)
+ self.assertEqual(updated_cls, [suspected_cl])
def testGetSuspectedCLsForTestTryJob(self):
suspected_cl1 = {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
suspected_cl2 = {
'revision': 'rev2',
- 'commit_position': '2',
+ 'commit_position': 2,
'url': 'url_2',
'repo_name': 'chromium'
}
suspected_cl3 = {
'revision': 'rev3',
- 'commit_position': '3',
+ 'commit_position': 3,
'url': 'url_3',
'repo_name': 'chromium'
}
analysis = WfAnalysis.Create('m', 'b', 1)
- analysis.suspected_cls = []
+ analysis.suspected_cls = [suspected_cl3]
analysis.put()
- result = {
- 'culprit': {
- 'a_test': {
- 'tests': {
- 'a_test1': suspected_cl1,
- 'a_test2': suspected_cl1
- }
- },
- 'b_test': {
- 'tests': {
- 'b_test1': suspected_cl2
- }
- },
- 'c_test': {
- 'revision': 'rev3',
- 'commit_position': '3',
- 'url': 'url_3',
- 'repo_name': 'chromium',
- 'tests': {}
- }
- }
+ try_job_suspected_cls = {
+ 'rev1': suspected_cl1,
+ 'rev2': suspected_cl2
}
+ result = identify_try_job_culprit_pipeline._GetSuspectedCLs(
+ analysis, try_job_suspected_cls)
self.assertEqual(
- identify_try_job_culprit_pipeline._GetSuspectedCLs(analysis, result),
- [suspected_cl3, suspected_cl2, suspected_cl1])
+ result, [suspected_cl3, suspected_cl1, suspected_cl2])
def testGetSuspectedCLsForTestTryJobWithHeuristicResult(self):
suspected_cl = {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
@@ -359,19 +341,8 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
analysis.suspected_cls = [suspected_cl]
analysis.put()
- result = {
- 'culprit': {
- 'a_test': {
- 'revision': 'rev1',
- 'commit_position': '1',
- 'url': 'url_1',
- 'repo_name': 'chromium',
- 'tests': {}
- }
- }
- }
self.assertEqual(
- identify_try_job_culprit_pipeline._GetSuspectedCLs(analysis, result),
+ identify_try_job_culprit_pipeline._GetSuspectedCLs(analysis, {}),
[suspected_cl])
def testIdentifyCulpritForCompileTryJobNoCulprit(self):
@@ -442,7 +413,7 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
expected_culprit = 'rev2'
expected_suspected_cl = {
'revision': 'rev2',
- 'commit_position': '2',
+ 'commit_position': 2,
'url': 'url_2',
'repo_name': 'chromium'
}
@@ -543,7 +514,7 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
suspected_cl = {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
@@ -694,10 +665,6 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'status': 'failed',
'valid': True,
'failures': ['b_test1']
- },
- 'c_test': {
- 'status': 'passed',
- 'valid': True
}
},
'rev2': {
@@ -709,11 +676,6 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'b_test': {
'status': 'passed',
'valid': True
- },
- 'c_test': {
- 'status': 'failed',
- 'valid': True,
- 'failures': []
}
}
}
@@ -737,13 +699,13 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
a_test1_suspected_cl = {
'revision': 'rev1',
- 'commit_position': '1',
+ 'commit_position': 1,
'url': 'url_1',
'repo_name': 'chromium'
}
a_test2_suspected_cl = {
'revision': 'rev2',
- 'commit_position': '2',
+ 'commit_position': 2,
'url': 'url_2',
'repo_name': 'chromium'
}
@@ -763,10 +725,6 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'status': 'failed',
'valid': True,
'failures': ['b_test1']
- },
- 'c_test': {
- 'status': 'passed',
- 'valid': True
}
},
'rev2': {
@@ -778,11 +736,6 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'b_test': {
'status': 'passed',
'valid': True
- },
- 'c_test': {
- 'status': 'failed',
- 'valid': True,
- 'failures': []
}
}
}
@@ -800,13 +753,6 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
'tests': {
'b_test1': b_test1_suspected_cl
}
- },
- 'c_test': {
- 'revision': 'rev2',
- 'commit_position': '2',
- 'url': 'url_2',
- 'repo_name': 'chromium',
- 'tests': {}
}
}
}
@@ -826,37 +772,54 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
},
'b_test': {
'b_test1': 'rev1',
- },
- 'c_test': 'rev2'
}
+ }
self.assertEqual(expected_culprit_data, try_job_data.culprits)
self.assertEqual(analysis.result_status,
result_status.FOUND_UNTRIAGED)
self.assertEqual(analysis.suspected_cls,
- [a_test2_suspected_cl, a_test1_suspected_cl])
+ [a_test1_suspected_cl, a_test2_suspected_cl])
def testAnalysisIsUpdatedOnlyIfStatusOrSuspectedCLsChanged(self):
master_name = 'm'
builder_name = 'b'
build_number = 1
try_job_id = '1'
+ repo_name = 'chromium'
+ revision = 'rev1'
+ commit_position = 1
- suspected_cl = {
- 'revision': 'rev1',
- 'commit_position': '1',
+ heuristic_suspected_cl = {
+ 'revision': revision,
+ 'commit_position': commit_position,
'url': 'url_1',
- 'repo_name': 'chromium'
+ 'repo_name': repo_name
}
analysis = WfAnalysis.Create(master_name, builder_name, build_number)
- analysis.suspected_cls = [suspected_cl]
+ analysis.suspected_cls = [heuristic_suspected_cl]
analysis.result_status = result_status.FOUND_UNTRIAGED
analysis.put()
version = analysis.version
+
+ build_key = '%s/%s/%d' % (master_name, builder_name, build_number)
+ suspected_cl = WfSuspectedCL.Create(repo_name, revision, commit_position)
+ suspected_cl.approach = analysis_approach_type.HEURISTIC
+ suspected_cl.builds = {
+ build_key: {
+ 'approach': analysis_approach_type.HEURISTIC,
+ 'failure_type':failure_type.COMPILE,
+ 'failures': {'compile':[]},
+ 'status': None,
+ 'top_score': 4
+ }
+ }
+ suspected_cl.put()
+
compile_result = {
'report': {
'result': {
- 'rev1': 'failed',
+ revision: 'failed',
},
},
}
@@ -869,7 +832,7 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
try_job.compile_results = [{
'report': {
'result': {
- 'rev1': 'failed',
+ revision: 'failed',
},
},
'try_job_id': try_job_id,
@@ -878,14 +841,27 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
try_job.put()
pipeline = IdentifyTryJobCulpritPipeline()
- pipeline.run(master_name, builder_name, build_number, ['rev1'],
+ pipeline.run(master_name, builder_name, build_number, [revision],
failure_type.COMPILE, '1', compile_result)
self.assertEqual(analysis.result_status,
result_status.FOUND_UNTRIAGED)
- self.assertEqual(analysis.suspected_cls, [suspected_cl])
+ self.assertEqual(analysis.suspected_cls, [heuristic_suspected_cl])
self.assertEqual(version, analysis.version) # No update to analysis.
+ expected_builds = {
+ build_key: {
+ 'approach': analysis_approach_type.BOTH,
+ 'failure_type':failure_type.COMPILE,
+ 'failures': {'compile':[]},
+ 'status': None,
+ 'top_score': 4
+ }
+ }
+ suspected_cl = WfSuspectedCL.Get(repo_name, revision)
+ self.assertEqual(analysis_approach_type.BOTH, suspected_cl.approach)
+ self.assertEqual(expected_builds, suspected_cl.builds)
+
def testFindCulpritForEachTestFailureRevisionNotRun(self):
blame_list = ['rev1']
result = {
@@ -946,7 +922,6 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
self.assertEqual(try_job.test_results, [])
self.assertEqual(try_job.status, analysis_status.COMPLETED)
-
def testNotifyCulprits(self):
instances = []
class Mocked_SendNotificationForCulpritPipeline(object):
@@ -1069,4 +1044,8 @@ class IdentifyTryJobCulpritPipelineTest(testing.AppengineTestCase):
master_name, builder_name, build_number, None,
heuristic_cls, compile_suspected_cl)
self.assertEqual(1, len(instances))
- self.assertTrue(instances[0].started)
+ self.assertTrue(instances[0].started)
+
+ def testGetTestFailureCausedByCLResultNone(self):
+ self.assertIsNone(
+ identify_try_job_culprit_pipeline._GetTestFailureCausedByCL(None))
« appengine/findit/waterfall/suspected_cl_util.py ('K') | « appengine/findit/waterfall/test/build_failure_analysis_test.py ('k') | appengine/findit/waterfall/test/suspected_cl_util_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698