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

Django Testing Additions #421

Merged
merged 7 commits into from
Sep 19, 2023

Conversation

christophertubbs
Copy link
Contributor

Added specialized testing functionality for all django applications

@christophertubbs christophertubbs self-assigned this Sep 7, 2023
@christophertubbs christophertubbs added the enhancement New feature or request label Sep 7, 2023
and added Django testing to the run_tests script
added a line to the django test script to help shed light on issues
if things don't work right.
@christophertubbs christophertubbs marked this pull request as ready for review September 8, 2023 20:54
Copy link
Member

@aaraney aaraney left a comment

Choose a reason for hiding this comment

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

I have one minor request and then we should be on the way. Thanks, @christophertubbs!

DEFAULT_ROOT = str(APPLICATION_ROOT / "python")
"""The root of the python libraries and services"""

MANAGE_MARKER = "os.environ.setdefault('DJANGO_SETTINGS_MODULE'"
Copy link
Member

Choose a reason for hiding this comment

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

Is there a missing closing paren here?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
MANAGE_MARKER = "os.environ.setdefault('DJANGO_SETTINGS_MODULE'"
MANAGE_MARKER = "os.environ.setdefault('DJANGO_SETTINGS_MODULE')"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That won't work - the marker is supposed to match on lines like x = os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'something.settings'). Cutting it off with the ) character will prevent it from matching on what it needs to match on.

file=sys.stderr
)
# Exit with a code of -1 to indicate that this was an application error, not a test error or failure
exit(-1)
Copy link
Member

Choose a reason for hiding this comment

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

POSIX exit codes are unsigned 8 bit integers. Fortunately, python is smart enough and wraps this around to 255, but you may want to change this to some other meaningful positive number.

message = TestMessage(status=current_status, content=current_content, description=current_description)
self.messages.append(message)

def print(self, verbose: bool = None, quiet: bool = None):
Copy link
Member

Choose a reason for hiding this comment

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

I think both verbose and quiet's default value should be False instead of None here.

Changed the default variables for quiet and verbose from None to False
aaraney
aaraney previously approved these changes Sep 18, 2023
@aaraney
Copy link
Member

aaraney commented Sep 18, 2023

Thanks @christophertubbs! Merge at will!

incremented the version for the evaluationservice
added dmod_venv to the gitignore
aaraney
aaraney previously approved these changes Sep 19, 2023
Added venv activation and deactivation around the django tests
added missing logic to append django errors to the total error count
@christophertubbs christophertubbs merged commit cf71809 into NOAA-OWP:master Sep 19, 2023
1 check failed
@christophertubbs christophertubbs deleted the Django-Testing branch September 19, 2023 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants