ronantakizawa/codereview-bench huggingface dataset:
Repository: pytest-dev/pytest
File: testing/python/fixtures.py (Python)

Code before:
        out2 = pytester.runpytest_subprocess("-v")
        output1 = [
            line
            for line in out1.outlines
            if line.startswith("test_deterministic_fixture_collection.py::test_foo")
        ]
        output2 = [
            line
            for line in out2.outlines
            if line.startswith("test_deterministic_fixture_collection.py::test_foo")
        ]
        assert len(output1) == 12
        assert output1 == output2


class FixtureFunctionDefTestClass:
    def __init__(self) -> None:
        self.i = 10

    @pytest.fixture
    def fixture_function_def_test_method(self):
        return self.i


@pytest.fixture
def fixture_function_def_test_func():
    return 9


def test_get_wrapped_func_returns_method():
    obj = FixtureFunctionDefTestClass()
    wrapped_function_result = (
        obj.fixture_function_def_test_method._get_wrapped_function()
    )
    assert inspect.ismethod(wrapped_function_result)
    assert wrapped_function_result() == 10


def test_get_wrapped_func_returns_function():
    assert fixture_function_def_test_func._get_wrapped_function()() == 9


class TestRequestScopeAccess:
    pytestmark = pytest.mark.parametrize(
        ("scope", "ok", "error"),
        [
            ["session", "", "path class function module"],
            ["module", "module path", "cls function"],
            ["class", "module path cls", "function"],
            ["function", "module path cls function", ""],
        ],

Review comment (suggestion):
I would write tests that register fixtures as pytester tests, to avoid "polluting" the actual pytest-testsuite pytest session.

Code after:
                return 1

            def test_something(test_app):
                assert test_app == 1
        """
        )
        reprec = pytester.inline_run()
        reprec.assertoutcome(passed=1)

    def test_params_and_ids(self, pytester: Pytester) -> None:
        pytester.makepyfile(
            """
            import pytest

            @pytest.fixture(params=[object(), object()],
                            ids=['alpha', 'beta'])
            def fix(request):
                return request.param

            def test_foo(fix):
                assert 1
        """
        )
        res = pytester.runpytest("-v")
        res.stdout.fnmatch_lines(["*test_foo*alpha*", "*test_foo*beta*"])

    def test_params_and_ids_yieldfixture(self, pytester: Pytester) -> None:
        pytester.makepyfile(
            """
            import pytest

            @pytest.fixture(params=[object(), object()], ids=['alpha', 'beta'])
            def fix(request):
                 yield request.param

            def test_foo(fix):
                assert 1
        """
        )
        res = pytester.runpytest("-v")
        res.stdout.fnmatch_lines(["*test_foo*alpha*", "*test_foo*beta*"])

    def test_deterministic_fixture_collection(
        self, pytester: Pytester, monkeypatch
    ) -> None:
        """#920"""
        pytester.makepyfile(
            """
            import pytest

            @pytest.fixture(scope="module",
Repository: pytest-dev/pytest
File: src/_pytest/mark/expression.py (Python)

Code before:
r"""Evaluate match expressions, as used by `-k` and `-m`.

The grammar is:

expression: expr? EOF
expr:       and_expr ('or' and_expr)*
and_expr:   not_expr ('and' not_expr)*
not_expr:   'not' not_expr | '(' expr ')' | ident
ident:      (\w|:|\+|-|\.|\[|\]|\\|/)+

The semantics are:

- Empty expression evaluates to False.
- ident evaluates to True of False according to a provided matcher function.
- or/and/not evaluate according to the usual boolean semantics.
"""

from __future__ import annotations

import ast
import dataclasses
import enum
import re
import types
from typing import Callable
from typing import Iterator
from typing import Mapping
from typing import NoReturn
from typing import Sequence


__all__ = [
    "Expression",

Review comment (suggestion):
- Would remove the space after `ident (` for consistency.
- The `*` (zero-or-more) at the end of this line should be `?` (zero-or-one), I think.
- The definitions of the `name` and `value` productions are missing.

Code after:
r"""Evaluate match expressions, as used by `-k` and `-m`.

The grammar is:

expression: expr? EOF
expr:       and_expr ('or' and_expr)*
and_expr:   not_expr ('and' not_expr)*
not_expr:   'not' not_expr | '(' expr ')' | ident ( '(' name '=' value ( ', ' name '=' value )*  ')')*

ident:      (\w|:|\+|-|\.|\[|\]|\\|/)+

The semantics are:

- Empty expression evaluates to False.
- ident evaluates to True of False according to a provided matcher function.
- or/and/not evaluate according to the usual boolean semantics.
"""

from __future__ import annotations

import ast
import dataclasses
import enum
import keyword
import re
import types
from typing import Iterator
from typing import Literal
from typing import Mapping
from typing import NoReturn
from typing import overload
from typing import Protocol
from typing import Sequence
Repository: pytest-dev/pytest
File: src/_pytest/mark/expression.py (Python)

Code before:
r"""Evaluate match expressions, as used by `-k` and `-m`.

The grammar is:

expression: expr? EOF
expr:       and_expr ('or' and_expr)*
and_expr:   not_expr ('and' not_expr)*
not_expr:   'not' not_expr | '(' expr ')' | ident ('(' name '=' value ( ', ' name '=' value )*  ')')?

ident:      (\w|:|\+|-|\.|\[|\]|\\|/)+
name:       a valid ident, but not a reserved keyword
value:      (unescaped) str | int | bool | None

The semantics are:

- Empty expression evaluates to False.
- ident evaluates to True or False according to a provided matcher function.
- or/and/not evaluate according to the usual boolean semantics.
- ident with parentheses and keyword arguments evaluates to True or False according to a provided matcher function.
"""

from __future__ import annotations

import ast
import dataclasses
import enum
import keyword
import re
import types
from typing import Iterator
from typing import Literal
from typing import Mapping
from typing import NoReturn
from typing import overload
from typing import Protocol
from typing import Sequence


Review comment (security):
```suggestion
value:      (unescaped) string literal | (-)?[0-9]+ | 'False' | 'True' | 'None'
```

(Even though `isdigit` is not exactly `[0-9]` I think we shouldn't promise beyond that)

Code after:
r"""Evaluate match expressions, as used by `-k` and `-m`.

The grammar is:

expression: expr? EOF
expr:       and_expr ('or' and_expr)*
and_expr:   not_expr ('and' not_expr)*
not_expr:   'not' not_expr | '(' expr ')' | ident kwargs?

ident:      (\w|:|\+|-|\.|\[|\]|\\|/)+
kwargs:     ('(' name '=' value ( ', ' name '=' value )*  ')')
name:       a valid ident, but not a reserved keyword
value:      (unescaped) string literal | (-)?[0-9]+ | 'False' | 'True' | 'None'

The semantics are:

- Empty expression evaluates to False.
- ident evaluates to True or False according to a provided matcher function.
- or/and/not evaluate according to the usual boolean semantics.
- ident with parentheses and keyword arguments evaluates to True or False according to a provided matcher function.
"""

from __future__ import annotations

import ast
import dataclasses
import enum
import keyword
import re
import types
from typing import Iterator
from typing import Literal
from typing import Mapping
from typing import NoReturn
from typing import overload
from typing import Protocol
from typing import Sequence
Repository: pytest-dev/pytest
File: src/_pytest/mark/expression.py (Python)

Code before:
r"""Evaluate match expressions, as used by `-k` and `-m`.

The grammar is:

expression: expr? EOF
expr:       and_expr ('or' and_expr)*
and_expr:   not_expr ('and' not_expr)*
not_expr:   'not' not_expr | '(' expr ')' | ident ('(' name '=' value ( ', ' name '=' value )*  ')')?

ident:      (\w|:|\+|-|\.|\[|\]|\\|/)+
name:       a valid ident, but not a reserved keyword
value:      (unescaped) str | int | bool | None

The semantics are:

- Empty expression evaluates to False.
- ident evaluates to True or False according to a provided matcher function.
- or/and/not evaluate according to the usual boolean semantics.
- ident with parentheses and keyword arguments evaluates to True or False according to a provided matcher function.
"""

from __future__ import annotations

import ast
import dataclasses
import enum
import keyword
import re
import types
from typing import Iterator
from typing import Literal
from typing import Mapping
from typing import NoReturn
from typing import overload
from typing import Protocol
from typing import Sequence


Review comment (suggestion):
i wonder if 
```suggestion
not_expr:   'not' not_expr | '(' expr ')' | ident kwargs?

ident:      (\w|:|\+|-|\.|\[|\]|\\|/)+
kwargs: ('(' name '=' value ( ', ' name '=' value )*  ')')
name:       a valid ident, but not a reserved keyword
```

helps with readability

Code after:
r"""Evaluate match expressions, as used by `-k` and `-m`.

The grammar is:

expression: expr? EOF
expr:       and_expr ('or' and_expr)*
and_expr:   not_expr ('and' not_expr)*
not_expr:   'not' not_expr | '(' expr ')' | ident kwargs?

ident:      (\w|:|\+|-|\.|\[|\]|\\|/)+
kwargs:     ('(' name '=' value ( ', ' name '=' value )*  ')')
name:       a valid ident, but not a reserved keyword
value:      (unescaped) string literal | (-)?[0-9]+ | 'False' | 'True' | 'None'

The semantics are:

- Empty expression evaluates to False.
- ident evaluates to True or False according to a provided matcher function.
- or/and/not evaluate according to the usual boolean semantics.
- ident with parentheses and keyword arguments evaluates to True or False according to a provided matcher function.
"""

from __future__ import annotations

import ast
import dataclasses
import enum
import keyword
import re
import types
from typing import Iterator
from typing import Literal
from typing import Mapping
from typing import NoReturn
from typing import overload
from typing import Protocol
from typing import Sequence
Repository: pytest-dev/pytest
File: src/_pytest/python_api.py (Python)

Code before:
        self.excinfo: _pytest._code.ExceptionInfo[E] | None = None

    def __enter__(self) -> _pytest._code.ExceptionInfo[E]:
        self.excinfo = _pytest._code.ExceptionInfo.for_later()
        return self.excinfo

    def __exit__(
        self,
        exc_type: type[BaseException] | None,
        exc_val: BaseException | None,
        exc_tb: TracebackType | None,
    ) -> bool:
        __tracebackhide__ = True
        if exc_type is None:
            fail(self.message)
        assert self.excinfo is not None
        if not issubclass(exc_type, self.expected_exception):
            return False
        # Cast to narrow the exception type now that it's verified.
        exc_info = cast(Tuple[Type[E], E, TracebackType], (exc_type, exc_val, exc_tb))
        self.excinfo.fill_unfilled(exc_info)
        try:
            if self.match_expr is not None:
                self.excinfo.match(self.match_expr)
        except re.error as e:
            fail(f"Invalid regex pattern provided to 'match': {e}")
        return True


Review comment (bug):
Let's minimize the scope of the `try`:

```suggestion
        if self.match_expr is not None:
            try:
                self.excinfo.match(self.match_expr)
            except re.error as e:
                fail(f"Invalid regex pattern provided to 'match': {e}")
```

Code after:
        self.excinfo: _pytest._code.ExceptionInfo[E] | None = None
        if self.match_expr is not None:
            re_error = None
            try:
                re.compile(self.match_expr)
            except re.error as e:
                re_error = e
            if re_error is not None:
                fail(f"Invalid regex pattern provided to 'match': {re_error}")

    def __enter__(self) -> _pytest._code.ExceptionInfo[E]:
        self.excinfo = _pytest._code.ExceptionInfo.for_later()
        return self.excinfo

    def __exit__(
        self,
        exc_type: type[BaseException] | None,
        exc_val: BaseException | None,
        exc_tb: TracebackType | None,
    ) -> bool:
        __tracebackhide__ = True
        if exc_type is None:
            fail(self.message)
        assert self.excinfo is not None
        if not issubclass(exc_type, self.expected_exception):
            return False
        # Cast to narrow the exception type now that it's verified.
        exc_info = cast(Tuple[Type[E], E, TracebackType], (exc_type, exc_val, exc_tb))
        self.excinfo.fill_unfilled(exc_info)
        if self.match_expr is not None:
            self.excinfo.match(self.match_expr)
        return True
Repository: pytest-dev/pytest
File: src/_pytest/terminal.py (Python)

Code before:
def pytest_addoption(parser: Parser) -> None:
    group = parser.getgroup("terminal reporting", "Reporting", after="general")
    group._addoption(
        "-v",
        "--verbose",
        action="count",
        default=0,
        dest="verbose",
        help="Increase verbosity",
    )
    group._addoption(
        "--no-header",
        action="store_true",
        default=False,
        dest="no_header",
        help="Disable header",
    )
    group._addoption(
        "--no-summary",
        action="store_true",
        default=False,
        dest="no_summary",
        help="Disable summary",
    )
    group._addoption(
        "--fold-skipped",
        action="store",
        dest="foldskipped",
        choices=["no", "yes"],
        default="yes",
        help="Whether to fold skipped tests in short summary or not. " "Default: yes.",
    )
    group._addoption(
        "-q",
        "--quiet",
        action=MoreQuietAction,
        default=0,
        dest="verbose",
        help="Decrease verbosity",
    )
    group._addoption(
        "--verbosity",
        dest="verbose",
        type=int,
        default=0,
        help="Set verbosity. Default: 0.",
    )
    group._addoption(
        "-r",
        action="store",
        dest="reportchars",

Review comment (suggestion):
if we don't plan to to add more variants, lets go with `--no-fold-skipped` as a store false, default true action following  the way flags work in tools like click

Code after:
def pytest_addoption(parser: Parser) -> None:
    group = parser.getgroup("terminal reporting", "Reporting", after="general")
    group._addoption(
        "-v",
        "--verbose",
        action="count",
        default=0,
        dest="verbose",
        help="Increase verbosity",
    )
    group._addoption(
        "--no-header",
        action="store_true",
        default=False,
        dest="no_header",
        help="Disable header",
    )
    group._addoption(
        "--no-summary",
        action="store_true",
        default=False,
        dest="no_summary",
        help="Disable summary",
    )
    group._addoption(
        "--no-fold-skipped",
        action="store_false",
        dest="fold_skipped",
        default=True,
        help="Do not fold skipped tests in short summary.",
    )
    group._addoption(
        "-q",
        "--quiet",
        action=MoreQuietAction,
        default=0,
        dest="verbose",
        help="Decrease verbosity",
    )
    group._addoption(
        "--verbosity",
        dest="verbose",
        type=int,
        default=0,
        help="Set verbosity. Default: 0.",
    )
    group._addoption(
        "-r",
        action="store",
        dest="reportchars",
        default=_REPORTCHARS_DEFAULT,
Repository: pytest-dev/pytest
File: testing/acceptance_test.py (Python)

Code before:
            "*test_async.py::test_1*",
            "*test_async.py::test_2*",
            "*test_async.py::test_3*",
        ]
    )
    result.assert_outcomes(failed=3)


def test_error_on_sync_test_async_fixture(pytester: Pytester) -> None:
    pytester.makepyfile(
        test_sync="""
            import pytest

            @pytest.fixture
            async def async_fixture():
                ...

            def test_foo(async_fixture):
                ...
        """
    )
    result = pytester.runpytest()
    result.stdout.fnmatch_lines(
        [
            (
                "*ERROR: Sync test requested async fixture. "
                "You may want to make the test asynchronous and run it with "
                "a suitable async framework test plugin.*"
            ),
            "*ERROR test_sync.py::test_foo*",
        ]
    )
    result.assert_outcomes(errors=1)


def test_error_on_sync_test_async_fixture_gen(pytester: Pytester) -> None:
    pytester.makepyfile(
        test_sync="""
            import pytest

            @pytest.fixture
            async def async_fixture():
                yield

            def test_foo(async_fixture):
                ...
        """
    )
    result = pytester.runpytest()
    result.stdout.fnmatch_lines(
        [

Review comment (suggestion):
As I commented, I think improving the message here is important:

```suggestion
                "*ERROR: Sync test 'test_foo' requested async fixture 'async_fixture'. "
```

Code after:
            "*test_async.py::test_1*",
            "*test_async.py::test_2*",
            "*test_async.py::test_3*",
        ]
    )
    result.assert_outcomes(failed=3)


def test_warning_on_sync_test_async_fixture(pytester: Pytester) -> None:
    pytester.makepyfile(
        test_sync="""
            import pytest

            @pytest.fixture
            async def async_fixture():
                ...

            def test_foo(async_fixture):
                # suppress unawaited coroutine warning
                try:
                    async_fixture.send(None)
                except StopIteration:
                    pass
        """
    )
    result = pytester.runpytest()
    result.stdout.fnmatch_lines(
        [
            "*== warnings summary ==*",
            (
                "*PytestRemovedIn9Warning: 'test_foo' requested an async "
                "fixture 'async_fixture', with no plugin or hook that handled it. "
                "This is usually an error, as pytest does not natively support it. "
                "This will turn into an error in pytest 9."
            ),
            "  See: https://docs.pytest.org/en/stable/deprecations.html#sync-test-depending-on-async-fixture",
        ]
    )
    result.assert_outcomes(passed=1, warnings=1)


def test_warning_on_sync_test_async_fixture_gen(pytester: Pytester) -> None:
    pytester.makepyfile(
        test_sync="""
            import pytest

            @pytest.fixture
            async def async_fixture():
                yield

            def test_foo(async_fixture):
Repository: pytest-dev/pytest
File: testing/test_unraisableexception.py (Python)

Code before:

        def test_it():
            obj = BrokenDel()
            del obj
            {"import gc; gc.collect()" * PYPY}

        def test_2(): pass
        """
    )

    class MyError(BaseException):
        pass

    with mock.patch("traceback.format_exception", side_effect=MyError):
        result = pytester.runpytest()
    assert result.ret == 1
    assert result.parseoutcomes() == {"passed": 1, "failed": 1}
    result.stdout.fnmatch_lines(
        ["E               RuntimeError: Failed to process unraisable exception"]
    )


def test_create_task_unraisable(pytester: Pytester) -> None:
    pytester.makepyfile(
        test_it="""
        import pytest

        class BrokenDel:
            def __init__(self):
                self.self = self  # make a reference cycle

            def __del__(self):
                raise ValueError("del is broken")

        def test_it():
            BrokenDel()
        """
    )

    was_enabled = gc.isenabled()
    gc.disable()
    try:
        result = pytester.runpytest()
    finally:
        if was_enabled:
            gc.enable()

    # TODO: should be a test failure or error
    assert result.ret == pytest.ExitCode.INTERNAL_ERROR

    assert result.parseoutcomes() == {"passed": 1}

Review comment (suggestion):
```suggestion
def test_create_task_unraisable(pytester: Pytester) -> None:
    # see: https://github.com/pytest-dev/pytest/issues/10404
```

Code after:
        class BrokenDel:
            def __del__(self):
                raise ValueError("del is broken")

        def test_it():
            obj = BrokenDel()
            del obj
            {"import gc; gc.collect()" * PYPY}

        def test_2(): pass
        """
    )

    class MyError(BaseException):
        pass

    with mock.patch("traceback.format_exception", side_effect=MyError):
        result = pytester.runpytest()
    assert result.ret == 1
    result.assert_outcomes(passed=1, failed=1)
    result.stdout.fnmatch_lines(
        ["E               RuntimeError: Failed to process unraisable exception"]
    )


def test_create_task_unraisable(pytester: Pytester) -> None:
    # see: https://github.com/pytest-dev/pytest/issues/10404
    pytester.makepyfile(
        test_it="""
        import pytest

        class BrokenDel:
            def __init__(self):
                self.self = self  # make a reference cycle

            def __del__(self):
                raise ValueError("del is broken")

        def test_it():
            BrokenDel()
        """
    )

    was_enabled = gc.isenabled()
    gc.disable()
    try:
        result = pytester.runpytest()
    finally:
        if was_enabled:
            gc.enable()