Skip to content
This repository has been archived by the owner on Nov 11, 2019. It is now read-only.

Commit

Permalink
shipit_static_analysis: Dump analyzers raw outputs as Taskcluster art…
Browse files Browse the repository at this point in the history
…ifacts for debugging. #1257
  • Loading branch information
jankeromnes authored and Bastien Abadie committed Jul 13, 2018
1 parent 5af6ce4 commit 6a9877a
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 27 deletions.
12 changes: 10 additions & 2 deletions src/shipit_static_analysis/shipit_static_analysis/clang/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,19 @@ def run_clang_format(self, filename, revision):
logger.info('Running clang-format', cmd=' '.join(cmd))

# Run command
clang_output = subprocess.check_output(cmd, cwd=settings.repo_dir)
clang_output = subprocess.check_output(cmd, cwd=settings.repo_dir).decode('utf-8')

# Dump raw clang-format output as a Taskcluster artifact (for debugging)
clang_output_path = os.path.join(
settings.taskcluster_results_dir,
'{}-clang-format.txt'.format(repr(revision)),
)
with open(clang_output_path, 'w') as f:
f.write(clang_output)

# Compare output with original file
src_lines = [x.rstrip('\n') for x in open(full_path).readlines()]
clang_lines = clang_output.decode('utf-8').split('\n')
clang_lines = clang_output.split('\n')

# Build issues from diff of diff !
diff = difflib.SequenceMatcher(
Expand Down
12 changes: 11 additions & 1 deletion src/shipit_static_analysis/shipit_static_analysis/clang/tidy.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,17 @@ def run(self, revision):
logger.error('Mach static analysis failed: {}'.format(e.output))
raise

issues = self.parse_issues(clang_output.decode('utf-8'), revision)
clang_output = clang_output.decode('utf-8')

# Dump raw clang-tidy output as a Taskcluster artifact (for debugging)
clang_output_path = os.path.join(
settings.taskcluster_results_dir,
'{}-clang-tidy.txt'.format(repr(revision)),
)
with open(clang_output_path, 'w') as f:
f.write(clang_output)

issues = self.parse_issues(clang_output, revision)

# Report stats for these issues
stats.report_issues('clang-tidy', issues)
Expand Down
9 changes: 9 additions & 0 deletions src/shipit_static_analysis/shipit_static_analysis/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import enum
import os
import tempfile

import requests
import yaml
Expand Down Expand Up @@ -43,6 +44,7 @@ def __init__(self):
self.cache_root = None
self.repo_dir = None
self.repo_shared_dir = None
self.taskcluster_results_dir = None

def setup(self, app_channel, cache_root, publication):
self.app_channel = app_channel
Expand All @@ -63,6 +65,13 @@ def setup(self, app_channel, cache_root, publication):
self.repo_dir = os.path.join(self.cache_root, 'sa-central')
self.repo_shared_dir = os.path.join(self.cache_root, 'sa-central-shared')

if 'TASK_ID' in os.environ and 'RUN_ID' in os.environ:
self.taskcluster_results_dir = '/tmp/results'
else:
self.taskcluster_results_dir = tempfile.mkdtemp()
if not os.path.isdir(self.taskcluster_results_dir):
os.makedirs(self.taskcluster_results_dir)

def __getattr__(self, key):
if key not in self.config:
raise AttributeError
Expand Down
14 changes: 12 additions & 2 deletions src/shipit_static_analysis/shipit_static_analysis/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,27 @@ def find_issues(self, path, revision):
path
]
returncode, output, error = run(' '.join(command), cwd=settings.repo_dir)
output = output.decode('utf-8')

# Dump raw mozlint output as a Taskcluster artifact (for debugging)
output_path = os.path.join(
settings.taskcluster_results_dir,
'{}-mozlint.txt'.format(repr(revision)),
)
with open(output_path, 'a') as f:
f.write(output)

if returncode == 0:
logger.debug('No Mozlint errors', path=path)
return
assert 'error: problem with lint setup' not in output.decode('utf-8'), \
assert 'error: problem with lint setup' not in output, \
'Mach lint setup failed'

# Load output as json
# Only consider last line, as ./mach lint may output
# linter setup output on stdout :/
try:
lines = list(filter(None, output.decode('utf-8').split('\n')))
lines = list(filter(None, output.split('\n')))
payload = json.loads(lines[-1])
except json.decoder.JSONDecodeError:
logger.warn('Invalid json output', path=path, lines=lines)
Expand Down
22 changes: 10 additions & 12 deletions src/shipit_static_analysis/shipit_static_analysis/revisions.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,12 @@ def namespaces(self):
'phabricator.diffphid.{}'.format(self.diff_phid),
]

def __repr__(self):
return self.diff_phid

def __str__(self):
return 'Phabricator #{} - {}'.format(self.diff_id, self.diff_phid)

def build_diff_name(self):
return '{}-clang-format.diff'.format(
self.diff_phid,
)

@property
def url(self):
return 'https://{}/D{}'.format(self.api.hostname, self.id)
Expand Down Expand Up @@ -172,6 +170,13 @@ def __init__(self, review_request_id, mercurial, diffset_revision):
self.review_request_id = int(review_request_id)
self.diffset_revision = int(diffset_revision)

def __repr__(self):
return '{}-{}-{}'.format(
self.mercurial[:8],
self.review_request_id,
self.diffset_revision,
)

def __str__(self):
return 'MozReview #{} - {}'.format(self.review_request_id, self.diffset_revision)

Expand All @@ -187,13 +192,6 @@ def namespaces(self):
'mozreview.rev.{}'.format(self.mercurial),
]

def build_diff_name(self):
return '{}-{}-{}-clang-format.diff'.format(
self.mercurial[:8],
self.review_request_id,
self.diffset_revision,
)

def load(self, repo):
'''
Load required revision from mercurial remote repo
Expand Down
11 changes: 3 additions & 8 deletions src/shipit_static_analysis/shipit_static_analysis/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import itertools
import os
import subprocess
import tempfile
from datetime import datetime
from datetime import timedelta

Expand Down Expand Up @@ -54,23 +53,19 @@ def __init__(self, reporters, analyzers, index_service):
if 'TASK_ID' in os.environ and 'RUN_ID' in os.environ:
self.taskcluster_task_id = os.environ['TASK_ID']
self.taskcluster_run_id = os.environ['RUN_ID']
self.taskcluster_results_dir = '/tmp/results'
self.on_taskcluster = True
else:
self.taskcluster_task_id = 'local instance'
self.taskcluster_run_id = 0
self.taskcluster_results_dir = tempfile.mkdtemp()
self.on_taskcluster = False
if not os.path.isdir(self.taskcluster_results_dir):
os.makedirs(self.taskcluster_results_dir)

# Load reporters to use
self.reporters = reporters
if not self.reporters:
logger.warn('No reporters configured, this analysis will not be published')

# Always add debug reporter and Diff reporter
self.reporters['debug'] = DebugReporter(output_dir=self.taskcluster_results_dir)
self.reporters['debug'] = DebugReporter(output_dir=settings.taskcluster_results_dir)

# Use TC index service client
self.index_service = index_service
Expand Down Expand Up @@ -295,8 +290,8 @@ def build_improvement_patch(self, revision, issues):
return

# Write diff in results directory
diff_name = revision.build_diff_name()
diff_path = os.path.join(self.taskcluster_results_dir, diff_name)
diff_name = '{}-clang-format.diff'.format(repr(revision))
diff_path = os.path.join(settings.taskcluster_results_dir, diff_name)
with open(diff_path, 'w') as f:
length = f.write(diff.decode('utf-8'))
logger.info('Improvement diff dumped', path=diff_path, length=length)
Expand Down
16 changes: 16 additions & 0 deletions src/shipit_static_analysis/tests/test_clang.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def test_clang_format(mock_config, mock_repository, mock_stats, mock_clang, mock
Test clang-format runner
'''
from shipit_static_analysis.clang.format import ClangFormat, ClangFormatIssue
from shipit_static_analysis.config import settings

# Write badly formatted c file
bad_file = os.path.join(mock_config.repo_dir, 'bad.cpp')
Expand Down Expand Up @@ -100,6 +101,13 @@ def test_clang_format(mock_config, mock_repository, mock_stats, mock_clang, mock
mock_workflow.build_improvement_patch(mock_revision, issues)
assert open(bad_file).read() == BAD_CPP_VALID

# Ensure the raw output dump exists
clang_output_path = os.path.join(
settings.taskcluster_results_dir,
'{}-clang-format.txt'.format(repr(mock_revision)),
)
assert os.path.isfile(clang_output_path)

# Test stats
mock_stats.flush()
metrics = mock_stats.get_metrics('issues.clang-format')
Expand All @@ -120,6 +128,7 @@ def test_clang_tidy(mock_repository, mock_config, mock_clang, mock_stats, mock_r
Test clang-tidy runner
'''
from shipit_static_analysis.clang.tidy import ClangTidy, ClangTidyIssue
from shipit_static_analysis.config import settings

# Init clang tidy runner
ct = ClangTidy()
Expand All @@ -143,6 +152,13 @@ def test_clang_tidy(mock_repository, mock_config, mock_clang, mock_stats, mock_r
assert issues[1].check == 'modernize-use-nullptr'
assert issues[1].line == 8

# Ensure the raw output dump exists
clang_output_path = os.path.join(
settings.taskcluster_results_dir,
'{}-clang-tidy.txt'.format(repr(mock_revision)),
)
assert os.path.isfile(clang_output_path)

# Test stats
mock_stats.flush()
metrics = mock_stats.get_metrics('issues.clang-tidy')
Expand Down
4 changes: 2 additions & 2 deletions src/shipit_static_analysis/tests/test_revisions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test_mozreview():
assert r.review_request_id == 164530
assert r.diffset_revision == 7
assert r.url == 'https://reviewboard.mozilla.org/r/164530/'
assert r.build_diff_name() == '308c22e7-164530-7-clang-format.diff'
assert repr(r) == '308c22e7-164530-7'


@responses.activate
Expand All @@ -40,7 +40,7 @@ def test_phabricator(mock_phabricator, mock_repository, mock_config):
assert r.diff_id == 42
assert r.diff_phid == 'PHID-DIFF-testABcd12'
assert r.url == 'https://phabricator.test/D51'
assert r.build_diff_name() == 'PHID-DIFF-testABcd12-clang-format.diff'
assert repr(r) == 'PHID-DIFF-testABcd12'
assert r.id == 51 # revision

# Check test.txt content
Expand Down

0 comments on commit 6a9877a

Please sign in to comment.