Ticket #19125 #441

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@lqc
lqc commented Oct 13, 2012

Sorry about the earlier version, I first didn't noticed there are already tests for startproject and then forgot to remove the duplicates. Ironicly, the diff on the ticket didn't have them so I didn't noticed.

@claudep claudep commented on the diff Oct 18, 2012
tests/regressiontests/admin_scripts/tests.py
self.assertOutput(err, "Error: '7testproject' is not a valid project name. Please make sure the name begins with a letter or underscore.")
self.assertFalse(os.path.exists(testproject_dir))
+ def test_path_as_project_name(self):
+ "Make sure the startproject management command validates a project name"
+
+ def cleanup(p):
+ if os.path.exists(p):
+ shutil.rmtree(p)
+
+ args = ['startproject', '../testproject']
+ testproject_dir = os.path.join(test_dir, '../testproject')
+ self.addCleanup(cleanup, testproject_dir)
+
+ out, err = self.run_django_admin(args)
+ self.assertOutput(err, "Error: '../testproject' is not a valid project name. Please make sure the name begins with a letter or underscore.")
+ self.assertFalse(os.path.exists(testproject_dir))
+
def test_simple_project_different_directory(self):
@claudep
claudep Oct 18, 2012 Member

Nitpicking: I would have reused the test_invalid_project_name test, looping on "for project_name in ('7testproject', '../testproject'):". This would allow us to test more names with less code duplication.

@lqc
lqc Oct 18, 2012

There is a lot more code duplication in that TestCase then that ;)

The isolation is also poor, because it modifies Django source tree, so this won't work in parallel and won't cleanup if startproject actually makes a directory, but with malformed name. I started refactoring it, but that would make the patch quite a huge change for this ticket. I can still do it, though.

@claudep
claudep Oct 18, 2012 Member

Here's how I would have written it: https://gist.github.com/3911240. Hopefully it's not worse than the previous version.

@claudep
Member
claudep commented Jan 24, 2013

Thanks for the patch, committed in 9893fa1

@claudep claudep closed this Jan 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment