Merge lp:~evfool/update-manager/fix665173 into lp:update-manager

[Robert Roth]

Show only one error message for sources not supporting changelogs, do not show one for the source and one for the binary source (this is how the message got duplicated).

[Barry Warsaw (barry)]

Thanks for your contribution. The patch looks okay to me (although I cannot upload it). Have you tried to add a test for this situation?

review: Approve

[Barry Warsaw (barry)]

Hmm, I guess the only problem could be that if one changelog source gave an HTTPError and then another source gave an BadStatusLine error (for example), you'd only see one of the two error messages, but maybe that's not terrible. If you were to get both errors, you've definitely got problems. ;)

[Robert Roth (evfool)]

I have tested it manually on a source not supporting changelogs, and the message is only shown once, as expected. Do you mean a unit-test? I think I could do that with a hard-coded launchpad changelog URL, that reports that changelogs are not supported. I'll check, I haven't written any Python unit-tests so fat, but I assume that the right place for this would be another method in the tests/test_changelog.py

[Barry Warsaw (barry)]

> I have tested it manually on a source not supporting changelogs, and the
> message is only shown once, as expected. Do you mean a unit-test? I think I
> could do that with a hard-coded launchpad changelog URL, that reports that
> changelogs are not supported. I'll check, I haven't written any Python unit-
> tests so fat, but I assume that the right place for this would be another
> method in the tests/test_changelog.py

I think that would be a good place for it. There should be lots of examples in the package to cargo cult from :). Give it a shot! Thanks.

[lp:~evfool/update-manager/fix665173]

2073. By Robert Roth on 2011-04-01

Unittest to test if the changelog not supported text gets in the changelog text only once

[Robert Roth (evfool)]

Really simple unittest added to check if the error message gets only once in the changelog. The package name is hardcoded for a package I have found that does not support changelogs, but not everyone will have that package though, so it could be replaced with a better package.

review: Needs Resubmitting

[Barry Warsaw (barry)]

Maybe mvo has some thoughts about that. The indentation looks a little weird in the merge proposal, but if the test passes for you then I think it's good. Let's see what Michael has to say. Thanks for doing this!

review: Approve

[Robert Roth (evfool)]

The requested changes have been commited.

review: Approve

[Michael Vogt (mvo)]

Hello! Thanks a lot for this branch (and the other branches!). Its getting a bit late here, but I will definitely merge this tonight or tomorrow.

[Michael Vogt (mvo)]

Thanks a bunch, merged (I modified the unittest a bit to make it not depend on a sources.list entry that has liboverlay-scrollbar-0.1-0

review: Approve

=== modified file 'UpdateManager/Core/MyCache.py'
--- UpdateManager/Core/MyCache.py	2010-08-11 06:52:22 +0000
+++ UpdateManager/Core/MyCache.py	2011-04-01 18:26:37 +0000
@@ -287,6 +287,7 @@
             # Try non official changelog location
             changelogs_uri_binary = self._guess_third_party_changelogs_uri_by_binary(name)
             changelogs_uri_source = self._guess_third_party_changelogs_uri_by_source(name)
+            error_message = ""
             for changelogs_uri in [changelogs_uri_binary,changelogs_uri_source]:
                 if changelogs_uri:
                     try:
@@ -294,15 +295,16 @@
                         self.all_changes[name] += changelog
                     except urllib2.HTTPError, e:
                         # no changelogs_uri or 404
-                        self.all_changes[name] += _(
+                        error_message = _(
                             "This change is not coming from a "
                             "source that supports changelogs.")
                     except (IOError, httplib.BadStatusLine, socket.error), e:
                         # network errors and others
                         logging.exception("error on changelog fetching")
-                        self.all_changes[name] += _(
+                        error_message = _(
                             "Failed to download the list of changes. \n"
                             "Please check your Internet connection.")
+	    self.all_changes[name] += error_message
             return
         # fixup epoch handling version
         srcpkg = self[name].sourcePackageName

=== modified file 'tests/test_changelog.py'
--- tests/test_changelog.py	2010-08-10 12:22:09 +0000
+++ tests/test_changelog.py	2011-04-01 18:26:37 +0000
@@ -37,7 +37,12 @@
         pkg = self.cache[pkgname]
         self.assertTrue(pkg.candidate.version in uri)
         self.assertTrue("gtk+2.0" in uri)
-
+    def test_changelog_not_supported(self):
+        pkgname = "liboverlay-scrollbar-0.1-0"
+        # test binary changelogs
+        self.cache.get_changelog(pkgname)
+	error = "This change is not coming from a source that supports changelogs."
+	self.assertEqual(self.cache.all_changes[pkgname].count(error),1)        
 
 if __name__ == '__main__':
     if len(sys.argv) > 1 and sys.argv[1] == "-v":