mirror of
https://github.com/pre-commit/pre-commit.git
synced 2026-02-17 08:14:42 +04:00
attempt minimum_pre_commit_version first when parsing configs
This commit is contained in:
Anthony Sottile
parent
23a2b7360e
commit
047439abff
4 changed files with 119 additions and 134 deletions
|
|
@ -102,6 +102,13 @@ class StagesMigration(StagesMigrationNoDefault):
|
|||
MANIFEST_HOOK_DICT = cfgv.Map(
|
||||
'Hook', 'id',
|
||||
|
||||
# check first in case it uses some newer, incompatible feature
|
||||
cfgv.Optional(
|
||||
'minimum_pre_commit_version',
|
||||
cfgv.check_and(cfgv.check_string, check_min_version),
|
||||
'0',
|
||||
),
|
||||
|
||||
cfgv.Required('id', cfgv.check_string),
|
||||
cfgv.Required('name', cfgv.check_string),
|
||||
cfgv.Required('entry', cfgv.check_string),
|
||||
|
|
@ -124,7 +131,6 @@ MANIFEST_HOOK_DICT = cfgv.Map(
|
|||
cfgv.Optional('description', cfgv.check_string, ''),
|
||||
cfgv.Optional('language_version', cfgv.check_string, C.DEFAULT),
|
||||
cfgv.Optional('log_file', cfgv.check_string, ''),
|
||||
cfgv.Optional('minimum_pre_commit_version', cfgv.check_string, '0'),
|
||||
cfgv.Optional('require_serial', cfgv.check_bool, False),
|
||||
StagesMigration('stages', []),
|
||||
cfgv.Optional('verbose', cfgv.check_bool, False),
|
||||
|
|
@ -345,6 +351,13 @@ DEFAULT_LANGUAGE_VERSION = cfgv.Map(
|
|||
CONFIG_SCHEMA = cfgv.Map(
|
||||
'Config', None,
|
||||
|
||||
# check first in case it uses some newer, incompatible feature
|
||||
cfgv.Optional(
|
||||
'minimum_pre_commit_version',
|
||||
cfgv.check_and(cfgv.check_string, check_min_version),
|
||||
'0',
|
||||
),
|
||||
|
||||
cfgv.RequiredRecurse('repos', cfgv.Array(CONFIG_REPO_DICT)),
|
||||
cfgv.Optional(
|
||||
'default_install_hook_types',
|
||||
|
|
@ -358,11 +371,6 @@ CONFIG_SCHEMA = cfgv.Map(
|
|||
cfgv.Optional('files', check_string_regex, ''),
|
||||
cfgv.Optional('exclude', check_string_regex, '^$'),
|
||||
cfgv.Optional('fail_fast', cfgv.check_bool, False),
|
||||
cfgv.Optional(
|
||||
'minimum_pre_commit_version',
|
||||
cfgv.check_and(cfgv.check_string, check_min_version),
|
||||
'0',
|
||||
),
|
||||
cfgv.WarnAdditionalKeys(
|
||||
(
|
||||
'repos',
|
||||
|
|
|
|||
|
|
@ -12,7 +12,6 @@ from pre_commit.all_languages import languages
|
|||
from pre_commit.clientlib import load_manifest
|
||||
from pre_commit.clientlib import LOCAL
|
||||
from pre_commit.clientlib import META
|
||||
from pre_commit.clientlib import parse_version
|
||||
from pre_commit.hook import Hook
|
||||
from pre_commit.lang_base import environment_dir
|
||||
from pre_commit.prefix import Prefix
|
||||
|
|
@ -124,15 +123,6 @@ def _hook(
|
|||
for dct in rest:
|
||||
ret.update(dct)
|
||||
|
||||
version = ret['minimum_pre_commit_version']
|
||||
if parse_version(version) > parse_version(C.VERSION):
|
||||
logger.error(
|
||||
f'The hook `{ret["id"]}` requires pre-commit version {version} '
|
||||
f'but version {C.VERSION} is installed. '
|
||||
f'Perhaps run `pip install --upgrade pre-commit`.',
|
||||
)
|
||||
exit(1)
|
||||
|
||||
lang = ret['language']
|
||||
if ret['language_version'] == C.DEFAULT:
|
||||
ret['language_version'] = root_config['default_language_version'][lang]
|
||||
|
|
|
|||
|
|
@ -40,56 +40,51 @@ def test_check_type_tag_success():
|
|||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
('config_obj', 'expected'), (
|
||||
(
|
||||
{
|
||||
'repos': [{
|
||||
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
|
||||
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
|
||||
'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}],
|
||||
}],
|
||||
},
|
||||
True,
|
||||
),
|
||||
(
|
||||
{
|
||||
'repos': [{
|
||||
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
|
||||
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
|
||||
'hooks': [
|
||||
{
|
||||
'id': 'pyflakes',
|
||||
'files': '\\.py$',
|
||||
'args': ['foo', 'bar', 'baz'],
|
||||
},
|
||||
],
|
||||
}],
|
||||
},
|
||||
True,
|
||||
),
|
||||
(
|
||||
{
|
||||
'repos': [{
|
||||
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
|
||||
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
|
||||
'hooks': [
|
||||
{
|
||||
'id': 'pyflakes',
|
||||
'files': '\\.py$',
|
||||
# Exclude pattern must be a string
|
||||
'exclude': 0,
|
||||
'args': ['foo', 'bar', 'baz'],
|
||||
},
|
||||
],
|
||||
}],
|
||||
},
|
||||
False,
|
||||
),
|
||||
'cfg',
|
||||
(
|
||||
{
|
||||
'repos': [{
|
||||
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
|
||||
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
|
||||
'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}],
|
||||
}],
|
||||
},
|
||||
{
|
||||
'repos': [{
|
||||
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
|
||||
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
|
||||
'hooks': [
|
||||
{
|
||||
'id': 'pyflakes',
|
||||
'files': '\\.py$',
|
||||
'args': ['foo', 'bar', 'baz'],
|
||||
},
|
||||
],
|
||||
}],
|
||||
},
|
||||
),
|
||||
)
|
||||
def test_config_valid(config_obj, expected):
|
||||
ret = is_valid_according_to_schema(config_obj, CONFIG_SCHEMA)
|
||||
assert ret is expected
|
||||
def test_config_valid(cfg):
|
||||
assert is_valid_according_to_schema(cfg, CONFIG_SCHEMA)
|
||||
|
||||
|
||||
def test_invalid_config_wrong_type():
|
||||
cfg = {
|
||||
'repos': [{
|
||||
'repo': 'git@github.com:pre-commit/pre-commit-hooks',
|
||||
'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
|
||||
'hooks': [
|
||||
{
|
||||
'id': 'pyflakes',
|
||||
'files': '\\.py$',
|
||||
# Exclude pattern must be a string
|
||||
'exclude': 0,
|
||||
'args': ['foo', 'bar', 'baz'],
|
||||
},
|
||||
],
|
||||
}],
|
||||
}
|
||||
assert not is_valid_according_to_schema(cfg, CONFIG_SCHEMA)
|
||||
|
||||
|
||||
def test_local_hooks_with_rev_fails():
|
||||
|
|
@ -198,14 +193,13 @@ def test_warn_mutable_rev_conditional():
|
|||
),
|
||||
)
|
||||
def test_sensible_regex_validators_dont_pass_none(validator_cls):
|
||||
key = 'files'
|
||||
validator = validator_cls('files', cfgv.check_string)
|
||||
with pytest.raises(cfgv.ValidationError) as excinfo:
|
||||
validator = validator_cls(key, cfgv.check_string)
|
||||
validator.check({key: None})
|
||||
validator.check({'files': None})
|
||||
|
||||
assert str(excinfo.value) == (
|
||||
'\n'
|
||||
f'==> At key: {key}'
|
||||
'==> At key: files'
|
||||
'\n'
|
||||
'=====> Expected string got NoneType'
|
||||
)
|
||||
|
|
@ -298,46 +292,36 @@ def test_validate_optional_sensible_regex_at_top_level(caplog, regex, warning):
|
|||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
('manifest_obj', 'expected'),
|
||||
'manifest_obj',
|
||||
(
|
||||
(
|
||||
[{
|
||||
'id': 'a',
|
||||
'name': 'b',
|
||||
'entry': 'c',
|
||||
'language': 'python',
|
||||
'files': r'\.py$',
|
||||
}],
|
||||
True,
|
||||
),
|
||||
(
|
||||
[{
|
||||
'id': 'a',
|
||||
'name': 'b',
|
||||
'entry': 'c',
|
||||
'language': 'python',
|
||||
'language_version': 'python3.4',
|
||||
'files': r'\.py$',
|
||||
}],
|
||||
True,
|
||||
),
|
||||
(
|
||||
# A regression in 0.13.5: always_run and files are permissible
|
||||
[{
|
||||
'id': 'a',
|
||||
'name': 'b',
|
||||
'entry': 'c',
|
||||
'language': 'python',
|
||||
'files': '',
|
||||
'always_run': True,
|
||||
}],
|
||||
True,
|
||||
),
|
||||
[{
|
||||
'id': 'a',
|
||||
'name': 'b',
|
||||
'entry': 'c',
|
||||
'language': 'python',
|
||||
'files': r'\.py$',
|
||||
}],
|
||||
[{
|
||||
'id': 'a',
|
||||
'name': 'b',
|
||||
'entry': 'c',
|
||||
'language': 'python',
|
||||
'language_version': 'python3.4',
|
||||
'files': r'\.py$',
|
||||
}],
|
||||
# A regression in 0.13.5: always_run and files are permissible
|
||||
[{
|
||||
'id': 'a',
|
||||
'name': 'b',
|
||||
'entry': 'c',
|
||||
'language': 'python',
|
||||
'files': '',
|
||||
'always_run': True,
|
||||
}],
|
||||
),
|
||||
)
|
||||
def test_valid_manifests(manifest_obj, expected):
|
||||
ret = is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA)
|
||||
assert ret is expected
|
||||
def test_valid_manifests(manifest_obj):
|
||||
assert is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
|
|
@ -393,8 +377,39 @@ def test_parse_version():
|
|||
|
||||
|
||||
def test_minimum_pre_commit_version_failing():
|
||||
cfg = {'repos': [], 'minimum_pre_commit_version': '999'}
|
||||
with pytest.raises(cfgv.ValidationError) as excinfo:
|
||||
cfgv.validate(cfg, CONFIG_SCHEMA)
|
||||
assert str(excinfo.value) == (
|
||||
f'\n'
|
||||
f'==> At Config()\n'
|
||||
f'==> At key: minimum_pre_commit_version\n'
|
||||
f'=====> pre-commit version 999 is required but version {C.VERSION} '
|
||||
f'is installed. Perhaps run `pip install --upgrade pre-commit`.'
|
||||
)
|
||||
|
||||
|
||||
def test_minimum_pre_commit_version_failing_in_config():
|
||||
cfg = {'repos': [sample_local_config()]}
|
||||
cfg['repos'][0]['hooks'][0]['minimum_pre_commit_version'] = '999'
|
||||
with pytest.raises(cfgv.ValidationError) as excinfo:
|
||||
cfgv.validate(cfg, CONFIG_SCHEMA)
|
||||
assert str(excinfo.value) == (
|
||||
f'\n'
|
||||
f'==> At Config()\n'
|
||||
f'==> At key: repos\n'
|
||||
f"==> At Repository(repo='local')\n"
|
||||
f'==> At key: hooks\n'
|
||||
f"==> At Hook(id='do_not_commit')\n"
|
||||
f'==> At key: minimum_pre_commit_version\n'
|
||||
f'=====> pre-commit version 999 is required but version {C.VERSION} '
|
||||
f'is installed. Perhaps run `pip install --upgrade pre-commit`.'
|
||||
)
|
||||
|
||||
|
||||
def test_minimum_pre_commit_version_failing_before_other_error():
|
||||
cfg = {'repos': 5, 'minimum_pre_commit_version': '999'}
|
||||
with pytest.raises(cfgv.ValidationError) as excinfo:
|
||||
cfg = {'repos': [], 'minimum_pre_commit_version': '999'}
|
||||
cfgv.validate(cfg, CONFIG_SCHEMA)
|
||||
assert str(excinfo.value) == (
|
||||
f'\n'
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ from unittest import mock
|
|||
|
||||
import cfgv
|
||||
import pytest
|
||||
import re_assert
|
||||
|
||||
import pre_commit.constants as C
|
||||
from pre_commit import lang_base
|
||||
|
|
@ -27,7 +26,6 @@ from pre_commit.util import cmd_output
|
|||
from pre_commit.util import cmd_output_b
|
||||
from testing.fixtures import make_config_from_repo
|
||||
from testing.fixtures import make_repo
|
||||
from testing.fixtures import modify_manifest
|
||||
from testing.language_helpers import run_language
|
||||
from testing.util import cwd
|
||||
from testing.util import get_resource_path
|
||||
|
|
@ -433,32 +431,6 @@ def test_hook_id_not_present(tempdir_factory, store, caplog):
|
|||
)
|
||||
|
||||
|
||||
def test_too_new_version(tempdir_factory, store, caplog):
|
||||
path = make_repo(tempdir_factory, 'script_hooks_repo')
|
||||
with modify_manifest(path) as manifest:
|
||||
manifest[0]['minimum_pre_commit_version'] = '999.0.0'
|
||||
config = make_config_from_repo(path)
|
||||
with pytest.raises(SystemExit):
|
||||
_get_hook(config, store, 'bash_hook')
|
||||
_, msg = caplog.messages
|
||||
pattern = re_assert.Matches(
|
||||
r'^The hook `bash_hook` requires pre-commit version 999\.0\.0 but '
|
||||
r'version \d+\.\d+\.\d+ is installed. '
|
||||
r'Perhaps run `pip install --upgrade pre-commit`\.$',
|
||||
)
|
||||
pattern.assert_matches(msg)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('version', ('0.1.0', C.VERSION))
|
||||
def test_versions_ok(tempdir_factory, store, version):
|
||||
path = make_repo(tempdir_factory, 'script_hooks_repo')
|
||||
with modify_manifest(path) as manifest:
|
||||
manifest[0]['minimum_pre_commit_version'] = version
|
||||
config = make_config_from_repo(path)
|
||||
# Should succeed
|
||||
_get_hook(config, store, 'bash_hook')
|
||||
|
||||
|
||||
def test_manifest_hooks(tempdir_factory, store):
|
||||
path = make_repo(tempdir_factory, 'script_hooks_repo')
|
||||
config = make_config_from_repo(path)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue