Merge pull request #2808 from pre-commit/hook-stage-and-type-match · hack3ric/pre-commit@f2661bf · GitHub
Skip to content

Commit f2661bf

Browse files
asottileasottile
authored
Merge pull request pre-commit#2808 from pre-commit/hook-stage-and-type-match
make --hook-type and stages match
2 parents 02e9680 + e3e17a1 commit f2661bf

10 files changed

Lines changed: 147 additions & 42 deletions

File tree

‎pre_commit/clientlib.py‎

Lines changed: 60 additions & 7 deletions

‎pre_commit/commands/hook_impl.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def _ns(
8484
) -> argparse.Namespace:
8585
return argparse.Namespace(
8686
color=color,
87-
hook_stage=hook_type.replace('pre-', ''),
87+
hook_stage=hook_type,
8888
remote_branch=remote_branch,
8989
local_branch=local_branch,
9090
from_ref=from_ref,

‎pre_commit/constants.py‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,4 @@
1010

1111
VERSION = importlib.metadata.version('pre_commit')
1212

13-
# `manual` is not invoked by any installed git hook. See #719
14-
STAGES = (
15-
'commit', 'merge-commit', 'prepare-commit-msg', 'commit-msg',
16-
'post-commit', 'manual', 'post-checkout', 'push', 'post-merge',
17-
'post-rewrite',
18-
)
19-
20-
HOOK_TYPES = (
21-
'pre-commit', 'pre-merge-commit', 'pre-push', 'prepare-commit-msg',
22-
'commit-msg', 'post-commit', 'post-checkout', 'post-merge',
23-
'post-rewrite',
24-
)
25-
2613
DEFAULT = 'default'

‎pre_commit/main.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from typing import Sequence
88

99
import pre_commit.constants as C
10+
from pre_commit import clientlib
1011
from pre_commit import git
1112
from pre_commit.color import add_color_option
1213
from pre_commit.commands.autoupdate import autoupdate
@@ -52,7 +53,7 @@ def _add_config_option(parser: argparse.ArgumentParser) -> None:
5253
def _add_hook_type_option(parser: argparse.ArgumentParser) -> None:
5354
parser.add_argument(
5455
'-t', '--hook-type',
55-
choices=C.HOOK_TYPES, action='append', dest='hook_types',
56+
choices=clientlib.HOOK_TYPES, action='append', dest='hook_types',
5657
)
5758

5859

@@ -73,7 +74,10 @@ def _add_run_options(parser: argparse.ArgumentParser) -> None:
7374
help='When hooks fail, run `git diff` directly afterward.',
7475
)
7576
parser.add_argument(
76-
'--hook-stage', choices=C.STAGES, default='commit',
77+
'--hook-stage',
78+
choices=clientlib.STAGES,
79+
type=clientlib.transform_stage,
80+
default='pre-commit',
7781
help='The stage during which the hook is fired. One of %(choices)s',
7882
)
7983
parser.add_argument(

‎testing/util.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def run_opts(
4646
to_ref='',
4747
remote_name='',
4848
remote_url='',
49-
hook_stage='commit',
49+
hook_stage='pre-commit',
5050
show_diff_on_failure=False,
5151
commit_msg_filename='',
5252
prepare_commit_message_source='',

‎tests/clientlib_test.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from pre_commit.clientlib import CONFIG_REPO_DICT
1313
from pre_commit.clientlib import CONFIG_SCHEMA
1414
from pre_commit.clientlib import DEFAULT_LANGUAGE_VERSION
15+
from pre_commit.clientlib import MANIFEST_HOOK_DICT
1516
from pre_commit.clientlib import MANIFEST_SCHEMA
1617
from pre_commit.clientlib import META_HOOK_DICT
1718
from pre_commit.clientlib import OptionalSensibleRegexAtHook
@@ -416,3 +417,50 @@ def test_warn_additional(schema):
416417
x for x in schema.items if isinstance(x, cfgv.WarnAdditionalKeys)
417418
)
418419
assert allowed_keys == set(warn_additional.keys)
420+
421+
422+
def test_stages_migration_for_default_stages():
423+
cfg = {
424+
'default_stages': ['commit-msg', 'push', 'commit', 'merge-commit'],
425+
'repos': [],
426+
}
427+
cfgv.validate(cfg, CONFIG_SCHEMA)
428+
cfg = cfgv.apply_defaults(cfg, CONFIG_SCHEMA)
429+
assert cfg['default_stages'] == [
430+
'commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit',
431+
]
432+
433+
434+
def test_manifest_stages_defaulting():
435+
dct = {
436+
'id': 'fake-hook',
437+
'name': 'fake-hook',
438+
'entry': 'fake-hook',
439+
'language': 'system',
440+
'stages': ['commit-msg', 'push', 'commit', 'merge-commit'],
441+
}
442+
cfgv.validate(dct, MANIFEST_HOOK_DICT)
443+
dct = cfgv.apply_defaults(dct, MANIFEST_HOOK_DICT)
444+
assert dct['stages'] == [
445+
'commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit',
446+
]
447+
448+
449+
def test_config_hook_stages_defaulting_missing():
450+
dct = {'id': 'fake-hook'}
451+
cfgv.validate(dct, CONFIG_HOOK_DICT)
452+
dct = cfgv.apply_defaults(dct, CONFIG_HOOK_DICT)
453+
assert dct == {'id': 'fake-hook'}
454+
455+
456+
def test_config_hook_stages_defaulting():
457+
dct = {
458+
'id': 'fake-hook',
459+
'stages': ['commit-msg', 'push', 'commit', 'merge-commit'],
460+
}
461+
cfgv.validate(dct, CONFIG_HOOK_DICT)
462+
dct = cfgv.apply_defaults(dct, CONFIG_HOOK_DICT)
463+
assert dct == {
464+
'id': 'fake-hook',
465+
'stages': ['commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit'],
466+
}

‎tests/commands/hook_impl_test.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ def test_check_args_length_prepare_commit_msg_error():
142142
def test_run_ns_pre_commit():
143143
ns = hook_impl._run_ns('pre-commit', True, (), b'')
144144
assert ns is not None
145-
assert ns.hook_stage == 'commit'
145+
assert ns.hook_stage == 'pre-commit'
146146
assert ns.color is True
147147

148148

@@ -245,7 +245,7 @@ def test_run_ns_pre_push_updating_branch(push_example):
245245
ns = hook_impl._run_ns('pre-push', False, args, stdin)
246246

247247
assert ns is not None
248-
assert ns.hook_stage == 'push'
248+
assert ns.hook_stage == 'pre-push'
249249
assert ns.color is False
250250
assert ns.remote_name == 'origin'
251251
assert ns.remote_url == src

‎tests/commands/run_test.py‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -354,13 +354,13 @@ def test_show_diff_on_failure(
354354
({'hook': 'bash_hook'}, (b'Bash hook', b'Passed'), 0, True),
355355
(
356356
{'hook': 'nope'},
357-
(b'No hook with id `nope` in stage `commit`',),
357+
(b'No hook with id `nope` in stage `pre-commit`',),
358358
1,
359359
True,
360360
),
361361
(
362-
{'hook': 'nope', 'hook_stage': 'push'},
363-
(b'No hook with id `nope` in stage `push`',),
362+
{'hook': 'nope', 'hook_stage': 'pre-push'},
363+
(b'No hook with id `nope` in stage `pre-push`',),
364364
1,
365365
True,
366366
),
@@ -818,7 +818,7 @@ def test_stages(cap_out, store, repo_with_passing_hook):
818818
'language': 'pygrep',
819819
'stages': [stage],
820820
}
821-
for i, stage in enumerate(('commit', 'push', 'manual'), 1)
821+
for i, stage in enumerate(('pre-commit', 'pre-push', 'manual'), 1)
822822
],
823823
}
824824
add_config_to_repo(repo_with_passing_hook, config)
@@ -833,8 +833,8 @@ def _run_for_stage(stage):
833833
assert printed.count(b'hook ') == 1
834834
return printed
835835

836-
assert _run_for_stage('commit').startswith(b'hook 1...')
837-
assert _run_for_stage('push').startswith(b'hook 2...')
836+
assert _run_for_stage('pre-commit').startswith(b'hook 1...')
837+
assert _run_for_stage('pre-push').startswith(b'hook 2...')
838838
assert _run_for_stage('manual').startswith(b'hook 3...')
839839

840840

@@ -1173,7 +1173,7 @@ def test_args_hook_only(cap_out, store, repo_with_passing_hook):
11731173
),
11741174
'language': 'system',
11751175
'files': r'\.py$',
1176-
'stages': ['commit'],
1176+
'stages': ['pre-commit'],
11771177
},
11781178
{
11791179
'id': 'do_not_commit',

‎tests/main_test.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,9 @@ def test_expected_fatal_error_no_git_repo(in_tmpdir, cap_out, mock_store_dir):
216216
'Is it installed, and are you in a Git repository directory?'
217217
)
218218
assert cap_out_lines[-1] == f'Check the log at {log_file}'
219+
220+
221+
def test_hook_stage_migration(mock_store_dir):
222+
with mock.patch.object(main, 'run') as mck:
223+
main.main(('run', '--hook-stage', 'commit'))
224+
assert mck.call_args[0][2].hook_stage == 'pre-commit'

‎tests/repository_test.py‎

Lines changed: 16 additions & 9 deletions

0 commit comments

Comments
 (0)