ronantakizawa/codereview-bench huggingface dataset:
Repository: django/django
File: scripts/check_migrations.py (Python)

Code before:
from django.conf import settings
from django.core.management import call_command


def main():
    repo_root = Path(__file__).resolve().parent.parent
    sys.path[:0] = [str(repo_root / "tests"), str(repo_root)]
    from runtests import ALWAYS_INSTALLED_APPS, get_apps_to_install, get_test_modules

    if not settings.configured:
        settings.configure(
            INSTALLED_APPS=ALWAYS_INSTALLED_APPS,
            MIGRATION_MODULES={},
        )
        django.setup()

    test_modules = list(get_test_modules(gis_enabled=False))
    installed_apps = list(ALWAYS_INSTALLED_APPS)
    for app in get_apps_to_install(test_modules):
        # Check against the list to prevent duplicate error
        if app not in installed_apps:
            installed_apps.append(app)
    apps.set_installed_apps(installed_apps)

    try:
        call_command("makemigrations", "--check", verbosity=1)
    except Exception as e:
        print(f"Error checking migrations: {str(e)}")


if __name__ == "__main__":
    main()


Review comment (suggestion):
Running this locally I got unexpected success because the database was `django.db.backends.dummy`.

Code after:
    repo_root = Path(__file__).resolve().parent.parent
    sys.path[:0] = [str(repo_root / "tests"), str(repo_root)]

    from runtests import ALWAYS_INSTALLED_APPS, get_apps_to_install, get_test_modules

    import django
    from django.apps import apps
    from django.core.management import call_command

    django.setup()

    test_modules = list(get_test_modules(gis_enabled=False))
    installed_apps = list(ALWAYS_INSTALLED_APPS)
    for app in get_apps_to_install(test_modules):
        # Check against the list to prevent duplicate errors.
        if app not in installed_apps:
            installed_apps.append(app)
    apps.set_installed_apps(installed_apps)

    # Note: We don't use check=True here because --check calls sys.exit(1)
    # instead of raising CommandError when migrations are missing.
    call_command("makemigrations", "--check", verbosity=3)


if __name__ == "__main__":
    main()
Repository: django/django
File: scripts/check_migrations.py (Python)

Code before:
from django.conf import settings
from django.core.management import call_command


def main():
    repo_root = Path(__file__).resolve().parent.parent
    sys.path[:0] = [str(repo_root / "tests"), str(repo_root)]
    from runtests import ALWAYS_INSTALLED_APPS, get_apps_to_install, get_test_modules

    if not settings.configured:
        settings.configure(
            INSTALLED_APPS=ALWAYS_INSTALLED_APPS,
            MIGRATION_MODULES={},
        )
        django.setup()

    test_modules = list(get_test_modules(gis_enabled=False))
    installed_apps = list(ALWAYS_INSTALLED_APPS)
    for app in get_apps_to_install(test_modules):
        # Check against the list to prevent duplicate error
        if app not in installed_apps:
            installed_apps.append(app)
    apps.set_installed_apps(installed_apps)

    try:
        call_command("makemigrations", "--check", verbosity=1)
    except Exception as e:
        print(f"Error checking migrations: {str(e)}")


if __name__ == "__main__":
    main()


Review comment (suggestion):
I'd add a comment that the reason we aren't using `check=True` is that `--check` directly calls `sys.exit(1)` instead of raising `CommandError`, which was a little unintuitive for me.

Code after:
    repo_root = Path(__file__).resolve().parent.parent
    sys.path[:0] = [str(repo_root / "tests"), str(repo_root)]

    from runtests import ALWAYS_INSTALLED_APPS, get_apps_to_install, get_test_modules

    import django
    from django.apps import apps
    from django.core.management import call_command

    django.setup()

    test_modules = list(get_test_modules(gis_enabled=False))
    installed_apps = list(ALWAYS_INSTALLED_APPS)
    for app in get_apps_to_install(test_modules):
        # Check against the list to prevent duplicate errors.
        if app not in installed_apps:
            installed_apps.append(app)
    apps.set_installed_apps(installed_apps)

    # Note: We don't use check=True here because --check calls sys.exit(1)
    # instead of raising CommandError when migrations are missing.
    call_command("makemigrations", "--check", verbosity=3)


