launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02674
[Merge] lp:~jtv/launchpad/bug-719247 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-719247 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#719247 Translations branch auto-approver not working
https://bugs.launchpad.net/bugs/719247
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-719247/+merge/50204
= Summary =
The TranslationsBranchApprover attempts to "approve" translation template uploads generated from bzr changes. Approval involves matching the upload up with a specific POTemplate in the database based on file paths. It may also involve creating a POTemplate for the purpose first.
For that purpose, the approver needs to figure out its "name" and "domain," two identifying strings for different purposes. Normally they're essentially the same string but the name has to comply with stricter rules so it can be used in URLs. The approver figures these out based on the upload's file path.
Some users were having problems with the mechanism. When their branches produce template uploads with generic names such as messages.pot, the approver is unable to extract a meaningful name and domain from the upload's path. And so it gives up. No template gets created, the upload does not get approved, and the translatable strings are never imported.
== Proposed fix ==
Simple: fall back on the project name if no meaningful identifier can be extracted from the upload's file path. This affects the approver's behaviour in a variety of scenarios and generally leads to more uploads being approved in reasonably sensible ways.
There's no particular need to do the same for distribution source packages, yet. We don't support branch imports for distributions yet. Also, translation domains really don't matter much for project uploads but they're crucial for Ubuntu uploads. They determine the translation files' names as they will be installed on end-users' systems, and so they must be globally unique — except for the few that are processed specially and not directly installed as MO files.
I also didn't make a similar change for the TranslationBuildApprover. On the one hand that's because we haven't actually seen this problem there yet (it's more aggressive and any problems are basically left for the user to recognize and debug), and on the other it's because the build approver doesn't have the same kind of protection against duplicate domains. We don't want to approve a combination of "messages.pot" and "po/messages.pot" because it's not clear whether they were meant to be the same template or not. My change didn't require a special check for that, since the TranslationsBranchApprover will come up with two templates both named after the project, and then recognize them as duplicates anyway.
== Pre-implementation notes ==
Discussed solution with Henning on IRC, after going over the problem with the former Translations team. We always knew we'd have to extend our approval rules in the future, based on our experience supplementing the automatic approver with manual approvers. This just makes the thing slightly more aggressive.
== Implementation details ==
The module that calculates domains and names based on paths is blissfully unaware of model objects. Its tests run without a layer. Not wanting to disturb that idyll, I made the caller provide the default name/domain as an extra, optional parameter.
As a test optimization I split TestTranslationsBranchApprover into two separate parts. It used to run in 30 seconds and take 15 seconds of setup. A small new test case specializes in checking that the approver has the database privileges it needs; this runs in about 3 seconds of testing plus 15 seconds of setup. The remainder I converted to FakeLibrarian. This didn't save much in itself, but it allowed me to skip the commits every time it switches to another database user. Getting rid of the commits brought it down to about 3 seconds, and the lighter layer another 8 seconds or so of setup.
It's always possible that the new test doesn't exercise all necessary database privileges, but in the long run it's probably better to have a specialized test for this anyway.
== Tests ==
Full EC2 run is underway, but:
{{{
./bin/test -vvc lp.translations -t lp.translations.tests.test_translationbranchapprover -t lp.translations.utilities.tests.test_templatenames
}}}
== Demo and Q/A ==
The staging server should be equipped for this. Set up branch synchronization for a productseries, and push a foo.pot to its branch; the foo.pot should appear on the project's import queue with Approved status (or if you're slow, Imported). But if you set things up similarly with a template called messages.pot, you should now see an upload to a template named after the project. The current situation is that that'd simply not get approved.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/tests/test_translationbranchapprover.py
lib/lp/translations/utilities/template.py
lib/lp/translations/utilities/tests/test_templatenames.py
lib/lp/translations/model/approver.py
--
https://code.launchpad.net/~jtv/launchpad/bug-719247/+merge/50204
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-719247 into lp:launchpad.
=== modified file 'lib/lp/translations/model/approver.py'
--- lib/lp/translations/model/approver.py 2010-10-29 10:17:14 +0000
+++ lib/lp/translations/model/approver.py 2011-02-17 18:38:50 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -22,6 +22,18 @@
from lp.translations.utilities.translation_import import TranslationImporter
+def get_product_name(productseries):
+ """Get the series' product name, if any.
+
+ :return: A string; either the product's name or, if `productseries`
+ is None, the empty string.
+ """
+ if productseries is None:
+ return ''
+ else:
+ return productseries.product.name
+
+
class TranslationNullApprover(object):
"""Does not approve any files."""
@@ -59,6 +71,7 @@
self.is_approval_possible = True
potemplate_names = set()
+ product_name = get_product_name(productseries)
importer = TranslationImporter()
self._potemplateset = getUtility(IPOTemplateSet).getSubset(
@@ -68,7 +81,7 @@
if importer.isTemplateName(path):
potemplate = self._potemplateset.getPOTemplateByPath(path)
if potemplate is None:
- name = make_name_from_path(path)
+ name = make_name_from_path(path, default=product_name)
potemplate = self._potemplateset.getPOTemplateByName(name)
else:
name = potemplate.name
@@ -117,7 +130,8 @@
if entry.path not in self._potemplates:
return entry
- domain = make_domain(entry.path)
+ product_name = get_product_name(entry.productseries)
+ domain = make_domain(entry.path, default=product_name)
if self._potemplates[entry.path] is None:
if self.unmatched_objects > 0:
# Unmatched entries in database, do not approve.
=== modified file 'lib/lp/translations/tests/test_translationbranchapprover.py'
--- lib/lp/translations/tests/test_translationbranchapprover.py 2010-10-29 05:58:51 +0000
+++ lib/lp/translations/tests/test_translationbranchapprover.py 2011-02-17 18:38:50 +0000
@@ -1,18 +1,20 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Translation File Auto Approver tests."""
__metaclass__ = type
-from unittest import TestLoader
-
import transaction
from zope.component import getUtility
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.validators.name import valid_name
-from canonical.testing.layers import LaunchpadZopelessLayer
+from canonical.librarian.testing.fake import FakeLibrarian
+from canonical.testing.layers import (
+ LaunchpadZopelessLayer,
+ ZopelessDatabaseLayer,
+ )
from lp.testing import TestCaseWithFactory
from lp.translations.enums import RosettaImportStatus
from lp.translations.interfaces.translationimportqueue import (
@@ -21,18 +23,13 @@
from lp.translations.model.approver import TranslationBranchApprover
-def become_the_approver(layer):
- """Switch to the branch-approver's database user identity."""
- transaction.commit()
- layer.switchDbUser('translationsbranchscanner')
-
-
class TestTranslationBranchApprover(TestCaseWithFactory):
- layer = LaunchpadZopelessLayer
+ layer = ZopelessDatabaseLayer
def setUp(self):
super(TestTranslationBranchApprover, self).setUp()
+ self.useFixture(FakeLibrarian())
self.queue = getUtility(ITranslationImportQueue)
self.series = self.factory.makeProductSeries()
@@ -64,55 +61,64 @@
template_path = self.factory.getUniqueString() + u'.pot'
entry = self._upload_file(template_path)
self.assertEqual(RosettaImportStatus.NEEDS_REVIEW, entry.status)
- approver = self._createApprover(template_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(template_path).approve(entry)
self.assertEqual(RosettaImportStatus.APPROVED, entry.status)
- def test_new_template_missing_domain(self):
- # A file can only be approved if the file path contains the
- # translation domain and is not generic.
+ def test_new_template_without_domain_in_path_uses_project_name(self):
+ # When a template upload for a project has a generic path, it
+ # can still be approved. The template will have a name and
+ # domain based on the project's name.
template_path = u'messages.pot'
entry = self._upload_file(template_path)
- approver = self._createApprover(template_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(template_path).approve(entry)
+ self.assertEqual(RosettaImportStatus.APPROVED, entry.status)
+ self.assertEqual(self.series.product.name, entry.potemplate.name)
+ self.assertEqual(
+ self.series.product.name, entry.potemplate.translation_domain)
+
+ def test_new_package_template_without_domain_is_not_approved(self):
+ # If an upload for a package has no information that a template
+ # name or domain could be based on, it is not approved.
+ # Template domains for packages must generally be unique for the
+ # entire distribution, but in practice it's too variable to
+ # figure out here.
+ package = self.factory.makeSourcePackage()
+ package_kwargs = {
+ 'distroseries': package.distroseries,
+ 'sourcepackagename': package.sourcepackagename,
+ }
+ entry = self.queue.addOrUpdateEntry(
+ 'messages.pot', self.factory.getUniqueString(), True,
+ self.factory.makePerson(), **package_kwargs)
+
+ TranslationBranchApprover(entry.path, **package_kwargs).approve(entry)
self.assertEqual(RosettaImportStatus.NEEDS_REVIEW, entry.status)
def test_new_template_not_a_template(self):
# Only template files will be approved currently.
path = u'eo.po'
entry = self._upload_file(path)
- approver = self._createApprover(path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(path).approve(entry)
self.assertEqual(RosettaImportStatus.NEEDS_REVIEW, entry.status)
def test_new_template_domain(self):
# The approver gets the translation domain for the entry from the
- # file path.
+ # file path if possible.
translation_domain = self.factory.getUniqueString()
template_path = translation_domain + u'.pot'
entry = self._upload_file(template_path)
- approver = self._createApprover(template_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(template_path).approve(entry)
self.assertEqual(
translation_domain, entry.potemplate.translation_domain)
def test_new_template_domain_with_xpi(self):
- # For xpi files the domain is taken from the directory.
+ # For xpi files, template files are always called "en-US.xpi" so
+ # the approver won't use that string for a domain. It'll fall
+ # back to the next possibility, which is the directory.
translation_domain = self.factory.getUniqueString()
template_path = translation_domain + '/en-US.xpi'
entry = self._upload_file(template_path)
- approver = self._createApprover(template_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(template_path).approve(entry)
self.assertEqual(
translation_domain, entry.potemplate.translation_domain)
@@ -121,10 +127,7 @@
translation_domain = (u'Invalid-Name_with illegal#Characters')
template_path = translation_domain + u'.pot'
entry = self._upload_file(template_path)
- approver = self._createApprover(template_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(template_path).approve(entry)
self.assertTrue(valid_name(entry.potemplate.name))
self.assertEqual(u'invalid-name-withillegalcharacters',
entry.potemplate.name)
@@ -135,67 +138,64 @@
template_path = translation_domain + u'.pot'
self._createTemplate(template_path, translation_domain)
entry = self._upload_file(template_path)
- approver = self._createApprover(template_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(template_path).approve(entry)
self.assertEqual(RosettaImportStatus.APPROVED, entry.status)
def test_replace_existing_potemplate(self):
- # When replacing an existing template, the queue entry is linked to
- # that existing entry.
+ # When replacing an existing template, the queue entry is linked
+ # to that existing entry.
translation_domain = self.factory.getUniqueString()
template_path = translation_domain + u'.pot'
potemplate = self._createTemplate(template_path, translation_domain)
entry = self._upload_file(template_path)
- approver = self._createApprover(template_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(template_path).approve(entry)
self.assertEqual(potemplate, entry.potemplate)
def test_replace_existing_any_path(self):
# If just one template file is found in the tree and just one
- # POTemplate object is in the database, the upload is always approved.
+ # POTemplate is in the database, the upload is always approved.
existing_domain = self.factory.getUniqueString()
existing_path = existing_domain + u'.pot'
potemplate = self._createTemplate(existing_path, existing_domain)
template_path = self.factory.getUniqueString() + u'.pot'
entry = self._upload_file(template_path)
- approver = self._createApprover(template_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(template_path).approve(entry)
self.assertEqual(RosettaImportStatus.APPROVED, entry.status)
self.assertEqual(potemplate, entry.potemplate)
def test_replace_existing_generic_path_approved(self):
# If an upload file has a generic path that does not yield a
- # translation domain it is still approved if an entry with the same
- # file name exists.
+ # translation domain, it is still approved if an entry with the
+ # same file name exists.
translation_domain = self.factory.getUniqueString()
generic_path = u'po/messages.pot'
self._createTemplate(generic_path, translation_domain)
entry = self._upload_file(generic_path)
- approver = self._createApprover(generic_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(generic_path).approve(entry)
self.assertEqual(RosettaImportStatus.APPROVED, entry.status)
- def test_replace_existing_generic_path_domain(self):
- # Uploads with a generic path do not overwrite the translation domain
- # on the existing POTemplate entry.
- translation_domain = self.factory.getUniqueString()
+ def test_does_not_replace_domain_if_path_contains_no_useful_name(self):
+ # For an upload to a package (where there's no fallback to a
+ # product name), if the path contains no meaningful domain name
+ # but matches that of an existing template, even though the
+ # entry gets approved for import into that template, the
+ # existing template's domain name stays as it was.
generic_path = u'po/messages.pot'
- self._createTemplate(generic_path, translation_domain)
- entry = self._upload_file(generic_path)
- approver = self._createApprover(generic_path)
-
- become_the_approver(self.layer)
+
+ package = self.factory.makeSourcePackage()
+ package_kwargs = {
+ 'distroseries': package.distroseries,
+ 'sourcepackagename': package.sourcepackagename,
+ }
+ template = self.factory.makePOTemplate(**package_kwargs)
+ original_domain = template.translation_domain
+ entry = self.queue.addOrUpdateEntry(
+ generic_path, self.factory.getUniqueString(), True,
+ template.owner, potemplate=template, **package_kwargs)
+
+ approver = TranslationBranchApprover(generic_path, **package_kwargs)
approver.approve(entry)
- self.assertEqual(
- translation_domain, entry.potemplate.translation_domain)
+ self.assertEqual(original_domain, template.translation_domain)
def test_add_template(self):
# When adding a template to an existing one it is approved if the
@@ -206,10 +206,7 @@
new_domain = self.factory.getUniqueString()
new_path = u"%s/%s.pot" % (new_domain, new_domain)
entry = self._upload_file(new_path)
- approver = self._createApprover((existing_path, new_path))
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover((existing_path, new_path)).approve(entry)
self.assertEqual(RosettaImportStatus.APPROVED, entry.status)
self.assertEqual(new_domain, entry.potemplate.translation_domain)
@@ -221,8 +218,6 @@
entry1 = self._upload_file(pot_path1)
entry2 = self._upload_file(pot_path2)
approver = self._createApprover((pot_path1, pot_path2))
-
- become_the_approver(self.layer)
approver.approve(entry1)
self.assertEqual(RosettaImportStatus.APPROVED, entry1.status)
approver.approve(entry2)
@@ -230,14 +225,12 @@
def test_duplicate_template_name(self):
# If two templates in the branch indicate the same translation
- # domain they are in conflict and will not be approved.
+ # domain, they are in conflict and will not be approved.
pot_path1 = "po/foo_domain.pot"
pot_path2 = "foo_domain/messages.pot"
entry1 = self._upload_file(pot_path1)
entry2 = self._upload_file(pot_path2)
approver = self._createApprover((pot_path1, pot_path2))
-
- become_the_approver(self.layer)
approver.approve(entry1)
self.assertEqual(RosettaImportStatus.NEEDS_REVIEW, entry1.status)
approver.approve(entry2)
@@ -257,8 +250,7 @@
for status in not_approve_status:
entry.setStatus(
status, getUtility(ILaunchpadCelebrities).rosetta_experts)
- approver = self._createApprover(pot_path)
- approver.approve(entry)
+ self._createApprover(pot_path).approve(entry)
self.assertEqual(status, entry.status)
def test_approveNewSharingTemplate(self):
@@ -273,10 +265,7 @@
dutch_pofile = self.factory.makePOFile(
'nl', potemplate=trunk_template)
entry = self._upload_file(pot_path)
- approver = self._createApprover(pot_path)
-
- become_the_approver(self.layer)
- approver.approve(entry)
+ self._createApprover(pot_path).approve(entry)
# This really did create a new template.
self.assertNotEqual(None, entry.potemplate)
@@ -289,5 +278,82 @@
self.assertNotEqual(dutch_pofile, new_dutch_pofile)
-def test_suite():
- return TestLoader().loadTestsFromName(__name__)
+class TestBranchApproverPrivileges(TestCaseWithFactory):
+ """Test database privileges required for the branch approver.
+
+ Runs the `TranslationsBranchApprover` through a few scenarios that
+ exercise its database privileges. This is a slow test because it
+ needs to commit a lot; it's not a place to verify anything other
+ than the database privileges.
+ """
+
+ layer = LaunchpadZopelessLayer
+
+ def becomeTheApprover(self):
+ """Assume the database role of the translations branch scanner.
+
+ This is the role that the TranslationsBranchApprover is actually
+ run under.
+ """
+ transaction.commit()
+ self.layer.switchDbUser('translationsbranchscanner')
+
+ def test_approve_new_product_template(self):
+ # The approver has sufficient privileges to create a new
+ # template on a product.
+ template = self.factory.makePOTemplate()
+ entry = self.factory.makeTranslationImportQueueEntry(
+ 'messages.pot', potemplate=template,
+ productseries=template.productseries)
+
+ self.becomeTheApprover()
+ approver = TranslationBranchApprover(
+ [template.path], productseries=template.productseries)
+ approver.approve(entry)
+
+ def test_approve_new_package_template(self):
+ # The approver has sufficient privileges to create a new
+ # template on a source package.
+ package = self.factory.makeSourcePackage()
+ package_kwargs = {
+ 'distroseries': package.distroseries,
+ 'sourcepackagename': package.sourcepackagename,
+ }
+ template = self.factory.makePOTemplate(**package_kwargs)
+ entry = self.factory.makeTranslationImportQueueEntry(
+ path='messages.pot', potemplate=template, **package_kwargs)
+
+ self.becomeTheApprover()
+ approver = TranslationBranchApprover(
+ [template.path], **package_kwargs)
+ approver.approve(entry)
+
+ def test_approve_sharing_template(self):
+ # The approver has sufficient privileges to approve templates
+ # that will have POFiles copied over from sharing templates.
+ productseries = self.factory.makeProductSeries()
+ package = self.factory.makeSourcePackage()
+ package_kwargs = {
+ 'distroseries': package.distroseries,
+ 'sourcepackagename': package.sourcepackagename,
+ }
+ self.factory.makePackagingLink(
+ productseries=productseries, **package_kwargs)
+
+ template_name = self.factory.getUniqueString()
+ template_path = "%s.pot" % template_name
+
+ self.factory.makePOFile(
+ potemplate=self.factory.makePOTemplate(
+ name=template_name, productseries=productseries))
+ self.factory.makePOFile(
+ potemplate=self.factory.makePOTemplate(
+ name=template_name, **package_kwargs))
+
+ new_series = self.factory.makeProductSeries(
+ product=productseries.product)
+ entry = self.factory.makeTranslationImportQueueEntry(
+ path=template_path, productseries=new_series)
+
+ self.becomeTheApprover()
+ TranslationBranchApprover([template_path], new_series).approve(entry)
=== modified file 'lib/lp/translations/utilities/template.py'
--- lib/lp/translations/utilities/template.py 2010-03-17 16:58:52 +0000
+++ lib/lp/translations/utilities/template.py 2011-02-17 18:38:50 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Functions to help with translation templates."""
@@ -26,7 +26,7 @@
]
-def make_domain(path):
+def make_domain(path, default=''):
"""Generate the translation domain name from the path of the template
file.
@@ -40,9 +40,9 @@
return domain
dname1, dname2 = os.path.split(dname)
if dname2 not in GENERIC_TEMPLATE_DIRS:
- return dname2
+ return dname2 or default
rest, domain = os.path.split(dname1)
- return domain
+ return domain or default
def make_name(domain):
@@ -50,7 +50,6 @@
return sanitize_name(domain.replace('_', '-').lower())
-def make_name_from_path(path):
+def make_name_from_path(path, default=''):
"""Make a template name from a file path."""
- return make_name(make_domain(path))
-
+ return make_name(make_domain(path, default=default))
=== modified file 'lib/lp/translations/utilities/tests/test_templatenames.py'
--- lib/lp/translations/utilities/tests/test_templatenames.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/utilities/tests/test_templatenames.py 2011-02-17 18:38:50 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
import unittest
@@ -13,7 +13,8 @@
class TemplateNamesTest(unittest.TestCase):
"""Test template name conversion utility function."""
- valid_paths = [
+ meaningful_path_domain = 'my_domain'
+ meaningful_paths = [
"my_domain.pot",
"po/my_domain.pot",
"my_domain/messages.pot",
@@ -22,26 +23,44 @@
"my_domain/en-US.xpi",
]
- invalid_paths = [
+ meaningless_paths = [
"messages.pot",
"po/messages.pot",
"en-US.xpi",
"po/en-US.xpi",
]
- def test_make_domain_valid_paths(self):
- # Valid paths yield "my_domain" as the translation domain.
- for path in self.valid_paths:
- domain = make_domain(path)
- self.assertEqual('my_domain', domain,
- "Path '%s' yielded domain '%s'" % (path, domain))
-
- def test_make_domain_invalid_paths(self):
- # Invalid paths yield the empty string as the translation domain.
- for path in self.invalid_paths:
- domain = make_domain(path)
- self.assertEqual('', domain,
- "Path '%s' yielded domain '%s'" % (path, domain))
+ def test_make_domain_extracts_domain_from_meaningful_paths(self):
+ # make_domain will spot a meaningful identifier in a typical
+ # useful template file path.
+ for path in self.meaningful_paths:
+ domain = make_domain(path)
+ self.assertEqual(
+ self.meaningful_path_domain, domain,
+ "Path '%s' yielded domain '%s'; should have found '%s'."
+ % (path, domain, self.meaningful_path_domain))
+
+ def test_make_domain_finds_no_domain_in_meaningless_paths(self):
+ # make_domain will not find any usable identifiers in a typical
+ # meaningless template file path, and default to the empty
+ # string.
+ for path in self.meaningless_paths:
+ domain = make_domain(path)
+ self.assertEqual(
+ '', domain,
+ "Path '%s' yielded domain '%s'; should have found nothing."
+ % (path, domain))
+
+ def test_make_domain_falls_back_on_default(self):
+ # When a path contains no usable identifier, make_domain falls
+ # back on the default you pass it.
+ default_domain = 'default_fallback'
+ for path in self.meaningless_paths:
+ domain = make_domain(path, default=default_domain)
+ self.assertEqual(
+ default_domain, domain,
+ "Path '%s' yielded domain '%s'; expected default '%s'."
+ % (path, domain, default_domain))
def test_make_name_underscore(self):
# Underscores are converted to dashes for template names.
@@ -57,9 +76,22 @@
def test_make_name_from_path(self):
# Chain both methods for convenience.
- self.assertEqual('my-domain', make_name_from_path(
- "po/My_Do@main/messages.pot"))
-
-def test_suite():
- return unittest.TestLoader().loadTestsFromName(__name__)
-
+ self.assertEqual(
+ 'my-domain', make_name_from_path("po/My_Do@main/messages.pot"))
+
+ def test_make_name_from_path_falls_back_on_default(self):
+ # make_name_from_path falls back on the default you pass if the
+ # path contains no helpful identifiers.
+ default_name = 'default-name'
+ self.assertEqual(
+ default_name,
+ make_name_from_path('messages.pot', default=default_name))
+
+ def test_make_name_from_path_sanitizes_default(self):
+ # If make_name_from_path has to fall back on the default you
+ # pass, it sanitizes the domain it gets for use as a template
+ # name, just as it would a domain that was extracted from the
+ # path.
+ self.assertEqual(
+ "foo-bar",
+ make_name_from_path('messages.pot', default="foo_bar"))
Follow ups
