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

Remove special case for test harness from application code. #120

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

geoffbeier
Copy link
Contributor

Addresses #88, at least partially.

This adds a context manager that ensures django settings are reset on exit, and has the two unit tests that call activate() use it.

It also adds two minimal settings modules that match the config those tests were seeing, and passes the names of those modules to activate() so that those are loaded using the same path as user-supplied settings modules.

I thought about using pytest-django's settings fixture, but the way activate() and Plus interact made that smell more than the original hack.

Addresses pydanny#88, at least partially.

This adds a context manager that ensures django settings are reset on exit, and has the two unit tests that call `activate()` use it.

It also adds two minimal settings modules that match the config those tests were seeing, and passes the names of those modules to `activate()` so that those are loaded using the same path as user-supplied settings modules.

I thought about using pytest-django's settings fixture, but the way `activate()` and `Plus` interact made that smell more than the original hack.
…able got created before attempting to delete it. Tests that threw before setting the variable would terminate with a KeyError in the context manager instead of seeing their exceptions reported appropriately.
Copy link
Owner

@pydanny pydanny left a comment

Choose a reason for hiding this comment

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

This is a very elegant solution.

And I'm not going to pretend I'm not jealous that you used a context manager to solve this issue. 😬

Well done! 😁

@pydanny pydanny merged commit 3a1f5c1 into pydanny:main Oct 20, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants