Code review comment for lp:~launchpad/launchpad/translation-sharing-status
- translation-sharing-status
- Merge into devel
Revision history for this message
| Henning Eggers (henninge) wrote : | # |
| 1 | === modified file 'lib/lp/translations/browser/tests/test_sharing_details.py' |
| 2 | --- lib/lp/translations/browser/tests/test_sharing_details.py 2011-03-15 10:48:28 +0000 |
| 3 | +++ lib/lp/translations/browser/tests/test_sharing_details.py 2011-03-15 18:58:11 +0000 |
| 4 | @@ -156,14 +156,14 @@ |
| 5 | self.assertFalse(self.view.is_upstream_synchronization_enabled) |
| 6 | |
| 7 | def test_is_upstream_synchronization_enabled__no_import(self): |
| 8 | - # If the source package is not linked to an upstream series, |
| 9 | + # If no synchronization is enabled on the upstream series, |
| 10 | # is_upstream_synchronization_enabled returns False. |
| 11 | self.configureSharing( |
| 12 | translation_import_mode=TranslationsBranchImportMode.NO_IMPORT) |
| 13 | self.assertFalse(self.view.is_upstream_synchronization_enabled) |
| 14 | |
| 15 | def test_is_upstream_synchronization_enabled__import_templates(self): |
| 16 | - # If the source package is not linked to an upstream series, |
| 17 | + # If only template synchronization is enabled on the upstream series, |
| 18 | # is_upstream_synchronization_enabled returns False. |
| 19 | self.configureSharing( |
| 20 | translation_import_mode= |
| 21 | @@ -171,8 +171,8 @@ |
| 22 | self.assertFalse(self.view.is_upstream_synchronization_enabled) |
| 23 | |
| 24 | def test_is_upstream_synchronization_enabled__import_translations(self): |
| 25 | - # If the source package is not linked to an upstream series, |
| 26 | - # is_upstream_synchronization_enabled returns False. |
| 27 | + # If full translation synchronization is enabled on the upstream |
| 28 | + # series, is_upstream_synchronization_enabled returns False. |
| 29 | self.configureSharing( |
| 30 | translation_import_mode= |
| 31 | TranslationsBranchImportMode.IMPORT_TRANSLATIONS) |
| 32 | @@ -301,7 +301,7 @@ |
| 33 | return self.factory.makeSourcePackage(distroseries=distroseries) |
| 34 | |
| 35 | def _makeFullyConfiguredSharing(self): |
| 36 | - """Remove some redundant code from the tests.""" |
| 37 | + """Setup a fully configured sharing scenario.""" |
| 38 | packaging = self.factory.makePackagingLink(in_ubuntu=True) |
| 39 | productseries = packaging.productseries |
| 40 | sourcepackage = packaging.sourcepackage |
| 41 | @@ -313,12 +313,15 @@ |
| 42 | TranslationsBranchImportMode.IMPORT_TRANSLATIONS)) |
| 43 | return (sourcepackage, productseries) |
| 44 | |
| 45 | + def _getSharingDetailsViewBrowser(self, sourcepackage): |
| 46 | + return self.getViewBrowser( |
| 47 | + sourcepackage, no_login=True, rootsite="translations", |
| 48 | + view_name="+sharing-details") |
| 49 | + |
| 50 | def test_checklist_unconfigured(self): |
| 51 | # Without a packaging link, sharing is completely unconfigured |
| 52 | sourcepackage = self._makeSourcePackage() |
| 53 | - browser = self.getViewBrowser( |
| 54 | - sourcepackage, no_login=True, rootsite="translations", |
| 55 | - view_name="+sharing-details") |
| 56 | + browser = self._getSharingDetailsViewBrowser(sourcepackage) |
| 57 | checklist = find_tag_by_id(browser.contents, 'sharing-checklist') |
| 58 | self.assertIsNot(None, checklist) |
| 59 | self.assertTextMatchesExpressionIgnoreWhitespace(""" |
| 60 | @@ -332,9 +335,7 @@ |
| 61 | def test_checklist_partly_configured(self): |
| 62 | # Linking a source package takes care of one item. |
| 63 | packaging = self.factory.makePackagingLink(in_ubuntu=True) |
| 64 | - browser = self.getViewBrowser( |
| 65 | - packaging.sourcepackage, no_login=True, rootsite="translations", |
| 66 | - view_name="+sharing-details") |
| 67 | + browser = self._getSharingDetailsViewBrowser(packaging.sourcepackage) |
| 68 | checklist = find_tag_by_id(browser.contents, 'sharing-checklist') |
| 69 | self.assertIsNot(None, checklist) |
| 70 | self.assertTextMatchesExpressionIgnoreWhitespace(""" |
| 71 | @@ -349,9 +350,7 @@ |
| 72 | def test_checklist_fully_configured(self): |
| 73 | # A fully configured sharing setup. |
| 74 | sourcepackage = self._makeFullyConfiguredSharing()[0] |
| 75 | - browser = self.getViewBrowser( |
| 76 | - sourcepackage, no_login=True, rootsite="translations", |
| 77 | - view_name="+sharing-details") |
| 78 | + browser = self._getSharingDetailsViewBrowser(sourcepackage) |
| 79 | checklist = find_tag_by_id(browser.contents, 'sharing-checklist') |
| 80 | self.assertIsNot(None, checklist) |
| 81 | self.assertTextMatchesExpressionIgnoreWhitespace(""" |
| 82 | @@ -359,7 +358,7 @@ |
| 83 | Linked upstream series is .+ trunk series. |
| 84 | Change upstream link Remove upstream link |
| 85 | Upstream source branch is .+[.] |
| 86 | - Translations are enable on the upstream project. |
| 87 | + Translations are enabled on the upstream project. |
| 88 | Automatic synchronization of translations is enabled.""", |
| 89 | extract_text(checklist)) |
| 90 | |
| 91 | @@ -368,9 +367,7 @@ |
| 92 | sourcepackage = self._makeSourcePackage() |
| 93 | self.factory.makePOTemplate( |
| 94 | name='foo-template', sourcepackage=sourcepackage) |
| 95 | - browser = self.getViewBrowser( |
| 96 | - sourcepackage, no_login=True, rootsite="translations", |
| 97 | - view_name="+sharing-details") |
| 98 | + browser = self._getSharingDetailsViewBrowser(sourcepackage) |
| 99 | tbody = find_tag_by_id( |
| 100 | browser.contents, 'template-table').find('tbody') |
| 101 | self.assertIsNot(None, tbody) |
| 102 | @@ -386,9 +383,7 @@ |
| 103 | name=template_name, sourcepackage=sourcepackage) |
| 104 | self.factory.makePOTemplate( |
| 105 | name=template_name, productseries=productseries) |
| 106 | - browser = self.getViewBrowser( |
| 107 | - sourcepackage, no_login=True, rootsite="translations", |
| 108 | - view_name="+sharing-details") |
| 109 | + browser = self._getSharingDetailsViewBrowser(sourcepackage) |
| 110 | tbody = find_tag_by_id( |
| 111 | browser.contents, 'template-table').find('tbody') |
| 112 | self.assertIsNot(None, tbody) |
| 113 | @@ -404,9 +399,7 @@ |
| 114 | template_name = 'foo-template' |
| 115 | self.factory.makePOTemplate( |
| 116 | name=template_name, productseries=productseries) |
| 117 | - browser = self.getViewBrowser( |
| 118 | - sourcepackage, no_login=True, rootsite="translations", |
| 119 | - view_name="+sharing-details") |
| 120 | + browser = self._getSharingDetailsViewBrowser(sourcepackage) |
| 121 | tbody = find_tag_by_id( |
| 122 | browser.contents, 'template-table').find('tbody') |
| 123 | self.assertIsNot(None, tbody) |
| 124 | @@ -418,9 +411,7 @@ |
| 125 | # When sharing is fully configured but no upstream templates are |
| 126 | # found, a message is displayed. |
| 127 | sourcepackage = self._makeFullyConfiguredSharing()[0] |
| 128 | - browser = self.getViewBrowser( |
| 129 | - sourcepackage, no_login=True, rootsite="translations", |
| 130 | - view_name="+sharing-details") |
| 131 | + browser = self._getSharingDetailsViewBrowser(sourcepackage) |
| 132 | self.assertEqual( |
| 133 | ["No upstream templates have been found yet. Please follow " |
| 134 | "the import process by going to the Translation Import Queue " |
| 135 | @@ -432,9 +423,7 @@ |
| 136 | # message should be displayed. |
| 137 | sourcepackage, productseries = self._makeFullyConfiguredSharing() |
| 138 | self.factory.makePOTemplate(productseries=productseries) |
| 139 | - browser = self.getViewBrowser( |
| 140 | - sourcepackage, no_login=True, rootsite="translations", |
| 141 | - view_name="+sharing-details") |
| 142 | + browser = self._getSharingDetailsViewBrowser(sourcepackage) |
| 143 | self.assertEqual([], get_feedback_messages(browser.contents)) |
| 144 | |
| 145 | def test_no_message_with_incomplate_sharing(self): |
| 146 | @@ -444,7 +433,5 @@ |
| 147 | productseries = packaging.productseries |
| 148 | sourcepackage = packaging.sourcepackage |
| 149 | self.factory.makePOTemplate(productseries=productseries) |
| 150 | - browser = self.getViewBrowser( |
| 151 | - sourcepackage, no_login=True, rootsite="translations", |
| 152 | - view_name="+sharing-details") |
| 153 | + browser = self._getSharingDetailsViewBrowser(sourcepackage) |
| 154 | self.assertEqual([], get_feedback_messages(browser.contents)) |
| 155 | |
| 156 | === modified file 'lib/lp/translations/templates/sourcepackage-sharing-details.pt' |
| 157 | --- lib/lp/translations/templates/sourcepackage-sharing-details.pt 2011-03-14 15:43:14 +0000 |
| 158 | +++ lib/lp/translations/templates/sourcepackage-sharing-details.pt 2011-03-15 18:45:49 +0000 |
| 159 | @@ -56,7 +56,7 @@ |
| 160 | </li> |
| 161 | <li class="sprite yes" |
| 162 | tal:condition="view/is_upstream_translations_enabled"> |
| 163 | - Translations are enable on the upstream project. |
| 164 | + Translations are enabled on the upstream project. |
| 165 | <a tal:condition="view/is_packaging_configured" |
| 166 | tal:replace="structure context/productseries/product/menu:translations/settings/fmt:icon" /> |
| 167 | </li> |
Thank you for your review! I fixed the issues and also put the getViewBrowser
repetitions in a method. That was something I had already been considering.