if __name__ == "__main__":
    main()
Repository: django/django
File: scripts/check_migrations.py (Python)

Code before:
import sys
from pathlib import Path


def main():
    repo_root = Path(__file__).resolve().parent.parent
    sys.path[:0] = [str(repo_root / "tests"), str(repo_root)]

    from runtests import ALWAYS_INSTALLED_APPS, get_apps_to_install, get_test_modules

    import django
    from django.apps import apps
    from django.core.management import call_command

    django.setup()

    test_modules = list(get_test_modules(gis_enabled=False))
    installed_apps = list(ALWAYS_INSTALLED_APPS)
    for app in get_apps_to_install(test_modules):
        # Check against the list to prevent duplicate error
        if app not in installed_apps:
            installed_apps.append(app)
    apps.set_installed_apps(installed_apps)

    # Note: We don't use check=True here because --check calls sys.exit(1)
    # instead of raising CommandError when migrations are missing
    call_command("makemigrations", "--check", verbosity=1)


if __name__ == "__main__":
    main()


Review comment (bug):
I think we should run with `gis_enabled=True`, as I have several more failures:

```py
Migrations for 'gis_migrations':
  tests/gis_tests/gis_migrations/migrations/0003_remove_household_family_delete_heatmap_and_more.py
    - Remove field family from household
    - Delete model Heatmap
    - Remove field neighborhood from household
    - Delete model Family
    - Delete model Household
    - Delete model Neighborhood
Migrations for 'rasterapp':
  tests/gis_tests/rasterapp/migrations/0003_alter_rastermodel_id_alter_rasterrelatedmodel_id.py
    ~ Alter field id on rastermodel
    ~ Alter field id on rasterrelatedmodel
```

You could borrow the CI config in the `postgis.yml`.

Code after:
import sys
from pathlib import Path


def main():
    repo_root = Path(__file__).resolve().parent.parent
    sys.path[:0] = [str(repo_root / "tests"), str(repo_root)]

    from runtests import ALWAYS_INSTALLED_APPS, get_apps_to_install, get_test_modules

    import django
    from django.apps import apps
    from django.core.management import call_command

    django.setup()

    test_modules = list(get_test_modules(gis_enabled=False))
    installed_apps = list(ALWAYS_INSTALLED_APPS)
    for app in get_apps_to_install(test_modules):
        # Check against the list to prevent duplicate errors.
        if app not in installed_apps:
            installed_apps.append(app)
    apps.set_installed_apps(installed_apps)

    # Note: We don't use check=True here because --check calls sys.exit(1)
    # instead of raising CommandError when migrations are missing.
    call_command("makemigrations", "--check", verbosity=3)


if __name__ == "__main__":
    main()
Repository: django/django
File: scripts/check_migrations.py (Python)

Code before:
from pathlib import Path


def main():
    repo_root = Path(__file__).resolve().parent.parent
    sys.path[:0] = [str(repo_root / "tests"), str(repo_root)]

    from runtests import ALWAYS_INSTALLED_APPS, get_apps_to_install, get_test_modules

    import django
    from django.apps import apps
    from django.core.management import call_command

    django.setup()

    test_modules = list(get_test_modules(gis_enabled=False))
    installed_apps = list(ALWAYS_INSTALLED_APPS)
    for app in get_apps_to_install(test_modules):
        # Check against the list to prevent duplicate error
        if app not in installed_apps:
            installed_apps.append(app)
    apps.set_installed_apps(installed_apps)

    # Note: We don't use check=True here because --check calls sys.exit(1)
    # instead of raising CommandError when migrations are missing
    call_command("makemigrations", "--check", verbosity=1)


if __name__ == "__main__":
    main()


Review comment (bug):
verbosity=3 would be a nice convenience, so that when this fails on CI you don't have to re-run it manually to get the changes.

Code after:
from pathlib import Path


def main():
    repo_root = Path(__file__).resolve().parent.parent
    sys.path[:0] = [str(repo_root / "tests"), str(repo_root)]

    from runtests import ALWAYS_INSTALLED_APPS, get_apps_to_install, get_test_modules

    import django
    from django.apps import apps
    from django.core.management import call_command

    django.setup()

    test_modules = list(get_test_modules(gis_enabled=False))
    installed_apps = list(ALWAYS_INSTALLED_APPS)
    for app in get_apps_to_install(test_modules):
        # Check against the list to prevent duplicate errors.
        if app not in installed_apps:
            installed_apps.append(app)
    apps.set_installed_apps(installed_apps)

    # Note: We don't use check=True here because --check calls sys.exit(1)
    # instead of raising CommandError when migrations are missing.
    call_command("makemigrations", "--check", verbosity=3)


if __name__ == "__main__":
    main()
Repository: django/django
File: tests/sessions_tests/tests.py (Python)

Code before:
        msg = self.not_implemented_msg % "a save"
        with self.assertRaisesMessage(NotImplementedError, msg):
            await self.session.asave()

    def test_test_cookie(self):
        self.assertIs(self.session.has_key(self.session.TEST_COOKIE_NAME), False)
        self.session.set_test_cookie()
        self.assertIs(self.session.test_cookie_worked(), True)
        self.session.delete_test_cookie()
        self.assertIs(self.session.has_key(self.session.TEST_COOKIE_NAME), False)

    async def test_atest_cookie(self):
        self.assertIs(await self.session.ahas_key(self.session.TEST_COOKIE_NAME), False)
        await self.session.aset_test_cookie()
        self.assertIs(await self.session.atest_cookie_worked(), True)
        await self.session.adelete_test_cookie()
        self.assertIs(await self.session.ahas_key(self.session.TEST_COOKIE_NAME), False)

    def test_is_empty(self):
        self.assertIs(self.session.is_empty(), True)

    def test_bool(self):
        # Empty session is falsy
        self.assertIs(bool(self.session), False)
        # Session with data is truthy
        self.session._session_cache = {"foo": "bar"}
        self.assertIs(bool(self.session), True)


Review comment (performance):
Issue: Best to use the public APIs for mutating the state here, rather than modifying the cache directly.

Code after:
        msg = self.not_implemented_msg % "a save"
        with self.assertRaisesMessage(NotImplementedError, msg):
            await self.session.asave()

    def test_test_cookie(self):
        self.assertIs(self.session.has_key(self.session.TEST_COOKIE_NAME), False)
        self.session.set_test_cookie()
        self.assertIs(self.session.test_cookie_worked(), True)
        self.session.delete_test_cookie()
        self.assertIs(self.session.has_key(self.session.TEST_COOKIE_NAME), False)

    async def test_atest_cookie(self):
        self.assertIs(await self.session.ahas_key(self.session.TEST_COOKIE_NAME), False)
        await self.session.aset_test_cookie()
        self.assertIs(await self.session.atest_cookie_worked(), True)
        await self.session.adelete_test_cookie()
        self.assertIs(await self.session.ahas_key(self.session.TEST_COOKIE_NAME), False)

    def test_is_empty(self):
        self.assertIs(self.session.is_empty(), True)

    def test_bool(self):
        # Empty session is falsy
        self.assertIs(bool(self.session), False)
        # Session with data is truthy
        self.session["foo"] = "bar"
        self.assertIs(bool(self.session), True)
        # Session with key but no data is truthy
        session_with_key = SessionBase()
        session_with_key._session_key = "testkey1234"
        self.assertIs(bool(session_with_key), True)
Repository: django/django
File: tests/admin_inlines/tests.py (Python)

Code before:
    def test_tabular_inline_with_filter_horizontal(self):
        from selenium.webdriver.common.by import By

        self.admin_login(username="super", password="secret")
        self.selenium.get(
            self.live_server_url + reverse("admin:admin_inlines_courseproxy2_add")
        )
        m2m_widget = self.selenium.find_element(By.CSS_SELECTOR, "div.selector")
        self.assertTrue(m2m_widget.is_displayed())
        self.take_screenshot("tabular")

    @screenshot_cases(["desktop_size", "mobile_size", "rtl", "dark", "high_contrast"])
    def test_tabular_inline_m2m_widget_option_bg(self):
        from selenium.webdriver.common.by import By

        Person.objects.create(firstname="Lee")
        self.admin_login(username="super", password="secret")
        self.selenium.get(
            self.live_server_url + reverse("admin:admin_inlines_courseproxy2_add")
        )
        selector = self.selenium.find_element(By.CSS_SELECTOR, "div.selector")
        options = selector.find_elements(By.CSS_SELECTOR, "select option")
        self.assertGreater(len(options), 0)
        options[0].click()
        selector.find_element(By.CSS_SELECTOR, "p.selector-filter input").click()
        self.take_screenshot("foucus_out")


Review comment (suggestion):
```suggestion
        self.take_screenshot("focus_out")
```

Code after:
    def test_tabular_inline_with_filter_horizontal(self):
        from selenium.webdriver.common.by import By

        self.admin_login(username="super", password="secret")
        self.selenium.get(
            self.live_server_url + reverse("admin:admin_inlines_courseproxy2_add")
        )
        m2m_widget = self.selenium.find_element(By.CSS_SELECTOR, "div.selector")
        self.assertTrue(m2m_widget.is_displayed())
        self.take_screenshot("tabular")

    @screenshot_cases(["desktop_size", "mobile_size", "rtl", "dark", "high_contrast"])
    def test_tabular_inline_m2m_widget_option_bg(self):
        from selenium.webdriver.common.by import By

        Person.objects.create(firstname="Lee")
        self.admin_login(username="super", password="secret")
        self.selenium.get(
            self.live_server_url + reverse("admin:admin_inlines_courseproxy2_add")
        )
        selector = self.selenium.find_element(By.CSS_SELECTOR, "div.selector")
        options = selector.find_elements(By.CSS_SELECTOR, "select option")
        self.assertGreater(len(options), 0)
        options[0].click()
        selector.find_element(By.CSS_SELECTOR, "p.selector-filter input").click()
        self.take_screenshot("focus_out")
Repository: django/django
File: tests/admin_views/test_password_form.py (Python)

Code before:

        # The warning about password being lost is shown.
        warning = self.selenium.find_element(By.ID, "id_unusable_warning")
        self.assertIs(warning.is_displayed(), True)

        # Click to enable password-based authentication.
        pw_switch_on.click()

        # The warning disappears.
        self.assertIs(warning.is_displayed(), False)

        # The password fields are shown.
        self.assertIs(password1.is_displayed(), True)
        self.assertIs(password2.is_displayed(), True)

        # Only the set password submit button is visible.
        self.assertIs(submit_set.is_displayed(), True)
        self.assertIs(submit_unset.is_displayed(), False)

    @screenshot_cases(["desktop_size", "mobile_size", "rtl", "dark", "high_contrast"])
    def test_add_user_alignment(self):
        """
        Ensures that radio buttons and labels are properly aligned. A CSS
        regression on .wide legend would produce a visibly misaligned
        screenshot.
        """
        user_add_url = reverse("auth_test_admin:auth_user_add")
        self.admin_login(username="super", password="secret")
        self.selenium.get(self.live_server_url + user_add_url)

        self.take_screenshot("add_user")

Review comment (suggestion):
Please reword to avoid preambles like "ensures that". The test title should probably mention radio button field alignment, not the add user view used in the test..

Code after:

        # The warning about password being lost is shown.
        warning = self.selenium.find_element(By.ID, "id_unusable_warning")
        self.assertIs(warning.is_displayed(), True)

        # Click to enable password-based authentication.
        pw_switch_on.click()

        # The warning disappears.
        self.assertIs(warning.is_displayed(), False)

        # The password fields are shown.
        self.assertIs(password1.is_displayed(), True)
        self.assertIs(password2.is_displayed(), True)

        # Only the set password submit button is visible.
        self.assertIs(submit_set.is_displayed(), True)
        self.assertIs(submit_unset.is_displayed(), False)

    @screenshot_cases(["desktop_size", "mobile_size", "rtl", "dark", "high_contrast"])
    def test_fieldset_legend_wide_alignment(self):
        user_add_url = reverse("auth_test_admin:auth_user_add")
        self.admin_login(username="super", password="secret")
        self.selenium.get(self.live_server_url + user_add_url)

        # The fieldset legend is aligned with other fields.
        self.take_screenshot("fieldset_legend_wide")
Repository: django/django
File: django/utils/deprecation.py (Python)

Code before:
import functools
import inspect
import os
import warnings
from collections import Counter
from inspect import iscoroutinefunction, markcoroutinefunction

