ebuild: fetch: Factor out _get_checksum_failure_max_tries

The current fetch() function is quite long, which makes it hard to
know what I can change without adverse side effects.  By pulling this
logic out of the main function, we get clearer logic in fetch() and
more explicit input for the config extraction.

Following a suggestion by Tom Wijsman, I put the setting name in a new
'key' variable to cut down on the PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS
noise.

diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py
index 5316f03fbe53c8fd45ba845fd658d7a57cb28dd8..911500ad1e32e75db88dd36388cbca1060ea8ebc 100644 (file)
--- a/pym/portage/package/ebuild/fetch.py
+++ b/pym/portage/package/ebuild/fetch.py
@@ -240,6 +240,40 @@ _size_suffix_map = {
        'Y' : 80,
 }
 
+
+def _get_checksum_failure_max_tries(settings, default=5):
+       """
+       Get the maximum number of failed download attempts.
+
+       Generally, downloading the same file repeatedly from
+       every single available mirror is a waste of bandwidth
+       and time, so there needs to be a cap.
+       """
+       key = 'PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS'
+       v = default
+       try:
+               v = int(settings.get(key, default))
+       except (ValueError, OverflowError):
+               writemsg(_("!!! Variable %s contains "
+                       "non-integer value: '%s'\n")
+                       % (key, settings[key]),
+                       noiselevel=-1)
+               writemsg(_("!!! Using %s default value: %s\n")
+                       % (key, default),
+                       noiselevel=-1)
+               v = default
+       if v < 1:
+               writemsg(_("!!! Variable %s contains "
+                       "value less than 1: '%s'\n")
+                       % (key, v),
+                       noiselevel=-1)
+               writemsg(_("!!! Using %s default value: %s\n")
+                       % (key, default),
+                       noiselevel=-1)
+               v = default
+       return v
+
+
 def fetch(myuris, mysettings, listonly=0, fetchonly=0,
        locks_in_subdir=".locks", use_locks=1, try_mirrors=1, digests=None,
        allow_missing_digests=True):
@@ -263,31 +297,8 @@ def fetch(myuris, mysettings, listonly=0, fetchonly=0,
                        print(_(">>> \"mirror\" mode desired and \"mirror\" restriction found; skipping fetch."))
                        return 1
 
-       # Generally, downloading the same file repeatedly from
-       # every single available mirror is a waste of bandwidth
-       # and time, so there needs to be a cap.
-       checksum_failure_max_tries = 5
-       v = checksum_failure_max_tries
-       try:
-               v = int(mysettings.get("PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS",
-                       checksum_failure_max_tries))
-       except (ValueError, OverflowError):
-               writemsg(_("!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"
-                       " contains non-integer value: '%s'\n") % \
-                       mysettings["PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"], noiselevel=-1)
-               writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS "
-                       "default value: %s\n") % checksum_failure_max_tries,
-                       noiselevel=-1)
-               v = checksum_failure_max_tries
-       if v < 1:
-               writemsg(_("!!! Variable PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS"
-                       " contains value less than 1: '%s'\n") % v, noiselevel=-1)
-               writemsg(_("!!! Using PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS "
-                       "default value: %s\n") % checksum_failure_max_tries,
-                       noiselevel=-1)
-               v = checksum_failure_max_tries
-       checksum_failure_max_tries = v
-       del v
+       checksum_failure_max_tries = _get_checksum_failure_max_tries(
+               settings=mysettings)
 
        fetch_resume_size_default = "350K"
        fetch_resume_size = mysettings.get("PORTAGE_FETCH_RESUME_MIN_SIZE")

diff --git a/pym/portage/tests/ebuild/test_fetch.py b/pym/portage/tests/ebuild/test_fetch.py
new file mode 100644 (file)
index 0000000..26e0349
--- /dev/null
+++ b/pym/portage/tests/ebuild/test_fetch.py
@@ -0,0 +1,45 @@
+# Copyright 1998-2013 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+from portage.package.ebuild.fetch import (
+       _get_checksum_failure_max_tries,
+       )
+from portage.tests import TestCase
+
+
+class FetchTestCase(TestCase):
+       """
+       Test fetch and it's helper functions.
+
+       The fetch function, as it stands, is too complicated to test
+       on its own.  However, the new helper functions are much more
+       limited and easier to test.  Despite these tests, the helper
+       functions are internal implementation details, and their
+       presence and interface may change at any time.  Do not use
+       them directly (outside of these tests).
+       """
+
+       def test_get_checksum_failure_max_tries(self):
+               self.assertEqual(
+                       _get_checksum_failure_max_tries(settings={}),
+                       5)
+               self.assertEqual(
+                       _get_checksum_failure_max_tries(settings={
+                               'PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS': ''}),
+                       5)
+               self.assertEqual(
+                       _get_checksum_failure_max_tries(settings={
+                               'PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS': '3'}),
+                       3)
+               self.assertEqual(
+                       _get_checksum_failure_max_tries(settings={
+                               'PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS': '-1'}),
+                       5)
+               self.assertEqual(
+                       _get_checksum_failure_max_tries(settings={
+                               'PORTAGE_FETCH_CHECKSUM_TRY_MIRRORS': 'oops'}),
+                       5)
+               self.assertEqual(
+                       _get_checksum_failure_max_tries(
+                               settings={}, default=3),
+                       3)