ronantakizawa/codereview-bench huggingface dataset:
Repository: cookiecutter/cookiecutter
File: tests/test_cli.py (Python)

Code before:
        side_effect=cookiecutter
    )

    template_path = 'tests/fake-repo-pre/'
    result = runner.invoke(main, [
        template_path,
        '--no-input',
        '--replay',
        '-v'
    ])

    assert result.exit_code == 1

    expected_error_msg = (
        "You can not use both replay and no_input or extra_context "
        "at the same time."
    )
    assert expected_error_msg in result.output

    mock_cookiecutter.assert_called_once_with(
        template_path,
        None,
        True,
        replay=True
    )


Review comment (bug):
Then is it okay to rename the test to `test_cli_successful_exit_on_overwrite_if_exists_and_replay` or maybe `test_cli_does_not_exit_with_failure_on_overwrite_if_exists_and_replay`? :-)


Code after:
    result = runner.invoke(main, [
        template_path,
        '--no-input',
        '--replay',
        '-v'
    ])

    assert result.exit_code == 1

    expected_error_msg = (
        "You can not use both replay and no_input or extra_context "
        "at the same time."
    )

    assert expected_error_msg in result.output

    mock_cookiecutter.assert_called_once_with(
        template_path,
        None,
        True,
        replay=True,
        overwrite_if_exists=False
    )

@pytest.mark.usefixtures('remove_fake_project_dir')
def test_cli_exit_on_overwrite_if_exists_and_replay(mocker):
    mock_cookiecutter = mocker.patch(
        'cookiecutter.cli.cookiecutter',
        side_effect=cookiecutter
    )

    template_path = 'tests/fake-repo-pre/'
    result = runner.invoke(main, [
        template_path,
        '--replay',
        '-v',
        '-f',
    ])

    assert result.exit_code == 0

    mock_cookiecutter.assert_called_once_with(
        template_path,
        None,
        False,
        replay=True,
        overwrite_if_exists=True
    )


@pytest.mark.usefixtures('remove_fake_project_dir')
Repository: cookiecutter/cookiecutter
File: tests/repository/test_determine_repository_should_use_local_repo.py (Python)

Code before:
# -*- coding: utf-8 -*-
from cookiecutter.repository import determine_repo_dir


def test_determine_repository_should_use_local_repo():
    project_dir = determine_repo_dir(
        'tests/fake-repo',
        abbreviations={},
        clone_to_dir=None,
        checkout=None,
        no_input=True
    )

    assert 'tests/fake-repo' == project_dir


Review comment (suggestion):
Would be awesome to extend this to also check the error message:

``` python
with pytest.raises(exceptions.RepositoryNotFound) as excinfo:
    pass

expected_message = (
    'The repository tests/unknown-repo could not be located or does not contain '
    'a "cookiecutter.json" file.'
)
assert str(excinfo.value) == expected_message
```


Code after:
# -*- coding: utf-8 -*-
from cookiecutter import repository, exceptions

import pytest


def test_finds_local_repo():
    """A valid local repository should be returned."""
    project_dir = repository.determine_repo_dir(
        'tests/fake-repo',
        abbreviations={},
        clone_to_dir=None,
        checkout=None,
        no_input=True
    )

    assert 'tests/fake-repo' == project_dir


def test_local_repo_with_no_context_raises():
    """A local repository without a cookiecutter.json should raise a
    `RepositoryNotFound` exception.
    """
    with pytest.raises(exceptions.RepositoryNotFound) as err:
        repository.determine_repo_dir(
            'tests/fake-repo-bad',
            abbreviations={},
            clone_to_dir=None,
            checkout=None,
            no_input=True
        )

    assert str(err.value) == (
        'The repository tests/fake-repo-bad could not be located or does not '
        'contain a "cookiecutter.json" file.'
    )


def test_local_repo_typo():
    """An unknown local repository should raise a `RepositoryNotFound`
    exception.
    """
    with pytest.raises(exceptions.RepositoryNotFound) as err:
        repository.determine_repo_dir(
            'tests/unknown-repo',
            abbreviations={},
            clone_to_dir=None,
            checkout=None,
            no_input=True
        )

    assert str(err.value) == (
        'The repository tests/unknown-repo could not be located or does not '
        'contain a "cookiecutter.json" file.'
    )
Repository: cookiecutter/cookiecutter
File: tests/vcs/test_clone.py (Python)

Code before:
    branch = 'foobar'

    repo_dir = vcs.clone(
        repo_url,
        checkout=branch,
        clone_to_dir=clone_dir,
        no_input=True
    )

    assert repo_dir == expected_repo_dir

    mock_subprocess.assert_any_call(
        [repo_type, 'clone', repo_url],
        cwd=clone_dir,
        stderr=subprocess.STDOUT
    )
    mock_subprocess.assert_any_call(
        [repo_type, 'checkout', branch],
        cwd=expected_repo_dir,
        stderr=subprocess.STDOUT
    )


@pytest.mark.parametrize('error_message', [
    b"fatal: repository 'https://github.com/hackebro/cookiedozer' not found",
    b'hg: abort: HTTP Error 404: Not Found',
])
def test_clone_handles_repo_typo(mocker, clone_dir, error_message):
    """In `clone()`, repository not found errors should raise an
    appropriate exception.
    """
    # side_effect is set to an iterable here (and below),
    # because of a Python 3.4 unittest.mock regression
    # http://bugs.python.org/issue23661
    mocker.patch(
        'cookiecutter.vcs.subprocess.check_output',
        autospec=True,
        side_effect=[subprocess.CalledProcessError(
            -1, 'cmd', output=error_message
        )]
    )

    repository_url = 'https://github.com/hackebro/cookiedozer'
    with pytest.raises(exceptions.RepositoryNotFound) as err:
        vcs.clone(
            repository_url,
            clone_to_dir=clone_dir,
            no_input=True
        )

    assert str(err.value) == (

Review comment (suggestion):
`subprocess.CalledProcessError.output` won't be a bytestring in py2...

Code after:
    branch = 'foobar'

    repo_dir = vcs.clone(
        repo_url,
        checkout=branch,
        clone_to_dir=clone_dir,
        no_input=True
    )

    assert repo_dir == expected_repo_dir

    mock_subprocess.assert_any_call(
        [repo_type, 'clone', repo_url],
        cwd=clone_dir,
        stderr=subprocess.STDOUT
    )
    mock_subprocess.assert_any_call(
        [repo_type, 'checkout', branch],
        cwd=expected_repo_dir,
        stderr=subprocess.STDOUT
    )


@pytest.mark.parametrize('error_message', [
    (
        "fatal: repository 'https://github.com/hackebro/cookiedozer' "
        "not found"
    ).encode('utf-8'),
    'hg: abort: HTTP Error 404: Not Found'.encode('utf-8'),
])
def test_clone_handles_repo_typo(mocker, clone_dir, error_message):
    """In `clone()`, repository not found errors should raise an
    appropriate exception.
    """
    # side_effect is set to an iterable here (and below),
    # because of a Python 3.4 unittest.mock regression
    # http://bugs.python.org/issue23661
    mocker.patch(
        'cookiecutter.vcs.subprocess.check_output',
        autospec=True,
        side_effect=[subprocess.CalledProcessError(
            -1, 'cmd', output=error_message
        )]
    )

    repository_url = 'https://github.com/hackebro/cookiedozer'
    with pytest.raises(exceptions.RepositoryNotFound) as err:
        vcs.clone(
            repository_url,
            clone_to_dir=clone_dir,
            no_input=True
Repository: cookiecutter/cookiecutter
File: cookiecutter/extensions.py (Python)

Code before:
# -*- coding: utf-8 -*-

"""Jinja2 extensions."""

import json

from jinja2.ext import Extension


class JsonifyExtension(Extension):
    """Jinja2 extension to convert a Python object to JSON."""

    def __init__(self, environment):
        """Initialize the extension with the given environment."""
        super(JsonifyExtension, self).__init__(environment)

        def jsonify(obj):
            return json.dumps(obj, sort_keys=True, indent=4)

        environment.filters['jsonify'] = jsonify


Review comment (suggestion):
I do not like reassignments at all. And ``+=`` with strings.
Consider: 
```python
if punctuation:
    corpus = "".join((string.ascii_letters, string.punctuation))
else:
    corpus = string.ascii_letters
```



Code after:
    from random import choice

from jinja2.ext import Extension


class JsonifyExtension(Extension):
    """Jinja2 extension to convert a Python object to JSON."""

    def __init__(self, environment):
        """Initialize the extension with the given environment."""
        super(JsonifyExtension, self).__init__(environment)

        def jsonify(obj):
            return json.dumps(obj, sort_keys=True, indent=4)

        environment.filters['jsonify'] = jsonify


class RandomStringExtension(Extension):
    """Jinja2 extension to create a random string."""

    def __init__(self, environment):
        super(RandomStringExtension, self).__init__(environment)

        def random_ascii_string(length, punctuation=False):
            if punctuation:
                corpus = "".join((string.ascii_letters, string.punctuation))
            else:
                corpus = string.ascii_letters
            return "".join(choice(corpus) for _ in range(length))
        environment.globals.update(random_ascii_string=random_ascii_string)
Repository: cookiecutter/cookiecutter
File: tests/zipfile/test_unzip.py (Python)

Code before:
"""Tests for function unzip() from zipfile module."""
import tempfile

import pytest

from cookiecutter import zipfile
from cookiecutter.exceptions import InvalidZipRepository


@pytest.fixture
def clone_dir(tmp_path):
    """Simulate creation of a directory called `clone_dir` inside of `tmp_path`. \
    Returns a str to said directory."""
    clone_dir = tmp_path.joinpath("clone_dir")
    clone_dir.mkdir()
    return clone_dir


def mock_download():
    """Fake download function."""
    with open('tests/files/fake-repo-tmpl.zip', 'rb') as zf:
        chunk = zf.read(1024)
        while chunk:
            yield chunk
            chunk = zf.read(1024)


def test_unzip_local_file(mocker, clone_dir):
    """Local file reference can be unzipped."""
    mock_prompt_and_delete = mocker.patch(
        'cookiecutter.zipfile.prompt_and_delete', return_value=True, autospec=True
    )

    output_dir = zipfile.unzip(
        'tests/files/fake-repo-tmpl.zip', is_url=False, clone_to_dir=str(clone_dir)
    )

Review comment (suggestion):
hey @SharpEdgeMarshall 👋 , 

what do you think about moving this fixture in the `conftest.py` file to avoid fixtures duplication?

Thanks 🙏 

Code after:
"""Tests for function unzip() from zipfile module."""
import tempfile

import pytest

from cookiecutter import zipfile
from cookiecutter.exceptions import InvalidZipRepository


def mock_download():
    """Fake download function."""
    with open('tests/files/fake-repo-tmpl.zip', 'rb') as zf:
        chunk = zf.read(1024)
        while chunk:
            yield chunk
            chunk = zf.read(1024)


def test_unzip_local_file(mocker, clone_dir):
    """Local file reference can be unzipped."""
    mock_prompt_and_delete = mocker.patch(
        'cookiecutter.zipfile.prompt_and_delete', return_value=True, autospec=True
    )

    output_dir = zipfile.unzip(
        'tests/files/fake-repo-tmpl.zip', is_url=False, clone_to_dir=str(clone_dir)
    )

    assert output_dir.startswith(tempfile.gettempdir())
    assert not mock_prompt_and_delete.called


def test_unzip_protected_local_file_environment_password(mocker, clone_dir):
    """In `unzip()`, the environment can be used to provide a repo password."""
    mock_prompt_and_delete = mocker.patch(
        'cookiecutter.zipfile.prompt_and_delete', return_value=True, autospec=True
Repository: cookiecutter/cookiecutter
File: tests/test_custom_extensions_in_hooks.py (Python)

Code before:
"""
test_custom_extension_in_hooks.

Tests to ensure custom cookiecutter extensions are properly made available to
pre- and post-gen hooks.
"""
import codecs
import os

import pytest

from cookiecutter import main


@pytest.fixture(
    params=['custom-extension-pre', 'custom-extension-post'],
    ids=['pre_gen_hook', 'post_gen_hook'],
)
def template(request):
    """Fixture. Allows to split pre and post hooks test directories."""
    return 'tests/test-extensions/' + request.param


@pytest.fixture
def output_dir(tmp_path):
    """Fixture. Create and return custom temp directory for test."""
    output_path = tmp_path.joinpath("hello")
    output_path.mkdir()
    return str(output_path)


@pytest.fixture(autouse=True)
def modify_syspath(monkeypatch):
    """Fixture. Make sure that the custom extension can be loaded."""
    monkeypatch.syspath_prepend('tests/test-extensions/hello_extension')


def test_hook_with_extension(template, output_dir):
    """Verify custom Jinja2 extension correctly work in hooks and file rendering.

    Each file in hooks has simple tests inside and will raise error if not
    correctly rendered.
    """
    project_dir = main.cookiecutter(
        template,
        no_input=True,
        output_dir=output_dir,
        extra_context={'project_slug': 'foobar', 'name': 'Cookiemonster'},
    )


Review comment (suggestion):
We should consider moving this fixture to the base `conftest.py`, to avoid fixture duplication.

Code after:
"""
test_custom_extension_in_hooks.

Tests to ensure custom cookiecutter extensions are properly made available to
pre- and post-gen hooks.
"""
import codecs
import os

import pytest

from cookiecutter import main


@pytest.fixture(
    params=['custom-extension-pre', 'custom-extension-post'],
    ids=['pre_gen_hook', 'post_gen_hook'],
)
def template(request):
    """Fixture. Allows to split pre and post hooks test directories."""
    return 'tests/test-extensions/' + request.param


@pytest.fixture(autouse=True)
def modify_syspath(monkeypatch):
    """Fixture. Make sure that the custom extension can be loaded."""
    monkeypatch.syspath_prepend('tests/test-extensions/hello_extension')


def test_hook_with_extension(template, output_dir):
    """Verify custom Jinja2 extension correctly work in hooks and file rendering.

    Each file in hooks has simple tests inside and will raise error if not
    correctly rendered.
    """
    project_dir = main.cookiecutter(
        template,
        no_input=True,
        output_dir=output_dir,
        extra_context={'project_slug': 'foobar', 'name': 'Cookiemonster'},
    )

    readme_file = os.path.join(project_dir, 'README.rst')

    with codecs.open(readme_file, encoding='utf8') as f:
        readme = f.read().strip()

    assert readme == 'Hello Cookiemonster!'
Repository: cookiecutter/cookiecutter
File: tests/test_templates.py (Python)

Code before:

import pytest

from cookiecutter import main


@pytest.fixture
def output_dir(tmpdir):
    """Fixture. Create and return custom temp directory for test."""
    return str(tmpdir.mkdir('templates'))


@pytest.mark.parametrize("template", ["include", "no-templates", "extends", "super"])
def test_build_templates(template, output_dir):
    """
    Verify Templates Design keywords.

    no-templates is a compatibility tests for repo without `templates` directory
    """
    project_dir = main.cookiecutter(
        f'tests/test-templates/{template}',
        no_input=True,
        output_dir=output_dir,
    )

    readme = Path(project_dir, 'requirements.txt').read_text(encoding='utf-8')

    assert readme.splitlines() == [
        "pip",
        "Click",
        "pytest",
    ]


Review comment (suggestion):
```suggestion
    readme = Path(project_dir, 'requirements.txt').read_text()
```

Code after:

import pytest

from cookiecutter import main


@pytest.fixture
def output_dir(tmpdir):
    """Fixture. Create and return custom temp directory for test."""
    return str(tmpdir.mkdir('templates'))


@pytest.mark.parametrize("template", ["include", "no-templates", "extends", "super"])
def test_build_templates(template, output_dir):
    """
    Verify Templates Design keywords.

    no-templates is a compatibility tests for repo without `templates` directory
    """
    project_dir = main.cookiecutter(
        f'tests/test-templates/{template}',
        no_input=True,
        output_dir=output_dir,
    )

    readme = Path(project_dir, 'requirements.txt').read_text()

    assert readme.splitlines() == [
        "pip",
        "Click",
        "pytest",
    ]
Repository: cookiecutter/cookiecutter
File: tests/test_custom_extensions_in_hooks.py (Python)

Code before:
)
def template(request):
    """Fixture. Allows to split pre and post hooks test directories."""
    return 'tests/test-extensions/' + request.param


@pytest.fixture(autouse=True)
def modify_syspath(monkeypatch):
    """Fixture. Make sure that the custom extension can be loaded."""
    monkeypatch.syspath_prepend('tests/test-extensions/hello_extension')


def test_hook_with_extension(template, output_dir):
    """Verify custom Jinja2 extension correctly work in hooks and file rendering.

    Each file in hooks has simple tests inside and will raise error if not
    correctly rendered.
    """
    project_dir = main.cookiecutter(
        template,
        no_input=True,
        output_dir=output_dir,
        extra_context={'project_slug': 'foobar', 'name': 'Cookiemonster'},
    )

    readme = Path(project_dir, 'README.rst').read_text(encoding="utf8")
    assert readme.strip() == 'Hello Cookiemonster!'


Review comment (suggestion):
```suggestion
    readme = Path(project_dir, 'README.rst').read_text(encoding="utf-8")
```

Code after:
)
def template(request):
    """Fixture. Allows to split pre and post hooks test directories."""
    return f"tests/test-extensions/{request.param}"


@pytest.fixture(autouse=True)
def modify_syspath(monkeypatch):
    """Fixture. Make sure that the custom extension can be loaded."""
    monkeypatch.syspath_prepend('tests/test-extensions/hello_extension')


def test_hook_with_extension(template, output_dir):
    """Verify custom Jinja2 extension correctly work in hooks and file rendering.

    Each file in hooks has simple tests inside and will raise error if not
    correctly rendered.
    """
    project_dir = main.cookiecutter(
        template,
        no_input=True,
        output_dir=output_dir,
        extra_context={'project_slug': 'foobar', 'name': 'Cookiemonster'},
    )

    readme = Path(project_dir, 'README.rst').read_text(encoding="utf-8")
    assert readme.strip() == 'Hello Cookiemonster!'
Repository: cookiecutter/cookiecutter
File: tests/repository/test_determine_repo_dir_finds_existing_cookiecutter.py (Python)

Code before:
"""Tests around detection whether cookiecutter templates are cached locally."""
import os
from pathlib import Path

import pytest

from cookiecutter import repository


@pytest.fixture
def template():
    """Fixture. Return simple string as template name."""
    return 'cookiecutter-pytest-plugin'


@pytest.fixture
def cloned_cookiecutter_path(user_config_data, template):
    """Fixture. Create fake project directory in special user folder."""
    cookiecutters_dir = user_config_data['cookiecutters_dir']

    cloned_template_path = os.path.join(cookiecutters_dir, template)
    os.mkdir(cloned_template_path)

    Path(cloned_template_path, "cookiecutter.json").open('w')  # creates file

    return cloned_template_path


def test_should_find_existing_cookiecutter(
    template, user_config_data, cloned_cookiecutter_path
):
    """
    Should find folder created by `cloned_cookiecutter_path` and return it.

    This folder is considered like previously cloned project directory.
    """
    project_dir, cleanup = repository.determine_repo_dir(
        template=template,
        abbreviations={},
        clone_to_dir=user_config_data['cookiecutters_dir'],
        checkout=None,
        no_input=True,
    )

    assert cloned_cookiecutter_path == project_dir
    assert not cleanup


Review comment (suggestion):
```suggestion
    Path(cloned_template_path, "cookiecutter.json").touch()  # creates file
```

Code after:
"""Tests around detection whether cookiecutter templates are cached locally."""
import os
from pathlib import Path

import pytest

from cookiecutter import repository


@pytest.fixture
def template():
    """Fixture. Return simple string as template name."""
    return 'cookiecutter-pytest-plugin'


@pytest.fixture
def cloned_cookiecutter_path(user_config_data, template):
    """Fixture. Create fake project directory in special user folder."""
    cookiecutters_dir = user_config_data['cookiecutters_dir']

    cloned_template_path = os.path.join(cookiecutters_dir, template)
    os.mkdir(cloned_template_path)

    Path(cloned_template_path, "cookiecutter.json").touch()  # creates file

    return cloned_template_path


def test_should_find_existing_cookiecutter(
    template, user_config_data, cloned_cookiecutter_path
):
    """
    Should find folder created by `cloned_cookiecutter_path` and return it.

    This folder is considered like previously cloned project directory.
    """
    project_dir, cleanup = repository.determine_repo_dir(
        template=template,
        abbreviations={},
        clone_to_dir=user_config_data['cookiecutters_dir'],
        checkout=None,
        no_input=True,
    )

    assert cloned_cookiecutter_path == project_dir
    assert not cleanup