Merge pull request #3092 from pre-commit/minimum-version-first · aless10/pre-commit@d3fa7f4 · GitHub
Skip to content

Commit d3fa7f4

Browse files
asottileasottile
authored
Merge pull request pre-commit#3092 from pre-commit/minimum-version-first
attempt minimum_pre_commit_version first when parsing configs
2 parents 23a2b73 + 047439a commit d3fa7f4

4 files changed

Lines changed: 119 additions & 134 deletions

File tree

‎pre_commit/clientlib.py‎

Lines changed: 14 additions & 6 deletions

‎pre_commit/repository.py‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from pre_commit.clientlib import load_manifest
1313
from pre_commit.clientlib import LOCAL
1414
from pre_commit.clientlib import META
15-
from pre_commit.clientlib import parse_version
1615
from pre_commit.hook import Hook
1716
from pre_commit.lang_base import environment_dir
1817
from pre_commit.prefix import Prefix
@@ -124,15 +123,6 @@ def _hook(
124123
for dct in rest:
125124
ret.update(dct)
126125

127-
version = ret['minimum_pre_commit_version']
128-
if parse_version(version) > parse_version(C.VERSION):
129-
logger.error(
130-
f'The hook `{ret["id"]}` requires pre-commit version {version} '
131-
f'but version {C.VERSION} is installed. '
132-
f'Perhaps run `pip install --upgrade pre-commit`.',
133-
)
134-
exit(1)
135-
136126
lang = ret['language']
137127
if ret['language_version'] == C.DEFAULT:
138128
ret['language_version'] = root_config['default_language_version'][lang]

‎tests/clientlib_test.py‎

Lines changed: 105 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -40,56 +40,51 @@ def test_check_type_tag_success():
4040

4141

4242
@pytest.mark.parametrize(
43-
('config_obj', 'expected'), (
44-
(
45-
{
46-
'repos': [{
47-
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
48-
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
49-
'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}],
50-
}],
51-
},
52-
True,
53-
),
54-
(
55-
{
56-
'repos': [{
57-
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
58-
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
59-
'hooks': [
60-
{
61-
'id': 'pyflakes',
62-
'files': '\\.py$',
63-
'args': ['foo', 'bar', 'baz'],
64-
},
65-
],
66-
}],
67-
},
68-
True,
69-
),
70-
(
71-
{
72-
'repos': [{
73-
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
74-
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
75-
'hooks': [
76-
{
77-
'id': 'pyflakes',
78-
'files': '\\.py$',
79-
# Exclude pattern must be a string
80-
'exclude': 0,
81-
'args': ['foo', 'bar', 'baz'],
82-
},
83-
],
84-
}],
85-
},
86-
False,
87-
),
43+
'cfg',
44+
(
45+
{
46+
'repos': [{
47+
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
48+
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
49+
'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}],
50+
}],
51+
},
52+
{
53+
'repos': [{
54+
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
55+
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
56+
'hooks': [
57+
{
58+
'id': 'pyflakes',
59+
'files': '\\.py$',
60+
'args': ['foo', 'bar', 'baz'],
61+
},
62+
],
63+
}],
64+
},
8865
),
8966
)
90-
def test_config_valid(config_obj, expected):
91-
ret = is_valid_according_to_schema(config_obj, CONFIG_SCHEMA)
92-
assert ret is expected
67+
def test_config_valid(cfg):
68+
assert is_valid_according_to_schema(cfg, CONFIG_SCHEMA)
69+
70+
71+
def test_invalid_config_wrong_type():
72+
cfg = {
73+
'repos': [{
74+
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
75+
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
76+
'hooks': [
77+
{
78+
'id': 'pyflakes',
79+
'files': '\\.py$',
80+
# Exclude pattern must be a string
81+
'exclude': 0,
82+
'args': ['foo', 'bar', 'baz'],
83+
},
84+
],
85+
}],
86+
}
87+
assert not is_valid_according_to_schema(cfg, CONFIG_SCHEMA)
9388

9489

9590
def test_local_hooks_with_rev_fails():
@@ -198,14 +193,13 @@ def test_warn_mutable_rev_conditional():
198193
),
199194
)
200195
def test_sensible_regex_validators_dont_pass_none(validator_cls):
201-
key = 'files'
196+
validator = validator_cls('files', cfgv.check_string)
202197
with pytest.raises(cfgv.ValidationError) as excinfo:
203-
validator = validator_cls(key, cfgv.check_string)
204-
validator.check({key: None})
198+
validator.check({'files': None})
205199

206200
assert str(excinfo.value) == (
207201
'\n'
208-
f'==> At key: {key}'
202+
'==> At key: files'
209203
'\n'
210204
'=====> Expected string got NoneType'
211205
)
@@ -298,46 +292,36 @@ def test_validate_optional_sensible_regex_at_top_level(caplog, regex, warning):
298292

299293

300294
@pytest.mark.parametrize(
301-
('manifest_obj', 'expected'),
295+
'manifest_obj',
302296
(
303-
(
304-
[{
305-
'id': 'a',
306-
'name': 'b',
307-
'entry': 'c',
308-
'language': 'python',
309-
'files': r'\.py$',
310-
}],
311-
True,
312-
),
313-
(
314-
[{
315-
'id': 'a',
316-
'name': 'b',
317-
'entry': 'c',
318-
'language': 'python',
319-
'language_version': 'python3.4',
320-
'files': r'\.py$',
321-
}],
322-
True,
323-
),
324-
(
325-
# A regression in 0.13.5: always_run and files are permissible
326-
[{
327-
'id': 'a',
328-
'name': 'b',
329-
'entry': 'c',
330-
'language': 'python',
331-
'files': '',
332-
'always_run': True,
333-
}],
334-
True,
335-
),
297+
[{
298+
'id': 'a',
299+
'name': 'b',
300+
'entry': 'c',
301+
'language': 'python',
302+
'files': r'\.py$',
303+
}],
304+
[{
305+
'id': 'a',
306+
'name': 'b',
307+
'entry': 'c',
308+
'language': 'python',
309+
'language_version': 'python3.4',
310+
'files': r'\.py$',
311+
}],
312+
# A regression in 0.13.5: always_run and files are permissible
313+
[{
314+
'id': 'a',
315+
'name': 'b',
316+
'entry': 'c',
317+
'language': 'python',
318+
'files': '',
319+
'always_run': True,
320+
}],
336321
),
337322
)
338-
def test_valid_manifests(manifest_obj, expected):
339-
ret = is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA)
340-
assert ret is expected
323+
def test_valid_manifests(manifest_obj):
324+
assert is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA)
341325

342326

343327
@pytest.mark.parametrize(
@@ -393,8 +377,39 @@ def test_parse_version():
393377

394378

395379
def test_minimum_pre_commit_version_failing():
380+
cfg = {'repos': [], 'minimum_pre_commit_version': '999'}
381+
with pytest.raises(cfgv.ValidationError) as excinfo:
382+
cfgv.validate(cfg, CONFIG_SCHEMA)
383+
assert str(excinfo.value) == (
384+
f'\n'
385+
f'==> At Config()\n'
386+
f'==> At key: minimum_pre_commit_version\n'
387+
f'=====> pre-commit version 999 is required but version {C.VERSION} '
388+
f'is installed. Perhaps run `pip install --upgrade pre-commit`.'
389+
)
390+
391+
392+
def test_minimum_pre_commit_version_failing_in_config():
393+
cfg = {'repos': [sample_local_config()]}
394+
cfg['repos'][0]['hooks'][0]['minimum_pre_commit_version'] = '999'
395+
with pytest.raises(cfgv.ValidationError) as excinfo:
396+
cfgv.validate(cfg, CONFIG_SCHEMA)
397+
assert str(excinfo.value) == (
398+
f'\n'
399+
f'==> At Config()\n'
400+
f'==> At key: repos\n'
401+
f"==> At Repository(repo='local')\n"
402+
f'==> At key: hooks\n'
403+
f"==> At Hook(id='do_not_commit')\n"
404+
f'==> At key: minimum_pre_commit_version\n'
405+
f'=====> pre-commit version 999 is required but version {C.VERSION} '
406+
f'is installed. Perhaps run `pip install --upgrade pre-commit`.'
407+
)
408+
409+
410+
def test_minimum_pre_commit_version_failing_before_other_error():
411+
cfg = {'repos': 5, 'minimum_pre_commit_version': '999'}
396412
with pytest.raises(cfgv.ValidationError) as excinfo:
397-
cfg = {'repos': [], 'minimum_pre_commit_version': '999'}
398413
cfgv.validate(cfg, CONFIG_SCHEMA)
399414
assert str(excinfo.value) == (
400415
f'\n'

‎tests/repository_test.py‎

Lines changed: 0 additions & 28 deletions

0 commit comments

Comments
 (0)