Skip to content

Commit f0a6558

Browse files
La0La0
authored
bot: Support covdir in Phabricator notifier (#39)
1 parent 832871c commit f0a6558

File tree

4 files changed

+86
-29
lines changed

4 files changed

+86
-29
lines changed

‎bot/code_coverage_bot/phabricator.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,19 @@ def __init__(self, repo_dir, revision):
2828
self.revision = revision
2929

3030
def _find_coverage(self, report, path):
31-
return next((sf['coverage'] for sf in report['source_files'] if sf['name'] == path), None)
31+
'''
32+
Find coverage value in a covdir report
33+
'''
34+
assert isinstance(report, dict)
35+
36+
parts = path.split('/')
37+
for part in filter(None, parts):
38+
if part not in report['children']:
39+
logger.warn('Path {} not found in report'.format(path))
40+
return
41+
report = report['children'][part]
42+
43+
return report['coverage']
3244

3345
def _build_coverage_map(self, annotate, coverage_record):
3446
# We can't use plain line numbers to map coverage data from the build changeset to the

‎bot/tests/conftest.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,3 +331,40 @@ def mock_taskcluster():
331331
taskcluster_config.options = {
332332
'rootUrl': 'http://taskcluster.test',
333333
}
334+
335+
336+
def covdir_report(codecov):
337+
'''
338+
Convert source files to covdir format
339+
'''
340+
assert isinstance(codecov, dict)
341+
assert 'source_files' in codecov
342+
343+
out = {}
344+
for cov in codecov['source_files']:
345+
assert '/' not in cov['name']
346+
coverage = cov['coverage']
347+
total = len(coverage)
348+
covered = sum(l is not None and l > 0 for l in coverage)
349+
out[cov['name']] = {
350+
'children': {},
351+
'name': cov['name'],
352+
'coverage': coverage,
353+
'coveragePercent': 100.0 * covered / total,
354+
'linesCovered': covered,
355+
'linesMissed': total - covered,
356+
'linesTotal': total,
357+
}
358+
359+
# Covdir has a root level
360+
def _sum(name):
361+
return sum(c[name] for c in out.values())
362+
return {
363+
'children': out,
364+
'name': 'src',
365+
'coverage': [],
366+
'coveragePercent': _sum('coveragePercent') / len(out) if out else 0,
367+
'linesCovered': _sum('linesCovered'),
368+
'linesMissed': _sum('linesMissed'),
369+
'linesTotal': _sum('linesTotal'),
370+
}

‎bot/tests/test_notifier.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from code_coverage_bot.notifier import notify_email
55
from code_coverage_bot.phabricator import PhabricatorUploader
6+
from conftest import covdir_report
67
from mercurial import add_file
78
from mercurial import changesets
89
from mercurial import commit
@@ -28,12 +29,12 @@ def test_notification(mock_secrets, mock_taskcluster, mock_phabricator, fake_hg_
2829
assert stack[0]['desc'] == "Commit [(b'A', b'file')]Differential Revision: https://phabricator.services.mozilla.com/D1"
2930
assert stack[1]['desc'] == "Commit [(b'M', b'file')]Differential Revision: https://phabricator.services.mozilla.com/D2"
3031

31-
report = {
32+
report = covdir_report({
3233
'source_files': [{
3334
'name': 'file',
3435
'coverage': [None, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0],
3536
}]
36-
}
37+
})
3738
phab = PhabricatorUploader(local, revision)
3839
changesets_coverage = phab.generate(report, stack)
3940

‎bot/tests/test_phabricator.py

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import responses
88

99
from code_coverage_bot.phabricator import PhabricatorUploader
10+
from conftest import covdir_report
1011
from mercurial import add_file
1112
from mercurial import changesets
1213
from mercurial import commit
@@ -25,12 +26,13 @@ def test_simple(mock_secrets, mock_phabricator, fake_hg_repo):
2526
copy_pushlog_database(remote, local)
2627

2728
phabricator = PhabricatorUploader(local, revision)
28-
results = phabricator.generate({
29+
report = covdir_report({
2930
'source_files': [{
3031
'name': 'file',
3132
'coverage': [None, 0, 1, 1, 1, 1, 0],
3233
}]
33-
}, changesets(local, revision))
34+
})
35+
results = phabricator.generate(report, changesets(local, revision))
3436

3537
assert results == {
3638
1: {
@@ -42,12 +44,7 @@ def test_simple(mock_secrets, mock_phabricator, fake_hg_repo):
4244
}
4345
}
4446

45-
phabricator.upload({
46-
'source_files': [{
47-
'name': 'file',
48-
'coverage': [None, 0, 1, 1, 1, 1, 0],
49-
}]
50-
}, changesets(local, revision))
47+
phabricator.upload(report, changesets(local, revision))
5148

5249
assert len(responses.calls) >= 3
5350

@@ -97,9 +94,10 @@ def test_file_with_no_coverage(mock_secrets, fake_hg_repo):
9794
copy_pushlog_database(remote, local)
9895

9996
phabricator = PhabricatorUploader(local, revision)
100-
results = phabricator.generate({
97+
report = covdir_report({
10198
'source_files': []
102-
}, changesets(local, revision))
99+
})
100+
results = phabricator.generate(report, changesets(local, revision))
103101

104102
assert results == {
105103
1: {}
@@ -118,12 +116,13 @@ def test_one_commit_without_differential(mock_secrets, fake_hg_repo):
118116
copy_pushlog_database(remote, local)
119117

120118
phabricator = PhabricatorUploader(local, revision)
121-
results = phabricator.generate({
119+
report = covdir_report({
122120
'source_files': [{
123121
'name': 'file_one_commit',
124122
'coverage': [None, 0, 1, 1, 1, 1, 0],
125123
}]
126-
}, changesets(local, revision))
124+
})
125+
results = phabricator.generate(report, changesets(local, revision))
127126

128127
assert results == {}
129128

@@ -144,7 +143,7 @@ def test_two_commits_two_files(mock_secrets, fake_hg_repo):
144143
copy_pushlog_database(remote, local)
145144

146145
phabricator = PhabricatorUploader(local, revision)
147-
results = phabricator.generate({
146+
report = covdir_report({
148147
'source_files': [{
149148
'name': 'file1_commit1',
150149
'coverage': [None, 0, 1, 1, 1, 1, 0],
@@ -155,7 +154,8 @@ def test_two_commits_two_files(mock_secrets, fake_hg_repo):
155154
'name': 'file3_commit2',
156155
'coverage': [1, 1, 0, 1, None],
157156
}]
158-
}, changesets(local, revision))
157+
})
158+
results = phabricator.generate(report, changesets(local, revision))
159159

160160
assert results == {
161161
1: {
@@ -196,12 +196,13 @@ def test_changesets_overwriting(mock_secrets, fake_hg_repo):
196196
copy_pushlog_database(remote, local)
197197

198198
phabricator = PhabricatorUploader(local, revision)
199-
results = phabricator.generate({
199+
report = covdir_report({
200200
'source_files': [{
201201
'name': 'file',
202202
'coverage': [None, 0, 1, 1, 1, 1, 0],
203203
}]
204-
}, changesets(local, revision))
204+
})
205+
results = phabricator.generate(report, changesets(local, revision))
205206

206207
assert results == {
207208
1: {
@@ -236,12 +237,13 @@ def test_changesets_displacing(mock_secrets, fake_hg_repo):
236237
copy_pushlog_database(remote, local)
237238

238239
phabricator = PhabricatorUploader(local, revision)
239-
results = phabricator.generate({
240+
report = covdir_report({
240241
'source_files': [{
241242
'name': 'file',
242243
'coverage': [0, 1, None, 0, 1, 1, 1, 1, 0, 1, 0],
243244
}]
244-
}, changesets(local, revision))
245+
})
246+
results = phabricator.generate(report, changesets(local, revision))
245247

246248
assert results == {
247249
1: {
@@ -276,12 +278,13 @@ def test_changesets_reducing_size(mock_secrets, fake_hg_repo):
276278
copy_pushlog_database(remote, local)
277279

278280
phabricator = PhabricatorUploader(local, revision)
279-
results = phabricator.generate({
281+
report = covdir_report({
280282
'source_files': [{
281283
'name': 'file',
282284
'coverage': [None, 0, 1, 1, 1],
283285
}]
284-
}, changesets(local, revision))
286+
})
287+
results = phabricator.generate(report, changesets(local, revision))
285288

286289
assert results == {
287290
1: {
@@ -316,12 +319,14 @@ def test_changesets_overwriting_one_commit_without_differential(mock_secrets, fa
316319
copy_pushlog_database(remote, local)
317320

318321
phabricator = PhabricatorUploader(local, revision)
319-
results = phabricator.generate({
322+
323+
report = covdir_report({
320324
'source_files': [{
321325
'name': 'file',
322326
'coverage': [None, 0, 1, 1, 1, 1, 0],
323327
}]
324-
}, changesets(local, revision))
328+
})
329+
results = phabricator.generate(report, changesets(local, revision))
325330

326331
assert results == {
327332
1: {
@@ -349,9 +354,10 @@ def test_removed_file(mock_secrets, fake_hg_repo):
349354
copy_pushlog_database(remote, local)
350355

351356
phabricator = PhabricatorUploader(local, revision)
352-
results = phabricator.generate({
357+
report = covdir_report({
353358
'source_files': []
354-
}, changesets(local, revision))
359+
})
360+
results = phabricator.generate(report, changesets(local, revision))
355361

356362
assert results == {
357363
1: {}
@@ -376,12 +382,13 @@ def test_backout_removed_file(mock_secrets, fake_hg_repo):
376382
copy_pushlog_database(remote, local)
377383

378384
phabricator = PhabricatorUploader(local, revision)
379-
results = phabricator.generate({
385+
report = covdir_report({
380386
'source_files': [{
381387
'name': 'file',
382388
'coverage': [None, 0, 1, 1, 1, 1, 0],
383389
}]
384-
}, changesets(local, revision))
390+
})
391+
results = phabricator.generate(report, changesets(local, revision))
385392

386393
assert results == {
387394
1: {

0 commit comments

Comments
 (0)