diff --git a/flake8/hooks.py b/flake8/hooks.py index 125e3cb..e938632 100644 --- a/flake8/hooks.py +++ b/flake8/hooks.py @@ -6,7 +6,7 @@ import sys import stat from subprocess import Popen, PIPE import shutil -from tempfile import mkdtemp +import tempfile try: from configparser import ConfigParser except ImportError: # Python 2 @@ -48,15 +48,10 @@ def git_hook(complexity=-1, strict=False, ignore=None, lazy=False): if complexity > -1: options['max_complexity'] = complexity - tmpdir = mkdtemp() + tmpdir = tempfile.mkdtemp() flake8_style = get_style_guide(config_file=DEFAULT_CONFIG, paths=['.'], **options) - # Since we use a temporary directory, the exclude path needs to be fixed - # to prepend that. - path_join = os.path.join - flake8_style.options.exclude = [path_join(tmpdir, x) if "/" in x else x - for x in flake8_style.options.exclude] filepatterns = flake8_style.options.filename # Copy staged versions to temporary directory @@ -74,15 +69,16 @@ def git_hook(complexity=-1, strict=False, ignore=None, lazy=False): dirname = os.path.join(tmpdir, dirname) if not os.path.isdir(dirname): os.makedirs(dirname) - filename = os.path.join(dirname, filename) - # write staged version of file to temporary directory - with open(filename, "wb") as fh: - fh.write(out) # check_files() only does this check if passed a dir; so we do it - if ((pep8.filename_match(filename, filepatterns) and - not flake8_style.excluded(filename))): + if ((pep8.filename_match(file_, filepatterns) and + not flake8_style.excluded(file_))): + + filename = os.path.join(dirname, filename) files_to_check.append(filename) + # write staged version of file to temporary directory + with open(filename, "wb") as fh: + fh.write(out) # Run the checks report = flake8_style.check_files(files_to_check) diff --git a/flake8/tests/test_git_hook.py b/flake8/tests/test_git_hook.py deleted file mode 100644 index 3efa477..0000000 --- a/flake8/tests/test_git_hook.py +++ /dev/null @@ -1,97 +0,0 @@ -import unittest -import mock - -import flake8.hooks -import flake8.engine - - -def side_effect(command, raw_output=False, decode=False): - if command.startswith("git diff-index") and not raw_output: - return 0, ["ignore.py", "myproject/main.py", - "myproject/test/test.py"], [] - elif command.startswith("git show") and raw_output: - return 0, "dummy standard output", "dummy standard error" - else: - raise NotImplementedError("side_effect") - - -def mock_open(read_data=""): - # - # mock_open from older versions of mock do not have readline - # - # There is a patch for this issue (http://bugs.python.org/issue17467). - # I didn't use it directly since I needed readline to return None after all - # lines have been read. The multiple None objects appended at the end - # achieve this. I'm not sure why I need several -- one or two weren't - # enough. - # - mo = mock.mock_open(read_data=read_data) - lines = [x + "\n" for x in read_data.split("\n")] + [None]*10 - mo.return_value.readline.side_effect = lines - return mo - - -class TestFixExclude(unittest.TestCase): - """The scenario used to test fix_exclude is as follows. There is a git - repo with two files: ignore.py, myproject/main.py and - myproject/test/test.py. We want to run flake over myproject/main.py only. - To achieve this, we add myproject/test/test.py to the list of exclude - patterns -- in real life, this is done by editing configuration files - (http://flake8.readthedocs.org/en/latest/config.html), but for the test we - just mock it. - - Without the fix, since the exclude pattern for test.py contains a slash, - pep8.read_config will convert it to an absolute path, using os.curdir as a - base. This behavior causes pep8.filename_match to fail, since our files - will be in tmpdir, not os.curdir. The end result is flake8 ends up - checking myproject/test/test.py, which is something we don't want. - - With the fix, the exclude pattern is set correctly, and flake8 only checks - one file: myproject/main.py. - - The exclude pattern for ignore.py is not affected by the fix, since it - doesn't contain a slash, and isn't converted to an absolute path by - pep8.read_config. - """ - - def setUp(self): - self.config = "[flake8]\nexclude = ignore.py,myproject/test/test.py" - - @mock.patch("os.makedirs") - @mock.patch.object(flake8.engine.StyleGuide, "check_files") - @mock.patch("flake8.hooks.fix_exclude") - @mock.patch("flake8.hooks.mkdtemp", return_value="/tmp") - @mock.patch("flake8.hooks.run", side_effect=side_effect) - def test_without_fix(self, mock_run, mock_mkdtemp, mock_fix_exclude, - mock_check_files, mock_makedirs): - # - # http://www.voidspace.org.uk/python/mock/helpers.html#mock.mock_open - # - with mock.patch("ConfigParser.open", mock_open(read_data=self.config), - create=True): - with mock.patch("flake8.hooks.open", mock.mock_open(), create=True): - flake8.hooks.git_hook() - - self.assertEquals(mock_check_files.call_count, 1) - args = mock_check_files.call_args[0][0] - print args - self.assertEquals(len(args), 2) - - @mock.patch("os.makedirs") - @mock.patch.object(flake8.engine.StyleGuide, "check_files") - @mock.patch("flake8.hooks.mkdtemp", return_value="/tmp") - @mock.patch("flake8.hooks.run", side_effect=side_effect) - def test_with_fix(self, mock_run, mock_mkdtemp, mock_check_files, - mock_makedirs): - # - # http://www.voidspace.org.uk/python/mock/helpers.html#mock.mock_open - # - with mock.patch("ConfigParser.open", mock_open(read_data=self.config), - create=True): - with mock.patch("flake8.hooks.open", mock.mock_open(), create=True): - flake8.hooks.git_hook() - - self.assertEquals(mock_check_files.call_count, 1) - args = mock_check_files.call_args[0][0] - print args - self.assertEquals(len(args), 1) diff --git a/flake8/tests/test_hooks.py b/flake8/tests/test_hooks.py new file mode 100644 index 0000000..31fcd81 --- /dev/null +++ b/flake8/tests/test_hooks.py @@ -0,0 +1,37 @@ +"""Module containing the tests for flake8.hooks.""" +import os +import unittest + +try: + from unittest import mock +except ImportError: + import mock + +import flake8.hooks + + +def excluded(filename): + return filename.endswith('afile.py') + + +class TestGitHook(unittest.TestCase): + @mock.patch('os.makedirs') + @mock.patch('flake8.hooks.open', create=True) + @mock.patch('shutil.rmtree') + @mock.patch('tempfile.mkdtemp', return_value='/fake/tmp') + @mock.patch('flake8.hooks.run', + return_value=(None, ['foo/afile.py', 'foo/bfile.py'], None)) + @mock.patch('flake8.hooks.get_style_guide') + def test_prepends_tmp_directory_to_exclude(self, get_style_guide, run, + *args): + style_guide = get_style_guide.return_value = mock.Mock() + style_guide.options.exclude = ['foo/afile.py'] + style_guide.options.filename = ['foo/*'] + style_guide.excluded = excluded + + flake8.hooks.git_hook() + + dirname, filename = os.path.split(os.path.abspath('foo/bfile.py')) + tmpdir = os.path.join('/fake/tmp', dirname[1:]) + tmpfile = os.path.join(tmpdir, 'bfile.py') + style_guide.check_files.assert_called_once_with([tmpfile])