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

Refactor migrations commands tests #4190

Merged
merged 2 commits into from
Feb 23, 2015

Conversation

aaugustin
Copy link
Member

Ensure that running the tests doesn't write in the code tree.

@aaugustin aaugustin force-pushed the refactor-migrations-commands-tests branch from f1d9273 to c828a05 Compare February 22, 2015 22:37
@aaugustin
Copy link
Member Author

@andrewgodwin Can you do a quick sanity check? Most importantly look at the docstring of temporary_migration_module in the second commit. Thanks!

There's no reason to assume that sys.path[0] is an appropriate location
for generating code. Specifically that doesn't work with extend_sys_path
which puts the additional directories at the end of sys.path.

In order to create a new migrations module, instead of using an
arbitrary entry from sys.path, import as much as possible from the path
to the module, then create missing submodules from there.

Without this change, the tests introduced in the following commit fail,
which seems sufficient to prevent regressions for such a refactoring.
@aaugustin aaugustin force-pushed the refactor-migrations-commands-tests branch from c828a05 to 3ca4132 Compare February 23, 2015 18:55
@andrewgodwin
Copy link
Member

Looks good to me - a lot cleaner than the previous tests' behaviour.

This is preferrable to writing in the current working directory because
it eliminates the risk to leak unwanted files, which can result in very
weird test failures.

Also this will help if we ever try to run these tests concurrently.
@aaugustin aaugustin force-pushed the refactor-migrations-commands-tests branch from 3ca4132 to 903d1a5 Compare February 23, 2015 21:23
@aaugustin aaugustin merged commit 903d1a5 into django:master Feb 23, 2015
@aaugustin aaugustin deleted the refactor-migrations-commands-tests branch February 23, 2015 21:33
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.

2 participants