Merge lp:~benji/launchpad/bug-953913 into lp:launchpad

[Benji York]

fix a test fragility by making translation template's sharing key consistent between test runs

During the Yellow squad's work we have found many tests that have
inter-test dependencies, this branch fixes one of them.

When run independently the TestTranslationTemplateChangeJob.test_splits_and_merges
test fails (intermittently for some, consistently for others). It turns
out that the test fails because the sharingKey value can vary between
runs. This branch makes the sharingKey consistent so the values the
test verifies are consistent from one run to another.

Tests:

    bin/test -c -t lp.translations.tests.test_translationpackagingjob.TestTranslationTemplateChangeJob.test_splits_and_merges

Lint: the lint report is clean

QA: none

[Gary Poster (gary)]

Nice, thank you

review: Approve

[Данило Шеган (danilo)]

Hi guys, sorry for taking a look at this so late (I wasn't working the Thu/Fri).

I find it interesting that this has failed intermittently. Let me digest what happens in a test (the fact that this fix is here helps with that a lot :).

`potemplate` is the template we are renaming with one `potmsgset`.
`old_shared` is sharing with `potemplate` that same potmsgset.
`new_shared` is a different template with a different `target_potmsgset` but identical English text for it.

`potemplate` is renamed to start sharing with `new_shared`.

Two things happen:
 1. splitting: `potmsgset` is split into two similar ones: one in `potemplate`, another in `old_shared` (this should have nothing to do with development_focus)
 2. merging: whatever came out of 1 and ended up in `old_shared` (could be `potmsgset` itself or a different potmsgset object), and `target_potmsgset` into a single potmsgset, with the development_focus potmsgset being preferred for keeping.

So, if there's any instability (for this particular test), it's probably in how splitting is done (i.e. sometimes we get the translation_focus `potmsgset` in old_shared, sometimes a new POTMsgSet), and it'd probably make sense to make that use sharingKey as well, i.e. prefer to keep the translation_focus potmsgset object where it is. Then, in 1, we'd always have `potmsgset` remain in `potemplate`, and `old_shared` will end up with eg. `split_potmsgset`. But then, both of `split_potmsgset` and `target_potmsgset` and which one is kept on merging would depend on their IDs, which in testing we can probably rely on to grow sequentially, but shouldn't.

Basically, this code produces a few equivalent, but non-identical "solutions" to the splitting/merging problem. It makes it harder to unit-test, which is likely why I introduced the problem :)

So, IMHO, for this particular test, it would be better to remove the dependency on the development_focus altogether. I suspect having

=== modified file 'lib/lp/translations/tests/test_translationpackagingjob.py'
--- lib/lp/translations/tests/test_translationpackagingjob.py 2012-03-23 21:06:28 +0000
+++ lib/lp/translations/tests/test_translationpackagingjob.py 2012-03-26 07:22:36 +0000
@@ -340,7 +340,9 @@
         # New POTMsgSet is now different from the old one (it's been split),
         # but matches the target potmsgset (it's been merged into it).
         new_potmsgset = potemplate.getPOTMsgSets()[0]
- self.assertNotEqual(potmsgset, new_potmsgset)
+ old_potmsgset = old_shared.getPOTMsgSets()[0]
+ target_potmsgset = new_shared.getPOTMsgSets()[0]
+ self.assertNotEqual(old_potmsgset, new_potmsgset)
         self.assertEqual(target_potmsgset, new_potmsgset)

         # Translations have been merged as well.

would be a sufficient (and more correct) fix instead of the one here (though, adding a comment why is this needed to refetch all the potmsgsets would be rather helpful :).

=== modified file 'lib/lp/translations/tests/test_translationpackagingjob.py'
--- lib/lp/translations/tests/test_translationpackagingjob.py	2012-01-01 02:58:52 +0000
+++ lib/lp/translations/tests/test_translationpackagingjob.py	2012-03-23 21:15:30 +0000
@@ -312,8 +312,9 @@
         """Changing a template makes the translations split and then
         re-merged in the new target sharing set."""
         potemplate = self.factory.makePOTemplate(name='template')
-        other_ps = self.factory.makeProductSeries(
-            product=potemplate.productseries.product)
+        product = potemplate.productseries.product
+        other_ps = self.factory.makeProductSeries(product=product)
+        product.development_focus = other_ps
         old_shared = self.factory.makePOTemplate(name='template',
                                                  productseries=other_ps)
         new_shared = self.factory.makePOTemplate(name='renamed',