-
Notifications
You must be signed in to change notification settings - Fork 1
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
Conversation
There was a problem hiding this 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 thetests/
directory to the root level. - Module Renaming: The main module has been renamed from
testapp
tomain
. - 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 tomain
. - Updated line number for secret in
tests/conftest.py
from 120 to 123.
- Updated file paths to reflect the move of
- pyproject.toml
- Updated
DJANGO_SETTINGS_MODULE
tomain.settings.test
. - Added
testapp
topythonpath
. - Updated
django_settings_module
undertool.django-stubs
tomain.settings.test
.
- Updated
- testapp/main/README.md
- Updated imports from
testapp.factories
tomain.factories
andtestapp.messages
tomain.messages
.
- Updated imports from
- testapp/main/admin.py
- Updated import from
testapp.models
tomain.models
.
- Updated import from
- testapp/main/factories.py
- Updated import from
testapp.models
tomain.models
.
- Updated import from
- testapp/main/migrations/0001_initial.py
- Updated ManyToManyField to point to
main.SecondLevel2
instead oftestapp.SecondLevel2
. - Updated ForeignKey to point to
main.SecondLevel1
instead oftestapp.SecondLevel1
.
- Updated ManyToManyField to point to
- testapp/main/migrations/0002_auditabletestmodel.py
- Updated dependency from
testapp
tomain
.
- Updated dependency from
- testapp/main/migrations/0003_auditabletestmodelaudit.py
- Updated dependency from
testapp
tomain
. - Updated ForeignKey to point to
main.auditabletestmodel
instead oftestapp.auditabletestmodel
.
- Updated dependency from
- testapp/main/settings/dev.py
- Updated import_settings_modules to use
main.settings.shared
. - Updated
MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC
tomitol.digitalcredentials.main.integration.build_credential
.
- Updated import_settings_modules to use
- testapp/main/settings/example.dev.py
- Updated import_settings_modules to use
main.settings.shared
. - Updated
MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC
tomain.integration.build_credential
. - Updated
MITOL_MAIL_MESSAGE_CLASSES
tomain.messages.SampleMessage
.
- Updated import_settings_modules to use
- testapp/main/settings/shared.py
- Updated Django settings description to
Django settings for main project
. - Updated installed apps to include
main
instead oftestapp
. - Updated
ROOT_URLCONF
tomain.urls
. - Updated template directory to
BASE_DIR + "/main/templates/"
. - Updated
WSGI_APPLICATION
tomain.wsgi.application
.
- Updated Django settings description to
- testapp/main/settings/test.py
- Updated import_settings_modules to use
main.settings.shared
. - Updated
MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC
tomain.integration.build_credential
. - Updated
MITOL_MAIL_MESSAGE_CLASSES
tomain.messages.SampleMessage
.
- Updated import_settings_modules to use
- testapp/main/urls.py
- Updated import from
testapp.views
tomain.views
.
- Updated import from
- testapp/main/views.py
- Updated import from
testapp.models
tomain.models
.
- Updated import from
- testapp/main/wsgi.py
- Updated
DJANGO_SETTINGS_MODULE
tomain.settings.dev
.
- Updated
- testapp/manage.py
- Updated
DJANGO_SETTINGS_MODULE
tomain.settings.dev
.
- Updated
- tests/conftest.py
- Added imports for
json
,contextmanager
, andPath
. - Added fixtures
open_data_fixture_file
andload_data_fixture_json
for loading data fixtures.
- Added imports for
- tests/mitol/common/test_apps.py
- Moved
import testapp
below other imports.
- Moved
- tests/mitol/common/test_envs.py
- Updated
DJANGO_SETTINGS_MODULE
tomain.settings.test
.
- Updated
- tests/mitol/common/test_models.py
- Updated imports from
testapp.models
tomain.models
. - Moved
from mitol.common.factories import UserFactory
andfrom mitol.common.utils.serializers import serialize_model_object
below themain.models
import.
- Updated imports from
- tests/mitol/digitalcredentials/test_backend.py
- Moved
from main.factories import DemoCoursewareDigitalCredentialRequestFactory
abovefrom mitol.digitalcredentials.backend import
- Removed
from testapp.factories import DemoCoursewareDigitalCredentialRequestFactory
- Moved
- tests/mitol/digitalcredentials/test_models.py
- Moved
from main.factories import
abovefrom mitol.digitalcredentials.factories import LearnerDIDFactory
- Removed
from testapp.factories import
- Moved
- tests/mitol/digitalcredentials/test_serializers.py
- Moved
from main.factories import
abovefrom mitol.digitalcredentials.factories import LearnerDIDFactory
- Removed
from testapp.factories import
- Moved
- tests/mitol/digitalcredentials/test_views.py
- Moved
from main.factories import
abovefrom mitol.digitalcredentials.backend import create_deep_link_url
- Removed
from testapp.factories import
- Moved
- tests/mitol/google_sheets/test_views.py
- Replaced
from testapp.utils import set_request_session
withfrom main.utils import set_request_session
- Replaced
- tests/mitol/google_sheets_deferrals/test_api.py
- Removed
import os
- Replaced
request_csv_rows
fixture to useopen_data_fixture_file
- Removed
- tests/mitol/google_sheets_refunds/test_api.py
- Removed
import os
- Replaced
request_csv_rows
fixture to useopen_data_fixture_file
- Removed
- tests/mitol/mail/testapp/test_messages.py
- Updated import from
testapp.messages
tomain.messages
.
- Updated import from
- tests/mitol/payment_gateway/api/test_cybersource.py
- Removed
import os
- Moved
from main.factories import CartItemFactory, OrderFactory, RefundFactory
abovefrom mitol.common.utils.datetime import now_in_utc
- Removed
from testapp.factories import CartItemFactory, OrderFactory, RefundFactory
- Replaced
response_payload
fixture to useload_data_fixture_json
- Removed
- uv.lock
- Updated version of
mitol-django-transcoding
from2025.2.22
to2025.2.25
.
- Updated version of
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
-
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. ↩
There was a problem hiding this 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
tomain
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
frompathlib
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.
fbdbe19
to
1c2f2ac
Compare
1c2f2ac
to
8730088
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
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
undertests
while also havingtests/mitol
be an implicit package. This removes that implicit package and instead makes thetests/
directory have the same structure as thesrc/
directory. It also renames thetestapp
app tomain
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?
uv run testapp/manage.py showmigrations
, you should see the expected output.