[gentoo-portage-dev] [PATCH v3 1/4] pym/portage/package/ebuild/fetch.py: Factor out _get_checksum_failure_max_tries

[W. Trevor King]

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.
---
 pym/portage/package/ebuild/fetch.py    | 61 ++++++++++++++++++++--------------
 pym/portage/tests/ebuild/test_fetch.py | 45 +++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 25 deletions(-)
 create mode 100644 pym/portage/tests/ebuild/test_fetch.py

diff --git a/pym/portage/package/ebuild/fetch.py b/pym/portage/package/ebuild/fetch.py
index 5316f03..911500a 100644
--- 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
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)
-- 
1.8.5.2.8.g0f6c0d1