from asgiref.sync import sync_to_async

import django
from django.utils.inspect import signature


@functools.cache
def django_file_prefixes():
    try:
        file = django.__file__
    except AttributeError:
        return ()
    if file is None:
        return ()
    return (os.path.dirname(file),)


class RemovedInNextVersionWarning(DeprecationWarning):
    pass


class RemovedInDjango70Warning(PendingDeprecationWarning):
    pass


RemovedAfterNextVersionWarning = RemovedInDjango70Warning


class warn_about_renamed_method:
    def __init__(
        self, class_name, old_method_name, new_method_name, deprecation_warning
    ):
        self.class_name = class_name
        self.old_method_name = old_method_name
        self.new_method_name = new_method_name
        self.deprecation_warning = deprecation_warning

Review comment (suggestion):
Interesting, I guess this means that these environements will have _leaky_ warnings (pointing at Django internals) but that's not a huge deal.

Code after:
import functools
import inspect
import os
import warnings
from collections import Counter
from inspect import iscoroutinefunction, markcoroutinefunction

from asgiref.sync import sync_to_async

import django
from django.utils.inspect import signature


@functools.cache
def django_file_prefixes():
    file = getattr(django, "__file__", None)
    if file is None:
        return ()
    return (os.path.dirname(file),)


class RemovedInNextVersionWarning(DeprecationWarning):
    pass


class RemovedInDjango70Warning(PendingDeprecationWarning):
    pass


RemovedAfterNextVersionWarning = RemovedInDjango70Warning


class warn_about_renamed_method:
    def __init__(
        self, class_name, old_method_name, new_method_name, deprecation_warning
    ):
        self.class_name = class_name
        self.old_method_name = old_method_name
        self.new_method_name = new_method_name
        self.deprecation_warning = deprecation_warning

    def __call__(self, f):
        def wrapper(*args, **kwargs):
Repository: django/django
File: django/utils/deprecation.py (Python)

Code before:
import functools
import inspect
import os
import warnings
from collections import Counter
from inspect import iscoroutinefunction, markcoroutinefunction

from asgiref.sync import sync_to_async

import django
from django.utils.inspect import signature


@functools.cache
def django_file_prefixes():
    try:
        file = django.__file__
    except AttributeError:
        return ()
    if file is None:
        return ()
    return (os.path.dirname(file),)


class RemovedInNextVersionWarning(DeprecationWarning):
    pass


class RemovedInDjango70Warning(PendingDeprecationWarning):
    pass


RemovedAfterNextVersionWarning = RemovedInDjango70Warning


class warn_about_renamed_method:
    def __init__(
        self, class_name, old_method_name, new_method_name, deprecation_warning
    ):
        self.class_name = class_name
        self.old_method_name = old_method_name
        self.new_method_name = new_method_name
        self.deprecation_warning = deprecation_warning

    def __call__(self, f):
        def wrapper(*args, **kwargs):

Review comment (suggestion):
I would rather we do something like this:
```suggestion
    file = getattr(django, "__file__", None)
    if file is None:
        return ()
```
If you don't object, I can push this tiny change during the afternoon.

Code after:
import functools
import inspect
import os
import warnings
from collections import Counter
from inspect import iscoroutinefunction, markcoroutinefunction

from asgiref.sync import sync_to_async

import django
from django.utils.inspect import signature


@functools.cache
def django_file_prefixes():
    file = getattr(django, "__file__", None)
    if file is None:
        return ()
    return (os.path.dirname(file),)


class RemovedInNextVersionWarning(DeprecationWarning):
    pass


class RemovedInDjango70Warning(PendingDeprecationWarning):
    pass


RemovedAfterNextVersionWarning = RemovedInDjango70Warning


class warn_about_renamed_method:
    def __init__(
        self, class_name, old_method_name, new_method_name, deprecation_warning
    ):
        self.class_name = class_name
        self.old_method_name = old_method_name
        self.new_method_name = new_method_name
        self.deprecation_warning = deprecation_warning

    def __call__(self, f):
        def wrapper(*args, **kwargs):
            warnings.warn(
                "`%s.%s` is deprecated, use `%s` instead."
                % (self.class_name, self.old_method_name, self.new_method_name),