Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move testapp out of tests/ and rename main module #189

Merged
merged 1 commit into from
Feb 27, 2025
Merged

Conversation

rhysyngsun
Copy link
Collaborator

@rhysyngsun rhysyngsun commented Feb 26, 2025

What are the relevant tickets?

Part of https://github.com/mitodl/hq/issues/6584

Description (What does it do?)

I've been hitting issues in #187 with path resolution of modules. It appears to be due in part of having testapp under tests while also having tests/mitol be an implicit package. This removes that implicit package and instead makes the tests/ directory have the same structure as the src/ directory. It also renames the testapp app to main to match our modern projects and moves it into its own dedicated directory at the root of the project. This makes the delineation between actual tests code and the test app used in those tests more clear.

How can this be tested?

  • Tests should pass.
  • Run uv run testapp/manage.py showmigrations, you should see the expected output.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @rhysyngsun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request addresses issues with module path resolution by moving the testapp directory out of the tests/ directory and renaming the main module to main. This change aims to resolve conflicts arising from having testapp under tests while tests/mitol is an implicit package. The PR also updates various settings and imports to reflect the new location and name of the main application module.

Highlights

  • Directory Restructuring: The testapp directory has been moved out of the tests/ directory to the root level.
  • Module Renaming: The main module has been renamed from testapp to main.
  • Settings Updates: Django settings files have been updated to reflect the new module name and location.
  • Import Adjustments: Imports throughout the codebase have been updated to point to the correct module locations.

Changelog

Click here to see the changelog
  • .secrets.baseline
    • Updated file paths to reflect the move of testapp and renaming to main.
    • Updated line number for secret in tests/conftest.py from 120 to 123.
  • pyproject.toml
    • Updated DJANGO_SETTINGS_MODULE to main.settings.test.
    • Added testapp to pythonpath.
    • Updated django_settings_module under tool.django-stubs to main.settings.test.
  • testapp/main/README.md
    • Updated imports from testapp.factories to main.factories and testapp.messages to main.messages.
  • testapp/main/admin.py
    • Updated import from testapp.models to main.models.
  • testapp/main/factories.py
    • Updated import from testapp.models to main.models.
  • testapp/main/migrations/0001_initial.py
    • Updated ManyToManyField to point to main.SecondLevel2 instead of testapp.SecondLevel2.
    • Updated ForeignKey to point to main.SecondLevel1 instead of testapp.SecondLevel1.
  • testapp/main/migrations/0002_auditabletestmodel.py
    • Updated dependency from testapp to main.
  • testapp/main/migrations/0003_auditabletestmodelaudit.py
    • Updated dependency from testapp to main.
    • Updated ForeignKey to point to main.auditabletestmodel instead of testapp.auditabletestmodel.
  • testapp/main/settings/dev.py
    • Updated import_settings_modules to use main.settings.shared.
    • Updated MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC to mitol.digitalcredentials.main.integration.build_credential.
  • testapp/main/settings/example.dev.py
    • Updated import_settings_modules to use main.settings.shared.
    • Updated MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC to main.integration.build_credential.
    • Updated MITOL_MAIL_MESSAGE_CLASSES to main.messages.SampleMessage.
  • testapp/main/settings/shared.py
    • Updated Django settings description to Django settings for main project.
    • Updated installed apps to include main instead of testapp.
    • Updated ROOT_URLCONF to main.urls.
    • Updated template directory to BASE_DIR + "/main/templates/".
    • Updated WSGI_APPLICATION to main.wsgi.application.
  • testapp/main/settings/test.py
    • Updated import_settings_modules to use main.settings.shared.
    • Updated MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC to main.integration.build_credential.
    • Updated MITOL_MAIL_MESSAGE_CLASSES to main.messages.SampleMessage.
  • testapp/main/urls.py
    • Updated import from testapp.views to main.views.
  • testapp/main/views.py
    • Updated import from testapp.models to main.models.
  • testapp/main/wsgi.py
    • Updated DJANGO_SETTINGS_MODULE to main.settings.dev.
  • testapp/manage.py
    • Updated DJANGO_SETTINGS_MODULE to main.settings.dev.
  • tests/conftest.py
    • Added imports for json, contextmanager, and Path.
    • Added fixtures open_data_fixture_file and load_data_fixture_json for loading data fixtures.
  • tests/mitol/common/test_apps.py
    • Moved import testapp below other imports.
  • tests/mitol/common/test_envs.py
    • Updated DJANGO_SETTINGS_MODULE to main.settings.test.
  • tests/mitol/common/test_models.py
    • Updated imports from testapp.models to main.models.
    • Moved from mitol.common.factories import UserFactory and from mitol.common.utils.serializers import serialize_model_object below the main.models import.
  • tests/mitol/digitalcredentials/test_backend.py
    • Moved from main.factories import DemoCoursewareDigitalCredentialRequestFactory above from mitol.digitalcredentials.backend import
    • Removed from testapp.factories import DemoCoursewareDigitalCredentialRequestFactory
  • tests/mitol/digitalcredentials/test_models.py
    • Moved from main.factories import above from mitol.digitalcredentials.factories import LearnerDIDFactory
    • Removed from testapp.factories import
  • tests/mitol/digitalcredentials/test_serializers.py
    • Moved from main.factories import above from mitol.digitalcredentials.factories import LearnerDIDFactory
    • Removed from testapp.factories import
  • tests/mitol/digitalcredentials/test_views.py
    • Moved from main.factories import above from mitol.digitalcredentials.backend import create_deep_link_url
    • Removed from testapp.factories import
  • tests/mitol/google_sheets/test_views.py
    • Replaced from testapp.utils import set_request_session with from main.utils import set_request_session
  • tests/mitol/google_sheets_deferrals/test_api.py
    • Removed import os
    • Replaced request_csv_rows fixture to use open_data_fixture_file
  • tests/mitol/google_sheets_refunds/test_api.py
    • Removed import os
    • Replaced request_csv_rows fixture to use open_data_fixture_file
  • tests/mitol/mail/testapp/test_messages.py
    • Updated import from testapp.messages to main.messages.
  • tests/mitol/payment_gateway/api/test_cybersource.py
    • Removed import os
    • Moved from main.factories import CartItemFactory, OrderFactory, RefundFactory above from mitol.common.utils.datetime import now_in_utc
    • Removed from testapp.factories import CartItemFactory, OrderFactory, RefundFactory
    • Replaced response_payload fixture to use load_data_fixture_json
  • uv.lock
    • Updated version of mitol-django-transcoding from 2025.2.22 to 2025.2.25.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Trivia time!

What is the purpose of the __init__.py file in Python packages?

Click here for the answer
In Python versions 3.3 and later, the presence of `__init__.py` is no longer required for a directory to be considered a package. However, it can still be used to execute initialization code for the package or to define the symbols that a package exports.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request moves the testapp out of the tests/ directory and renames the main module to address path resolution issues. The changes appear to be well-organized and consistent. I've provided a few suggestions for minor improvements.

Summary of Findings

  • Consistency in naming: The renaming of testapp to main is generally well-handled, but there are a few instances where the old name persists in comments or variable names. Ensure that all references are updated for consistency.
  • File path usage: The change introduces the usage of Path from pathlib for file path manipulation. This is a good practice for cross-platform compatibility and readability. Ensure that the new code integrates smoothly with existing file handling methods.
  • Test data loading: The pull request introduces new fixtures for loading test data from JSON files. This is a good approach for managing test data, but ensure that the file paths are correctly configured and that the fixtures are used consistently across tests.

Assessment

The pull request effectively addresses the path resolution issues by moving the testapp and renaming the main module. The changes are well-structured and the tests should pass as indicated. I recommend addressing the comments to ensure consistency and maintainability. After addressing these comments, the pull request should be ready for merging, but users should have others review and approve this code before merging.

@jkachel jkachel self-assigned this Feb 27, 2025
Copy link
Contributor

@jkachel jkachel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@rhysyngsun rhysyngsun merged commit 8bdc03e into main Feb 27, 2025
8 checks passed
@rhysyngsun rhysyngsun deleted the nl/move-testapp branch February 27, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants