Hi Henning, Thanks for a thorough review. A lot of stuff needs polishing here, and in fact some of this polish I had lost in the shenanigans I went through trying to get the branches to fit together. So a good thing you spotted them. > > === added file 'lib/lp/translations/tests/helpers.py' > > --- lib/lp/translations/tests/helpers.py 1970-01-01 00:00:00 +0000 > > +++ lib/lp/translations/tests/helpers.py 2010-06-29 10:50:45 +0000 > > @@ -0,0 +1,146 @@ > > +# Copyright 2010 Canonical Ltd. This software is licensed under the > > +# GNU Affero General Public License version 3 (see the file LICENSE). > > + > > +__all__ = [ > > + 'get_all_important_translations', > > + 'make_translationmessage', > > + 'make_translationmessage_for_context', > > + ] > > + > > +from zope.security.proxy import removeSecurityProxy > > + > > +from canonical.database.sqlbase import sqlvalues > > + > > +from lp.translations.interfaces.translationmessage import ( > > + RosettaTranslationOrigin, > > + TranslationValidationStatus) > > +from lp.translations.model.translationmessage import ( > > + TranslationMessage) > > + > > +def make_translationmessage_for_context(factory, pofile, potmsgset=None, Oops, that should have been a double blank line of course. That mistake is in other places as well. > > + current=True, other=False, > > + diverged=False, translations=None): > > + """A low-level way of constructing TMs appropriate to `pofile` > context.""" > > + assert pofile is not None, "You must pass in an existing POFile." > > + > > + potemplate = pofile.potemplate > > + if potemplate.distroseries is not None: > > + ubuntu = current > > + upstream = other > > + else: > > + ubuntu = other > > + upstream = current > > + return make_translationmessage( > > + factory, pofile, potmsgset, ubuntu, upstream, diverged, > translations) > > + > > +def make_translationmessage(factory, pofile=None, potmsgset=None, > > + ubuntu=True, upstream=True, > > + diverged=False, translations=None): > > + """Creates a TranslationMessage directly and sets relevant parameters. > > + > > + This is very low level function used to test core Rosetta > > + functionality such as setCurrentTranslation() method. If not used > > + correctly, it will trigger unique constraints. > > + """ > > + if pofile is None: > > + pofile = factory.makePOFile('sr') > > + if potmsgset is None: > > + potmsgset = factory.makePOTMsgSet( > > + potemplate=pofile.potemplate) > > + if translations is None: > > + translations = [factory.getUniqueString()] > > + if diverged: > > + potemplate = pofile.potemplate > > + else: > > + potemplate = None > > + > > + # Parameters we don't care about are origin, submitter and > > + # validation_status. > > + origin = RosettaTranslationOrigin.SCM > > + submitter = pofile.owner > > + validation_status = TranslationValidationStatus.UNKNOWN > > + > > + translations = dict( > > + [(i, translations[i]) for i in range(len(translations))]) > > I just used "dict(enumerate(translations))" in the factory. Is that not the > same? Yes, it's the same and better. One of those bits of polish I lost. > > + > > + potranslations = removeSecurityProxy( > > + potmsgset)._findPOTranslations(translations) > > + new_message = TranslationMessage( > > + potmsgset=potmsgset, > > + potemplate=potemplate, > > + pofile=None, > > + language=pofile.language, > > + variant=pofile.variant, > > + origin=origin, > > + submitter=submitter, > > + msgstr0=potranslations[0], > > + msgstr1=potranslations[1], > > + msgstr2=potranslations[2], > > + msgstr3=potranslations[3], > > + msgstr4=potranslations[4], > > + msgstr5=potranslations[5], > > + validation_status=validation_status, > > + is_current_ubuntu=ubuntu, > > + is_current_upstream=upstream) > > + return new_message > > + > > +def get_all_translations_current_anywhere(pofile, potmsgset): > > + """Get all translation messages on this POTMsgSet used anywhere.""" > > + used_clause = ('(is_current_ubuntu IS TRUE OR ' > > + 'is_current_upstream IS TRUE)') > > + template_clause = 'TranslationMessage.potemplate IS NOT NULL' > > + clauses = [ > > + 'potmsgset = %s' % sqlvalues(potmsgset), > > + used_clause, > > + template_clause, > > + 'TranslationMessage.language = %s' % sqlvalues(pofile.language)] > > + if pofile.variant is None: > > + clauses.append('TranslationMessage.variant IS NULL') > > + else: > > + clauses.append( > > + 'TranslationMessage.variant=%s' % sqlvalues(pofile.variant)) > > + > > + order_by = '-COALESCE(potemplate, -1)' > > + > > + return TranslationMessage.select( > > + ' AND '.join(clauses), orderBy=[order_by]) > > This should be storm code, shouldn't it? Yes, it should. Fixed. > > + > > +def get_all_important_translations(pofile, potmsgset): > > Hm, I don't think "important" is a very useful term here as it is not very > exact. > > > + """Return all existing current translations. > > See, why not call it "get_all_current_translations"? You're touching a nerve there. I agree that the name is not very good ( http://simonwillison.net/2007/Jul/5/hard/ ) and needs fixing. But what you suggest here reveals the weakness in the naming of get_all_translations_current_anywhere in that it is very hard to distinguish from it. For this kind of structured version of a data set I like to use words like "summarize" or "describe." So how about summarize_current_translations? > > + > > + Returns a tuple containing 4 elements: > > + * current, shared translation for `potmsgset`. > > + * diverged translation for `potmsgset` in `pofile` or None. > > + * shared translation for `potmsgset` in "other" context. > > + * list of all other diverged translations (not including the one > > + diverged in `pofile`) or an empty list if there are none. > > + """ > > + current_shared = potmsgset.getCurrentTranslationMessage( > > + None, pofile.language, pofile.variant) > > + current_diverged = potmsgset.getCurrentTranslationMessage( > > + pofile.potemplate, pofile.language, pofile.variant) > > + if (current_diverged is not None and > > + current_diverged.potemplate is None): > > + current_diverged = None > > What is this message you could get from getCurrentTranslationMessage with > potemplate != None? Is that behavior intentended? Yes, that's intentional. It's simply a message that's diverged for the given pofile's template. What may be causing misunderstandings is that current_shared must have None as its potemplate, but current_diverged may not. Of course we now have is_diverged to help us here, so I'll use that instead of the potemplate check. > > + > > + other_shared = potmsgset.getImportedTranslationMessage( > > + None, pofile.language, pofile.variant) > > + other_diverged = potmsgset.getImportedTranslationMessage( > > + pofile.potemplate, pofile.language, pofile.variant) > > + assert other_diverged is None or other_diverged.potemplate is None, ( > > + "There is a diverged 'other' translation for " > > + "this same template, which isn't impossible.") > > Double negation ... ;-) Definitely not untrue. I did not fail to decline leaving this unchanged. > > === added file 'lib/lp/translations/tests/test_helpers.py' > > --- lib/lp/translations/tests/test_helpers.py 1970-01-01 00:00:00 +0000 > > +++ lib/lp/translations/tests/test_helpers.py 2010-06-29 10:50:45 +0000 > > @@ -0,0 +1,133 @@ > > +# Copyright 2010 Canonical Ltd. This software is licensed under the > > +# GNU Affero General Public License version 3 (see the file LICENSE). > > + > > +__metaclass__ = type > > + > > +from zope.component import getUtility > > + > > +from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities > > +from canonical.testing import ZopelessDatabaseLayer > > + > > +from lp.testing import TestCaseWithFactory > > + > > +from lp.translations.tests.helpers import ( > > + make_translationmessage, > > + get_all_important_translations) > > + > > + > > +class TestTranslationMessageHelpers(TestCaseWithFactory): > > + """Test discovery of translation suggestions.""" > > + > > + layer = ZopelessDatabaseLayer > > + > > + def setUp(self): > > + super(TestTranslationMessageHelpers, self).setUp() > > + ubuntu = getUtility(ILaunchpadCelebrities).ubuntu > > + self.factory.makeDistroRelease(distribution=ubuntu) > > Since you are not storing the return value this call obviously produces a side > effect that needs explanation. The intention seems to have been that this would create a new "current" Ubuntu series, but that is not in fact what happened. > > + sourcepackagename = self.factory.makeSourcePackageName() > > + potemplate = self.factory.makePOTemplate( > > + distroseries=ubuntu.currentseries, > > And how does that series you created earlier relate to this series? They seem to have been meant to be the same one, so I passed the previously created series here. > > + sourcepackagename=sourcepackagename) > > + self.pofile = self.factory.makePOFile('sr', potemplate=potemplate) > > + self.potmsgset = self.factory.makePOTMsgSet(potemplate=potemplate, > > + sequence=1) > > + > > + # A POFile in a different context from self.pofile. > > + self.other_pofile = self.factory.makePOFile( > > + language_code=self.pofile.language.code, > > + variant=self.pofile.variant) > > + > > + def test_make_translationmessage(self): > > + translations = [u"testing"] > > + tm = make_translationmessage(self.factory, pofile=self.pofile, > > + potmsgset=self.potmsgset, > > + translations=translations) > > + self.assertEquals(translations, tm.translations) > > + > > + def test_get_all_important_translations(self): > > + current_shared, current_diverged, other, divergences = ( > > + get_all_important_translations(self.pofile, self.potmsgset)) > > + self.assertIs(None, current_shared) > > + self.assertIs(None, current_diverged) > > + self.assertIs(None, other) > > + self.assertEquals([], divergences) > > + > > + def test_get_all_important_translations_current_shared(self): > > + tm = make_translationmessage( > > + self.factory, pofile=self.pofile, potmsgset=self.potmsgset, > > + ubuntu=True, upstream=False, diverged=False) > > + current_shared, current_diverged, other, divergences = ( > > + get_all_important_translations(self.pofile, self.potmsgset)) > > + self.assertEquals(tm, current_shared) > > + self.assertIs(None, current_diverged) > > + self.assertIs(None, other) > > + self.assertEquals([], divergences) > > + > > + def test_get_all_important_translations_current_both(self): > > + tm = make_translationmessage( > > + self.factory, pofile=self.pofile, potmsgset=self.potmsgset, > > + ubuntu=True, upstream=True, diverged=False) > > + current_shared, current_diverged, other, divergences = ( > > + get_all_important_translations(self.pofile, self.potmsgset)) > > + self.assertEquals(tm, current_shared) > > + self.assertIs(None, current_diverged) > > + self.assertEquals(tm, other) > > + self.assertEquals([], divergences) > > + > > + def test_get_all_important_translations_current_both_same(self): > > You are right, this is actually the same (i.e. identical) test as the previous > one. ;-) Wow, well spotted. As far as I can tell this was meant to test for the case where Ubuntu and upstream each have a shared message, and the two contain the same translations. So I made it test for that. > BTW, I'd enjoy comments on each test method to know what it is about. So would I! Adding them. > > + tm = make_translationmessage( > > + self.factory, pofile=self.pofile, potmsgset=self.potmsgset, > > + ubuntu=True, upstream=True, diverged=False) > > + current_shared, current_diverged, other, divergences = ( > > + get_all_important_translations(self.pofile, self.potmsgset)) > > + self.assertEquals(tm, current_shared) > > + self.assertIs(None, current_diverged) > > + self.assertEquals(tm, other) > > + self.assertEquals([], divergences) > > + > > + def test_get_all_important_translations_current_two_different(self): > > + tm_this = make_translationmessage( > > + self.factory, pofile=self.pofile, potmsgset=self.potmsgset, > > + ubuntu=True, upstream=False, diverged=False) > > + tm_other = make_translationmessage( > > + self.factory, pofile=self.pofile, potmsgset=self.potmsgset, > > + ubuntu=False, upstream=True, diverged=False) > > + current_shared, current_diverged, other, divergences = ( > > + get_all_important_translations(self.pofile, self.potmsgset)) > > + self.assertEquals(tm_this, current_shared) > > + self.assertIs(None, current_diverged) > > + self.assertEquals(tm_other, other) > > + self.assertEquals([], divergences) > > + > > + def test_get_all_important_translations_current_three_different(self): > > + tm_this = make_translationmessage( > > + self.factory, pofile=self.pofile, potmsgset=self.potmsgset, > > + ubuntu=True, upstream=False, diverged=False) > > + tm_other = make_translationmessage( > > + self.factory, pofile=self.pofile, potmsgset=self.potmsgset, > > + ubuntu=False, upstream=True, diverged=False) > > + tm_diverged = make_translationmessage( > > + self.factory, pofile=self.pofile, potmsgset=self.potmsgset, > > + ubuntu=True, upstream=False, diverged=True) > > + current_shared, current_diverged, other, divergences = ( > > + get_all_important_translations(self.pofile, self.potmsgset)) > > + self.assertEquals(tm_this, current_shared) > > + self.assertEquals(tm_diverged, current_diverged) > > + self.assertEquals(tm_other, other) > > + self.assertEquals([], divergences) > > + > > + def > test_get_all_important_translations_current_three_diverged_elsewhere( > > + self): > > Woa, I don't think we should be doing this, not even for test methods. Can you > not find a shorter name? Yes: I replaced the "two" and "three" in the names with 2 and 3 respectively, and abbreviated "elsewhere" to "elsewh." > > + tm_diverged = make_translationmessage( > > + self.factory, pofile=self.other_pofile, > potmsgset=self.potmsgset, > > + ubuntu=True, upstream=False, diverged=True) > > + self.assertTrue(tm_diverged.is_current_ubuntu) > > + self.assertEquals( > > + tm_diverged.potemplate, self.other_pofile.potemplate) > > + self.assertEquals(tm_diverged.potmsgset, self.potmsgset) > > Nitpick: you swapped the "expected, observed" order in these two. Wasn't me, but you're right. Fixed. > > === added file 'lib/lp/translations/tests/test_setcurrenttranslation.py' > > --- lib/lp/translations/tests/test_setcurrenttranslation.py 1970-01-01 > 00:00:00 +0000 > > +++ lib/lp/translations/tests/test_setcurrenttranslation.py 2010-06-29 > 10:50:45 +0000 > > @@ -0,0 +1,1186 @@ > > +# Copyright 2009-2010 Canonical Ltd. This software is licensed under the > > +# GNU Affero General Public License version 3 (see the file LICENSE). > > + > > +# pylint: disable-msg=C0102 > > + > > +__metaclass__ = type > > + > > +from zope.component import getUtility > > + > > +from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities > > +from canonical.testing import ZopelessDatabaseLayer > > + > > +from lp.testing import TestCaseWithFactory > > +from lp.translations.interfaces.translationmessage import ( > > + RosettaTranslationOrigin) > > +from lp.translations.interfaces.translations import ( > > + TranslationSide) > > +from lp.translations.model.translationmessage import ( > > + TranslationMessage) > > + > > +from lp.translations.tests.helpers import ( > > + make_translationmessage_for_context, > > + get_all_important_translations) > > + > > + > > +# This test is based on the matrix described on: > > +# https://dev.launchpad.net/Translations/Specs > > +# /UpstreamImportIntoUbuntu/FixingIsImported > > +# /setCurrentTranslation#Execution%20matrix > > + > > +class SetCurrentTranslationTestMixin: > > + """Tests for `POTMsgSet.setCurrentTranslation`.""" > > + > > + layer = ZopelessDatabaseLayer > > I think I'd rather see this in the actual TestCase class than in this Mixin. Okay, I've moved it. > > + > > + def constructTranslationMessage(self, pofile=None, potmsgset=None, > > + current=True, other=False, > diverged=False, > > + translations=None): > > + """Creates a TranslationMessage directly for `pofile` context.""" > > + if pofile is None: > > + pofile = self.pofile > > + if potmsgset is None: > > + potmsgset = self.potmsgset > > + return make_translationmessage_for_context( > > + self.factory, pofile, potmsgset, > > + current, other, diverged, translations) > > + > > + def constructOtherTranslationMessage(self, potmsgset=None, > current=True, > > + other=False, diverged=False, > > + translations=None): > > + """Creates a TranslationMessage for self.other_pofile context.""" > > + return self.constructTranslationMessage( > > + self.other_pofile, potmsgset, current, other, diverged, > > + translations) > > + > > + def constructDivergingTranslationMessage(self, potmsgset=None, > > + current=True, other=False, > > + diverged=False, > > + translations=None): > > + """Creates a TranslationMessage for self.diverging_pofile > context.""" > > + return self.constructTranslationMessage( > > + self.diverging_pofile, potmsgset, current, other, diverged, > > + translations) > > + > > + def setCurrentTranslation(self, translations, > > + share_with_other_side=False): > > + """Helper method to call 'setCurrentTranslation' method. > > + > > + It passes all the same parameters we use throughout the tests, > > + including self.potmsgset, self.pofile, self.pofile.owner and > origin. > > + """ > > + translations = dict( > > + [(i, translations[i]) for i in range(len(translations))]) > > Again, couldn't you use "enumerate"? Absolutely. > > + return self.potmsgset.setCurrentTranslation( > > + self.pofile, self.pofile.owner, translations, > > + origin=RosettaTranslationOrigin.ROSETTAWEB, > > + translation_side=TranslationSide.UBUNTU, > > + share_with_other_side=share_with_other_side) > > You refer to this as the "sharing policy" and I wonder if an enum called > "sharing_policy" would not be clearer? Actually I like this boolean for its simplicity. I think the name describes its meaning well. We've learned to rely on naming as a keyword argument to make its meaning clear in any context where it's used. An enum might be clearer if it led to better names, but in this case I don't think it would pay for the cost of defining an enum in the interface and importing it in various places. > [...] > > + def test_current_None__new_None__other_diverged(self): > > + # Current translation is None, and we have found no > > + # existing TM matching new translations. > > + # There is a current but diverged translation in "other" context. > > + tm_other = self.constructOtherTranslationMessage( > > + current=True, other=False, diverged=True) > > + self.assert_Current_Diverged_Other_DivergencesElsewhere_are( > > + None, None, None, [tm_other]) > > + > > + new_translations = [self.factory.getUniqueString()] > > + tm = self.setCurrentTranslation(new_translations) > > + > > + # We end up with a shared current translation. > > + self.assertTrue(tm is not None) > > + self.assert_Current_Diverged_Other_DivergencesElsewhere_are( > > + tm, None, None, [tm_other]) > > + > > + # Previously current is still diverged and current > > + # in exactly one context. > > + self.assertFalse(tm_other.is_current_upstream and > > + tm_other.is_current_ubuntu) > > Wrong multiline handling for function calls. ;-) Absolutely. Fixed throughout, I hope. > > + self.assertTrue(tm_other.is_current_upstream or > > + tm_other.is_current_ubuntu) > > Same here and for all aother occurences in this file. Fixed all that I found. > > + def test_current_None__new_shared__other_shared__identical__follows( > > + self): > > Good news: the self does still fit on the same line with the method name. > Phew! Huzzah. > > + # As above, and 'share_with_other_side' is a no-op in this case. > > + self.test_current_None__new_shared__other_shared__identical(True) > > + > > [...] > > > + def test_current_shared__new_None__other_shared__follows(self): > > # The sharing policy has no effect on the operation. > > Or something like that. Done throughout. > > + self.test_current_shared__new_None__other_shared(follows=True) > > + > > [...] > > > + def test_current_shared__new_shared__other_shared__follows(self): > > Same here. > > > + self.test_current_shared__new_shared__other_shared(follows=True) > > > + > > + def test_current_shared__new_shared__other_shared__identical( > > + self, follows=False): > > Hm, I really don't like this loss of readability. Maybe you should shorten all > test methods by leaving out the double underscores and abbreviating the > recurring terms. Something like this: > > def test_c_shared_n_shared_o_shared_identical(self, follows=False): > > I mean that for all method names in this file. A quick search and replace > should be able to do that. Hmm... I think under the circumstances it's probably necessary. And now that I've done it, I also see the naming pattern more clearly because it's not hidden in the jumble of repeating words. > > + # Current translation is 'shared', and we have found > > + # a shared existing TM matching new translations that is > > + # also current for "other" context. > > + new_translations = [self.factory.getUniqueString()] > > + tm_shared = self.constructTranslationMessage( > > + current=True, other=False, diverged=False) > > + tm_other = self.constructOtherTranslationMessage( > > + current=True, other=False, diverged=False, > > + translations=new_translations) > > + self.assert_Current_Diverged_Other_DivergencesElsewhere_are( > > + tm_shared, None, tm_other, []) > > + > > + tm = self.setCurrentTranslation( > > + new_translations, share_with_other_side=follows) > > + > > + # New translation message is shared for both contexts. > > + self.assertTrue(tm is not None) > > + self.assertNotEquals(tm_shared, tm) > > + self.assertEquals(tm_other, tm) > > + self.assert_Current_Diverged_Other_DivergencesElsewhere_are( > > + tm, None, tm, []) > > + > > + # Previous shared translation is now a suggestion. > > + self.assertFalse(tm_shared.is_current_ubuntu or > > + tm_shared.is_current_upstream) > > + > > + def test_current_shared__new_shared__other_shared__identical__follows( > > + self): > > Yes, I definitely think you should consider shortening the names. Done throughout. Documented, too. > > + # Since we are converging to the 'other' context anyway, it behaves > > + # the same when 'share_with_other_side=True' is passed in. > > + self.test_current_shared__new_shared__other_shared__identical(True) > > + > > [...] > > > + > > + def test_current_diverged__new_None__other_None__follows(self): > > + # XXX DaniloSegan 20100530: I am not sure 'follows' tests > > + # are needed when current is diverged: they'd make sense only > > + # when we end up converging the translation. > > + self.test_current_diverged__new_None__other_None(True) > > In any case, it should be "follows=True" to stay consistent. More instances > below. Done throughout. > [...] > > > + def > test_current_diverged__new_shared__other_shared__identical_s__follows( > > + self): > > Missing comment. I know this has been explained in earlier tests. I am just > thinking of the time when this test fails and somebody comes here to look and > has no clue what it is about. I added some fairly repetitive comments. Danilo already wrote a bunch of XXX comments to note that some of these tests may not be necessary, but now that we have them we might as well check that the behavior is unaffected. I removed those comments instead. > > + s = self > > Argh! Please, please, shorten the names. Done. > > + > s.test_current_diverged__new_shared__other_shared__identical_shared( > > + follows=True) > > + > > [...] > > + def test_current_diverged__new_diverged__other_diverged( > > + self, follows=False): > > + # Current translation is 'diverged' (no shared), and we have found > > + # an existing diverged in other context TM matching new > translations. > > + new_translations = [self.factory.getUniqueString()] > > + tm_diverged = self.constructTranslationMessage( > > + current=True, other=False, diverged=True) > > + tm_other_diverged = self.constructOtherTranslationMessage( > > + current=True, other=False, diverged=True, > > + translations=new_translations) > > + self.assert_Current_Diverged_Other_DivergencesElsewhere_are( > > + None, tm_diverged, None, [tm_other_diverged]) > > + > > + tm = self.setCurrentTranslation( > > + new_translations, share_with_other_side=follows) > > + > > + # New translation message stays diverged and current only for > > + # the active context. Existing divergence elsewhere is untouched. > > + # XXX DaniloSegan 20100530: it'd be nice to have this > > + # converge the diverged translation (since shared is None), > > + # though it's not a requirement: (tm, None, tm_other, []) > > + self.assertTrue(tm is not None) > > + self.assertNotEquals(tm_diverged, tm) > > + self.assertNotEquals(tm_other_diverged, tm) > > + self.assert_Current_Diverged_Other_DivergencesElsewhere_are( > > + None, tm, None, [tm_other_diverged]) > > Shouldn't there be at least one test where DivergencesElsewhere has more than > one element? Nice one. But that's for test_helpers, I think; all we need to know is that if there are multiple diverged messages elsewhere, they would all show up. > > + > > + # Previously current is not current anymore. > > + self.assertFalse(tm_diverged.is_current_ubuntu or > > + tm_diverged.is_current_upstream) > > + > > + def test_current_diverged__new_diverged__other_diverged__follows(self): > > + # XXX DaniloSegan 20100530: I am not sure 'follows' tests > > + # are needed when current is diverged: they'd make sense only > > + # when we end up converging the translation. > > + self.test_current_diverged__new_diverged__other_diverged(True) > > + > > + > > +class TestSetCurrentTranslation_Ubuntu(SetCurrentTranslationTestMixin, > > + TestCaseWithFactory): > > layer = ZopelessDatabaseLayer > > It just looks weired without ... So does "weired." Guess which one I fixed? :-) > > + def setUp(self): > > + super(TestSetCurrentTranslation_Ubuntu, self).setUp() > > + ubuntu = getUtility(ILaunchpadCelebrities).ubuntu > > + sharing_series = > self.factory.makeDistroRelease(distribution=ubuntu) > > + sourcepackagename = self.factory.makeSourcePackageName() > > + potemplate = self.factory.makePOTemplate( > > + distroseries=ubuntu.currentseries, > > + sourcepackagename=sourcepackagename) > > + sharing_potemplate = self.factory.makePOTemplate( > > + distroseries=sharing_series, > > + sourcepackagename=sourcepackagename, > > + name=potemplate.name) > > + self.pofile = self.factory.makePOFile('sr', potemplate=potemplate, > > + create_sharing=True) > > Wrong multiline handling. Also, wrong language code (should be 'nl')... :-P Given who wrote this, "sr" is appropriate. Fixed the indentation. > > + > > + # A POFile in the same context as self.pofile, used for diverged > > + # translations. > > + self.diverging_pofile = sharing_potemplate.getPOFileByLang( > > + self.pofile.language.code, self.pofile.variant) > > + > > + # A POFile in a different context from self.pofile and > > + # self.diverging_pofile. > > + self.other_pofile = self.factory.makePOFile( > > + language_code=self.pofile.language.code, > > + variant=self.pofile.variant) > > + > > + self.potmsgset = self.factory.makePOTMsgSet(potemplate=potemplate, > > + sequence=1) > > + > > + > > +# XXX JeroenVermeulen 2010-06-29 bug=546310: we can activate this once > > +# getCurrentTranslation and getImportedTranslation "change sides" based > > +# on the current template: lp:~danilo/launchpad/use-upstream-translations. > > Great to see we have this test ready to be let loose. Thanks for doing this in > this flexible manner. It's a bit of a standard trick. Thanks to the comprehensive testing Danilo provided, we'll instantly have a powerful body of tests for the get{Current,Imported}TranslationMessage changes as well. All good now? Jeroen