drivefs-uprev: Return UprevVersionedPackageResult instead of None
When drivefs attempts to uprev to the same version, the |uprev_result|
is returned as None. If the return from the PUpr handler is None, the
controller throws an error. The |UprevVersionedPackageResult| should be
returned to allow the property |uprevved| to return false (unless the
|add_result| method has been called, in which case it will return true.
BUG=chromium:1118212
TEST=run_pytest
TEST=https://chromeos-swarming.appspot.com/task?id=5092ea4f05c1e910
Change-Id: I0558c94135a46ebcb38abe87c7c036e689feb55e
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2596696
Tested-by: Ben Reich <benreich@chromium.org>
Commit-Queue: Ben Reich <benreich@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Chris McDonald <cjmcdonald@chromium.org>
diff --git a/service/packages.py b/service/packages.py
index 4a152b9..95f68fe 100644
--- a/service/packages.py
+++ b/service/packages.py
@@ -297,38 +297,39 @@
"""
DRIVEFS_PATH_PREFIX = 'src/private-overlays/chromeos-overlay/chromeos-base'
+ result = uprev_lib.UprevVersionedPackageResult()
+ all_changed_files = []
drivefs_version = get_latest_drivefs_version_from_refs(refs)
if not drivefs_version:
# No valid DriveFS version is identified.
- return None
+ return result
logging.debug('DriveFS version determined from refs: %s', drivefs_version)
- result = uprev_lib.UprevVersionedPackageResult()
-
+ # Attempt to uprev drivefs package.
pkg_path = os.path.join(DRIVEFS_PATH_PREFIX, 'drivefs')
uprev_result = uprev_lib.uprev_workon_ebuild_to_version(pkg_path,
drivefs_version,
chroot)
- all_changed_files = []
if not uprev_result:
- return None # alternatively raise Exception
-
+ return result
all_changed_files.extend(uprev_result.changed_files)
+ # Attempt to uprev drivefs-ipc package.
pkg_path = os.path.join(DRIVEFS_PATH_PREFIX, 'drivefs-ipc')
-
uprev_result = uprev_lib.uprev_workon_ebuild_to_version(pkg_path,
drivefs_version,
chroot)
if not uprev_result:
- return None # alternatively raise Exception
+ logging.warning(
+ 'drivefs package has changed files %s but drivefs-ipc does not',
+ all_changed_files)
+ return result
all_changed_files.extend(uprev_result.changed_files)
-
result.add_result(drivefs_version, all_changed_files)
return result
diff --git a/service/packages_unittest.py b/service/packages_unittest.py
index 9c76bfa..5b35e54 100644
--- a/service/packages_unittest.py
+++ b/service/packages_unittest.py
@@ -1082,3 +1082,93 @@
def test_no_refs_returns_none(self):
"""Test no refs supplied."""
self.assertEqual(packages.get_latest_drivefs_version_from_refs([]), None)
+
+
+class UprevDrivefsTest(cros_test_lib.MockTestCase):
+ """Tests for uprev_drivefs."""
+
+ def setUp(self):
+ self.refs = [
+ GitRef(
+ path='/chromeos/platform/drivefs-google3/',
+ ref='refs/tags/drivefs_45.0.2',
+ revision='123')
+ ]
+ self.MOCK_DRIVEFS_EBUILD_PATH = 'drivefs.45.0.2-r1.ebuild'
+ self.MOCK_DRIVEFS_IPC_EBUILD_PATH = 'drivefs-ipc.45.0.2-r1.ebuild'
+
+ def revisionBumpOutcome(self, ebuild_path):
+ return uprev_lib.UprevResult(uprev_lib.Outcome.REVISION_BUMP, [ebuild_path])
+
+ def majorBumpOutcome(self, ebuild_path):
+ return uprev_lib.UprevResult(uprev_lib.Outcome.VERSION_BUMP, [ebuild_path])
+
+ def sameVersionOutcome(self):
+ return uprev_lib.UprevResult(uprev_lib.Outcome.SAME_VERSION_EXISTS)
+
+ def test_latest_version_returns_none(self):
+ """Test no refs were supplied"""
+ output = packages.uprev_drivefs(None, [], None)
+ self.assertFalse(output.uprevved)
+
+ def test_drivefs_uprev_fails(self):
+ """Test a single ref is supplied."""
+ self.PatchObject(
+ uprev_lib, 'uprev_workon_ebuild_to_version', side_effect=[None, None])
+ output = packages.uprev_drivefs(None, self.refs, None)
+ self.assertFalse(output.uprevved)
+
+ def test_same_version_exists(self):
+ """Test the same version exists uprev should not happen."""
+ drivefs_outcome = self.sameVersionOutcome()
+ drivefs_ipc_outcome = self.sameVersionOutcome()
+ self.PatchObject(
+ uprev_lib,
+ 'uprev_workon_ebuild_to_version',
+ side_effect=[drivefs_outcome, drivefs_ipc_outcome])
+ output = packages.uprev_drivefs(None, self.refs, None)
+ self.assertFalse(output.uprevved)
+
+ def test_revision_bump_just_drivefs_package(self):
+ """Test drivefs package uprevs not drivefs-ipc, should not uprev."""
+ drivefs_outcome = self.revisionBumpOutcome(self.MOCK_DRIVEFS_EBUILD_PATH)
+ self.PatchObject(
+ uprev_lib,
+ 'uprev_workon_ebuild_to_version',
+ side_effect=[drivefs_outcome, None])
+ output = packages.uprev_drivefs(None, self.refs, None)
+ self.assertFalse(output.uprevved)
+
+ def test_revision_bump_both_packages(self):
+ """Test both packages uprev, should succeed."""
+ drivefs_outcome = self.revisionBumpOutcome(self.MOCK_DRIVEFS_EBUILD_PATH)
+ drivefs_ipc_outcome = self.revisionBumpOutcome(
+ self.MOCK_DRIVEFS_IPC_EBUILD_PATH)
+ self.PatchObject(
+ uprev_lib,
+ 'uprev_workon_ebuild_to_version',
+ side_effect=[drivefs_outcome, drivefs_ipc_outcome])
+ output = packages.uprev_drivefs(None, self.refs, None)
+ self.assertTrue(output.uprevved)
+
+ def test_major_bump_only_drivefs_packages(self):
+ """Test drivefs package uprevs not drivefs-ipc, should not uprev."""
+ drivefs_outcome = self.majorBumpOutcome(self.MOCK_DRIVEFS_EBUILD_PATH)
+ self.PatchObject(
+ uprev_lib,
+ 'uprev_workon_ebuild_to_version',
+ side_effect=[drivefs_outcome, None])
+ output = packages.uprev_drivefs(None, self.refs, None)
+ self.assertFalse(output.uprevved)
+
+ def test_major_bump_both_packages(self):
+ """Test both packages uprev, should succeed."""
+ drivefs_outcome = self.majorBumpOutcome(self.MOCK_DRIVEFS_EBUILD_PATH)
+ drivefs_ipc_outcome = self.majorBumpOutcome(
+ self.MOCK_DRIVEFS_IPC_EBUILD_PATH)
+ self.PatchObject(
+ uprev_lib,
+ 'uprev_workon_ebuild_to_version',
+ side_effect=[drivefs_outcome, drivefs_ipc_outcome])
+ output = packages.uprev_drivefs(None, self.refs, None)
+ self.assertTrue(output.uprevved)
