From b0f1cabd61a823160670dd0240774b5d19742020 Mon Sep 17 00:00:00 2001 From: Matt Bachmann Date: Fri, 12 Aug 2016 22:48:46 -0400 Subject: [PATCH 1/4] Issue-47 add test highlighting failure --- .../fixtures/git_diff_violations_two_files.txt | 14 ++++++++++++++ diff_cover/tests/fixtures/hello.py | 2 ++ diff_cover/tests/fixtures/hi.py | 2 ++ diff_cover/tests/fixtures/pyflakes_two_files.txt | 14 ++++++++++++++ diff_cover/tests/test_integration.py | 7 +++++++ 5 files changed, 39 insertions(+) create mode 100644 diff_cover/tests/fixtures/git_diff_violations_two_files.txt create mode 100644 diff_cover/tests/fixtures/hello.py create mode 100644 diff_cover/tests/fixtures/hi.py create mode 100644 diff_cover/tests/fixtures/pyflakes_two_files.txt diff --git a/diff_cover/tests/fixtures/git_diff_violations_two_files.txt b/diff_cover/tests/fixtures/git_diff_violations_two_files.txt new file mode 100644 index 00000000..1797e504 --- /dev/null +++ b/diff_cover/tests/fixtures/git_diff_violations_two_files.txt @@ -0,0 +1,14 @@ +diff --git a/hello.py b/hello.py +index b732142..b2ba069 100644 +--- a/hello.py ++++ b/hello.py +@@ -1 +1,2 @@ + print "hello" ++print unknown_var +diff --git a/hi.py b/hi.py +index bae1109..151d05d 100644 +--- a/hi.py ++++ b/hi.py +@@ -1 +1,2 @@ + print "hi" ++print unknown_var \ No newline at end of file diff --git a/diff_cover/tests/fixtures/hello.py b/diff_cover/tests/fixtures/hello.py new file mode 100644 index 00000000..c47d0e6c --- /dev/null +++ b/diff_cover/tests/fixtures/hello.py @@ -0,0 +1,2 @@ +print "hello" +print unknown_var \ No newline at end of file diff --git a/diff_cover/tests/fixtures/hi.py b/diff_cover/tests/fixtures/hi.py new file mode 100644 index 00000000..c47d0e6c --- /dev/null +++ b/diff_cover/tests/fixtures/hi.py @@ -0,0 +1,2 @@ +print "hello" +print unknown_var \ No newline at end of file diff --git a/diff_cover/tests/fixtures/pyflakes_two_files.txt b/diff_cover/tests/fixtures/pyflakes_two_files.txt new file mode 100644 index 00000000..77d5b5de --- /dev/null +++ b/diff_cover/tests/fixtures/pyflakes_two_files.txt @@ -0,0 +1,14 @@ +------------- +Diff Quality +Quality Report: pyflakes +Diff: origin/master...HEAD, staged, and unstaged changes +------------- +hello.py (0.0%): + 2: undefined name 'unknown_var' +hi.py (0.0%): + 2: undefined name 'unknown_var' +------------- +Total: 2 lines +Violations: 2 lines +% Quality: 0% +------------- \ No newline at end of file diff --git a/diff_cover/tests/test_integration.py b/diff_cover/tests/test_integration.py index ab106cdc..b9133e27 100644 --- a/diff_cover/tests/test_integration.py +++ b/diff_cover/tests/test_integration.py @@ -469,6 +469,13 @@ def test_added_file_pyflakes_console(self): ['diff-quality', '--violations=pyflakes'] ) + def test_added_file_pyflakes_console_two_files(self): + self._check_console_report( + 'git_diff_violations_two_files.txt', + 'pyflakes_two_files.txt', + ['diff-quality', '--violations=pyflakes'] + ) + def test_added_file_pylint_console(self): self._check_console_report( 'git_diff_violations.txt', From decbf17faf2f4ae73561385cbe3747414c59e3c2 Mon Sep 17 00:00:00 2001 From: Matt Bachmann Date: Fri, 12 Aug 2016 22:51:51 -0400 Subject: [PATCH 2/4] Issue-47 dont overwrite the reports variable. In fact, separate it so its *only* used when passing in reports. Otherwise just add to the report dict direcrtly --- diff_cover/violationsreporters/base.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/diff_cover/violationsreporters/base.py b/diff_cover/violationsreporters/base.py index 1be00b73..1087c0ba 100644 --- a/diff_cover/violationsreporters/base.py +++ b/diff_cover/violationsreporters/base.py @@ -114,7 +114,7 @@ def __init__(self, driver, reports=None, options=None): """ super(QualityReporter, self).__init__(driver.name) self.reports = self._load_reports(reports) if reports else None - self.violations_dict = {} + self.violations_dict = defaultdict(list) self.driver = driver self.options = options self.driver_tool_installed = None @@ -142,7 +142,9 @@ def violations(self, src_path): if not any(src_path.endswith(ext) for ext in self.driver.supported_extensions): return [] if src_path not in self.violations_dict: - if not self.reports: + if self.reports: + self.violations_dict = self.driver.parse_reports(self.reports) + else: if self.driver_tool_installed is None: self.driver_tool_installed = self.driver.installed() if not self.driver_tool_installed: @@ -152,8 +154,8 @@ def violations(self, src_path): command.append(self.options) command.append(src_path.encode(sys.getfilesystemencoding())) output, _ = execute(command) - self.reports = [output] - self.violations_dict = self.driver.parse_reports(self.reports) + self.violations_dict.update(self.driver.parse_reports([output])) + return self.violations_dict[src_path] def measured_lines(self, src_path): From 85a734908f1c4cbaa13fe0d70dc8d5e39d439254 Mon Sep 17 00:00:00 2001 From: Matt Bachmann Date: Fri, 12 Aug 2016 23:05:56 -0400 Subject: [PATCH 3/4] Issue-47 apparently even fixtures need to be valid python 3 for ci --- diff_cover/tests/fixtures/hello.py | 4 ++-- diff_cover/tests/fixtures/hi.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/diff_cover/tests/fixtures/hello.py b/diff_cover/tests/fixtures/hello.py index c47d0e6c..e2974381 100644 --- a/diff_cover/tests/fixtures/hello.py +++ b/diff_cover/tests/fixtures/hello.py @@ -1,2 +1,2 @@ -print "hello" -print unknown_var \ No newline at end of file +print("hello") +print(unknown_var) \ No newline at end of file diff --git a/diff_cover/tests/fixtures/hi.py b/diff_cover/tests/fixtures/hi.py index c47d0e6c..e2974381 100644 --- a/diff_cover/tests/fixtures/hi.py +++ b/diff_cover/tests/fixtures/hi.py @@ -1,2 +1,2 @@ -print "hello" -print unknown_var \ No newline at end of file +print("hello") +print(unknown_var) \ No newline at end of file From 552bfe4cc2b979c93330d206d69edaedc71f1d2b Mon Sep 17 00:00:00 2001 From: Matt Bachmann Date: Fri, 12 Aug 2016 23:36:02 -0400 Subject: [PATCH 4/4] Issue-47 skip files that have been deleted --- diff_cover/tests/test_violations_reporter.py | 29 ++++++++++++++++++-- diff_cover/violationsreporters/base.py | 7 +++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/diff_cover/tests/test_violations_reporter.py b/diff_cover/tests/test_violations_reporter.py index f09e79bb..53a7e48d 100644 --- a/diff_cover/tests/test_violations_reporter.py +++ b/diff_cover/tests/test_violations_reporter.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +import os import subprocess import xml.etree.cElementTree as etree from subprocess import Popen @@ -8,6 +9,7 @@ import mock import six +from diff_cover.violationsreporters import base from diff_cover.command_runner import CommandError, run_command_for_code from diff_cover.tests.helpers import unittest @@ -18,6 +20,10 @@ from six import BytesIO, StringIO +def _patch_so_all_files_exist(): + _mock_exists = patch.object(base.os.path, 'exists').start() + _mock_exists.returnvalue = True + def _setup_patch(return_value, status_code=0): mocked_process = mock.Mock() mocked_process.returncode = status_code @@ -314,6 +320,9 @@ def _coverage_xml( class Pep8QualityReporterTest(unittest.TestCase): + def setUp(self): + _patch_so_all_files_exist() + def tearDown(self): """ Undo all patches @@ -450,6 +459,7 @@ def tearDown(self): patch.stopall() def test_quality(self): + _patch_so_all_files_exist() # Patch the output of `pyflakes` return_string = '\n' + dedent(""" @@ -494,6 +504,7 @@ def test_no_quality_issues_emptystring(self): self.assertEqual([], quality.violations('file1.py')) def test_quality_error(self): + _patch_so_all_files_exist() # Patch the output of `pyflakes` _setup_patch((b"", b'whoops'), status_code=1) @@ -571,6 +582,7 @@ def tearDown(self): patch.stopall() def test_quality(self): + _patch_so_all_files_exist() # Patch the output of `flake8` return_string = '\n' + dedent(""" @@ -636,6 +648,7 @@ def test_no_quality_issues_emptystring(self): self.assertEqual([], quality.violations('file1.py')) def test_quality_error(self): + _patch_so_all_files_exist() # Patch the output of `flake8` _setup_patch((b"", 'whoops Ƕئ'.encode('utf-8')), status_code=1) @@ -666,6 +679,14 @@ def test_no_python_file(self): result = quality.violations(path) self.assertEqual(result, []) + def test_file_does_not_exist(self): + quality = QualityReporter(flake8_driver) + file_paths = ['ajshdjlasdhajksdh.py'] + # Expect that we get no results because that file does not exist + for path in file_paths: + result = quality.violations(path) + self.assertEqual(result, []) + def test_quality_pregenerated_report(self): # When the user provides us with a pre-generated flake8 report @@ -708,6 +729,9 @@ def test_quality_pregenerated_report(self): class PylintQualityReporterTest(unittest.TestCase): + def setUp(self): + _patch_so_all_files_exist() + def tearDown(self): """ Undo all patches. @@ -949,7 +973,7 @@ def test_quality(self): """ Test basic scenarios, including special characters that would appear in JavaScript and mixed quotation marks """ - + _patch_so_all_files_exist() # Patch the output of the linter cmd return_string = '\n' + dedent(""" ../test_file.js: line 3, col 9, Missing "use strict" statement. @@ -998,7 +1022,7 @@ def test_no_quality_issues_emptystring(self): self.assertEqual([], quality.violations('file1.js')) def test_quality_error(self): - + _patch_so_all_files_exist() _setup_patch((b"", 'whoops Ƕئ'.encode('utf-8')), status_code=1) with patch('diff_cover.violationsreporters.base.run_command_for_code') as code: code.return_value = 0 @@ -1141,6 +1165,7 @@ def setUp(self): @patch('sys.stderr', new_callable=StringIO) def test_quality_reporter(self, mock_stderr): + _patch_so_all_files_exist() with patch('diff_cover.violationsreporters.base.run_command_for_code') as code: code.return_value = 0 reporter = QualityReporter(pep8_driver) diff --git a/diff_cover/violationsreporters/base.py b/diff_cover/violationsreporters/base.py index 1087c0ba..79d2d02a 100644 --- a/diff_cover/violationsreporters/base.py +++ b/diff_cover/violationsreporters/base.py @@ -152,9 +152,10 @@ def violations(self, src_path): command = copy.deepcopy(self.driver.command) if self.options: command.append(self.options) - command.append(src_path.encode(sys.getfilesystemencoding())) - output, _ = execute(command) - self.violations_dict.update(self.driver.parse_reports([output])) + if os.path.exists(src_path): + command.append(src_path.encode(sys.getfilesystemencoding())) + output, _ = execute(command) + self.violations_dict.update(self.driver.parse_reports([output])) return self.violations_dict[src_path]