Repository: pre-commit/pre-commit
Pull request #2705 (2023-01-17 19:16:24)

speed up R unit tests


Patch:
diff --git a/pre_commit/languages/r.py b/pre_commit/languages/r.py
index 7ed3eaf..dc39860 100644
--- a/pre_commit/languages/r.py
+++ b/pre_commit/languages/r.py
@@ -64,7 +64,7 @@ def _entry_validate(entry: list[str]) -> None:
             raise ValueError('You can supply at most one expression.')
     elif len(entry) > 2:
         raise ValueError(
-            'The only valid syntax is `Rscript -e {expr}`',
+            'The only valid syntax is `Rscript -e {expr}`'
             'or `Rscript path/to/hook/script`',
         )
 


Test patch:
diff --git a/testing/resources/r_hooks_repo/.pre-commit-hooks.yaml b/testing/resources/r_hooks_repo/.pre-commit-hooks.yaml
index b3545d9..ef46cc0 100644
--- a/testing/resources/r_hooks_repo/.pre-commit-hooks.yaml
+++ b/testing/resources/r_hooks_repo/.pre-commit-hooks.yaml
@@ -1,26 +1,3 @@
-# parsing file
--   id: parse-file-no-opts-no-args
-    name: Say hi
-    entry: Rscript parse-file-no-opts-no-args.R
-    language: r
-    types: [r]
--   id: parse-file-no-opts-args
-    name: Say hi
-    entry: Rscript parse-file-no-opts-args.R
-    args: [--no-cache]
-    language: r
-    types: [r]
-## parsing expr
--   id: parse-expr-no-opts-no-args-1
-    name: Say hi
-    entry: Rscript -e '1+1'
-    language: r
-    types: [r]
--   id: parse-expr-args-in-entry-2
-    name: Say hi
-    entry: Rscript -e '1+1' -e '3' --no-cache3
-    language: r
-    types: [r]
 # real world
 -   id: hello-world
     name: Say hi
diff --git a/tests/languages/r_test.py b/tests/languages/r_test.py
index d234414..0c5e563 100644
--- a/tests/languages/r_test.py
+++ b/tests/languages/r_test.py
@@ -6,117 +6,86 @@ import pytest
 
 from pre_commit import envcontext
 from pre_commit.languages import r
+from pre_commit.prefix import Prefix
 from pre_commit.util import win_exe
-from testing.fixtures import make_config_from_repo
-from testing.fixtures import make_repo
-from tests.repository_test import _get_hook_no_install
-
-
-def _test_r_parsing(
-    tempdir_factory,
-    store,
-    hook_id,
-    expected_hook_expr=(),
-    expected_args=(),
-    config=None,
-):
-    repo = make_repo(tempdir_factory, 'r_hooks_repo')
-    config = make_config_from_repo(repo)
-    hook = _get_hook_no_install(config, store, hook_id)
-    ret = r._cmd_from_hook(hook.prefix, hook.entry, hook.args)
-    expected_path = os.path.join(hook.prefix.prefix_dir, f'{hook_id}.R')
-    expected = (
+
+
+def test_r_parsing_file_no_opts_no_args(tmp_path):
+    cmd = r._cmd_from_hook(Prefix(str(tmp_path)), 'Rscript some-script.R', ())
+    assert cmd == (
         'Rscript',
         '--no-save', '--no-restore', '--no-site-file', '--no-environ',
-        *(expected_hook_expr or (expected_path,)),
-        *expected_args,
+        str(tmp_path.joinpath('some-script.R')),
     )
-    assert ret == expected
-
-
-def test_r_parsing_file_no_opts_no_args(tempdir_factory, store):
-    hook_id = 'parse-file-no-opts-no-args'
-    _test_r_parsing(tempdir_factory, store, hook_id)
 
 
-def test_r_parsing_file_opts_no_args(tempdir_factory, store):
+def test_r_parsing_file_opts_no_args():
     with pytest.raises(ValueError) as excinfo:
         r._entry_validate(['Rscript', '--no-init', '/path/to/file'])
 
-    msg = excinfo.value.args
+    msg, = excinfo.value.args
     assert msg == (
-        'The only valid syntax is `Rscript -e {expr}`',
-        'or `Rscript path/to/hook/script`',
+        'The only valid syntax is `Rscript -e {expr}`'
+        'or `Rscript path/to/hook/script`'
     )
 
 
-def test_r_parsing_file_no_opts_args(tempdir_factory, store):
-    hook_id = 'parse-file-no-opts-args'
-    expected_args = ['--no-cache']
-    _test_r_parsing(
-        tempdir_factory, store, hook_id, expected_args=expected_args,
+def test_r_parsing_file_no_opts_args(tmp_path):
+    cmd = r._cmd_from_hook(
+        Prefix(str(tmp_path)),
+        'Rscript some-script.R',
+        ('--no-cache',),
+    )
+    assert cmd == (
+        'Rscript',
+        '--no-save', '--no-restore', '--no-site-file', '--no-environ',
+        str(tmp_path.joinpath('some-script.R')),
+        '--no-cache',
     )
 
 
-def test_r_parsing_expr_no_opts_no_args1(tempdir_factory, store):
-    hook_id = 'parse-expr-no-opts-no-args-1'
-    _test_r_parsing(
-        tempdir_factory, store, hook_id, expected_hook_expr=('-e', '1+1'),
+def test_r_parsing_expr_no_opts_no_args1(tmp_path):
+    cmd = r._cmd_from_hook(Prefix(str(tmp_path)), "Rscript -e '1+1'", ())
+    assert cmd == (
+        'Rscript',
+        '--no-save', '--no-restore', '--no-site-file', '--no-environ',
+        '-e', '1+1',
     )
 
 
-def test_r_parsing_expr_no_opts_no_args2(tempdir_factory, store):
-    with pytest.raises(ValueError) as execinfo:
+def test_r_parsing_expr_no_opts_no_args2():
+    with pytest.raises(ValueError) as excinfo:
         r._entry_validate(['Rscript', '-e', '1+1', '-e', 'letters'])
-    msg = execinfo.value.args
-    assert msg == ('You can supply at most one expression.',)
+    msg, = excinfo.value.args
+    assert msg == 'You can supply at most one expression.'
 
 
-def test_r_parsing_expr_opts_no_args2(tempdir_factory, store):
-    with pytest.raises(ValueError) as execinfo:
+def test_r_parsing_expr_opts_no_args2():
+    with pytest.raises(ValueError) as excinfo:
         r._entry_validate(
             ['Rscript', '--vanilla', '-e', '1+1', '-e', 'letters'],
         )
-    msg = execinfo.value.args
+    msg, = excinfo.value.args
     assert msg == (
-        'The only valid syntax is `Rscript -e {expr}`',
-        'or `Rscript path/to/hook/script`',
+        'The only valid syntax is `Rscript -e {expr}`'
+        'or `Rscript path/to/hook/script`'
     )
 
 
-def test_r_parsing_expr_args_in_entry2(tempdir_factory, store):
-    with pytest.raises(ValueError) as execinfo:
+def test_r_parsing_expr_args_in_entry2():
+    with pytest.raises(ValueError) as excinfo:
         r._entry_validate(['Rscript', '-e', 'expr1', '--another-arg'])
 
-    msg = execinfo.value.args
-    assert msg == ('You can supply at most one expression.',)
+    msg, = excinfo.value.args
+    assert msg == 'You can supply at most one expression.'
 
 
-def test_r_parsing_expr_non_Rscirpt(tempdir_factory, store):
-    with pytest.raises(ValueError) as execinfo:
+def test_r_parsing_expr_non_Rscirpt():
+    with pytest.raises(ValueError) as excinfo:
         r._entry_validate(['AnotherScript', '-e', '{{}}'])
 
-    msg = execinfo.value.args
-    assert msg == ('entry must start with `Rscript`.',)
-
-
-def test_r_parsing_file_local(tempdir_factory, store):
-    config = {
-        'repo': 'local',
-        'hooks': [{
-            'id': 'local-r',
-            'name': 'local-r',
-            'entry': 'Rscript path/to/script.R',
-            'language': 'r',
-        }],
-    }
-    hook = _get_hook_no_install(config, store, 'local-r')
-    ret = r._cmd_from_hook(hook.prefix, hook.entry, hook.args)
-    assert ret == (
-        'Rscript',
-        '--no-save', '--no-restore', '--no-site-file', '--no-environ',
-        hook.prefix.path('path/to/script.R'),
-    )
+    msg, = excinfo.value.args
+    assert msg == 'entry must start with `Rscript`.'
 
 
 def test_rscript_exec_relative_to_r_home():