Reland "deploy_chrome: Warn before deploying to device with mismatched board"
This is a reland of b35e6e3e8946efea1adcbe6e71033bd54ef34b90
If self.options.staging_only is true, _CheckBoard crashes because there
is no device. Thus the original change caused https://crbug.com/1085526.
Now _CheckBoard shall only be called after self.options.staging_only has
been checked and is false.
Original change's description:
> deploy_chrome: Warn before deploying to device with mismatched board
>
> deploy_chrome shall always require that a target board is specified,
> either from cros chrome_sdk or via --board. If the DUT board does not
> match, deploy_chrome shall log a warning, and unless --force is
> specified, it shall prompt for whether to continue.
>
> BUG=chromium:741966
> TEST=chromite/run_pytest
>
> Change-Id: Ia51491ebc82b99e127e1b523ce213f546abffe15
> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2125032
> Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
> Reviewed-by: Mike Frysinger <vapier@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Commit-Queue: Avery Musbach <amusbach@chromium.org>
> Tested-by: Avery Musbach <amusbach@chromium.org>
BUG=chromium:741966,chromium:1085526
TEST=chromite/run_pytest
Change-Id: I954f301f074dd89e1aebdfdd046e71bbb43f3d73
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2216412
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Tested-by: Avery Musbach <amusbach@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
diff --git a/scripts/deploy_chrome_unittest.py b/scripts/deploy_chrome_unittest.py
index 01c52fa..b8c784a 100644
--- a/scripts/deploy_chrome_unittest.py
+++ b/scripts/deploy_chrome_unittest.py
@@ -32,6 +32,7 @@
_REGULAR_TO = ('--to', 'monkey')
+_TARGET_BOARD = 'eve'
_GS_PATH = 'gs://foon'
@@ -42,27 +43,27 @@
class InterfaceTest(cros_test_lib.OutputTestCase):
"""Tests the commandline interface of the script."""
- BOARD = 'eve'
-
def testGsLocalPathUnSpecified(self):
"""Test no chrome path specified."""
with self.OutputCapturer():
- self.assertRaises2(SystemExit, _ParseCommandLine, list(_REGULAR_TO),
+ self.assertRaises2(SystemExit, _ParseCommandLine,
+ list(_REGULAR_TO) + ['--board', _TARGET_BOARD],
check_attrs={'code': 2})
def testGsPathSpecified(self):
"""Test case of GS path specified."""
- argv = list(_REGULAR_TO) + ['--gs-path', _GS_PATH]
+ argv = list(_REGULAR_TO) + ['--board', _TARGET_BOARD, '--gs-path', _GS_PATH]
_ParseCommandLine(argv)
def testLocalPathSpecified(self):
"""Test case of local path specified."""
- argv = list(_REGULAR_TO) + ['--local-pkg-path', '/path/to/chrome']
+ argv = list(_REGULAR_TO) + ['--board', _TARGET_BOARD, '--local-pkg-path',
+ '/path/to/chrome']
_ParseCommandLine(argv)
def testNoTarget(self):
"""Test no target specified."""
- argv = ['--gs-path', _GS_PATH]
+ argv = ['--board', _TARGET_BOARD, '--gs-path', _GS_PATH]
self.assertParseError(argv)
def assertParseError(self, argv):
@@ -70,44 +71,33 @@
self.assertRaises2(SystemExit, _ParseCommandLine, argv,
check_attrs={'code': 2})
- def testNoBoard(self):
- """Test cases where --board is not specified."""
- argv = ['--staging-only', '--build-dir=/path/to/nowhere']
- self.assertParseError(argv)
-
- # Don't need --board if no stripping is necessary.
- argv_nostrip = argv + ['--nostrip']
- _ParseCommandLine(argv_nostrip)
-
- # Don't need --board if strip binary is provided.
- argv_strip_bin = argv + ['--strip-bin', 'strip.bin']
- _ParseCommandLine(argv_strip_bin)
-
def testMountOptionSetsTargetDir(self):
- argv = list(_REGULAR_TO) + ['--gs-path', _GS_PATH, '--mount']
+ argv = list(_REGULAR_TO) + ['--board', _TARGET_BOARD, '--gs-path', _GS_PATH,
+ '--mount']
options = _ParseCommandLine(argv)
self.assertIsNot(options.target_dir, None)
def testMountOptionSetsMountDir(self):
- argv = list(_REGULAR_TO) + ['--gs-path', _GS_PATH, '--mount']
+ argv = list(_REGULAR_TO) + ['--board', _TARGET_BOARD, '--gs-path', _GS_PATH,
+ '--mount']
options = _ParseCommandLine(argv)
self.assertIsNot(options.mount_dir, None)
def testMountOptionDoesNotOverrideTargetDir(self):
- argv = list(_REGULAR_TO) + ['--gs-path', _GS_PATH, '--mount',
- '--target-dir', '/foo/bar/cow']
+ argv = list(_REGULAR_TO) + ['--board', _TARGET_BOARD, '--gs-path', _GS_PATH,
+ '--mount', '--target-dir', '/foo/bar/cow']
options = _ParseCommandLine(argv)
self.assertEqual(options.target_dir, '/foo/bar/cow')
def testMountOptionDoesNotOverrideMountDir(self):
- argv = list(_REGULAR_TO) + ['--gs-path', _GS_PATH, '--mount',
- '--mount-dir', '/foo/bar/cow']
+ argv = list(_REGULAR_TO) + ['--board', _TARGET_BOARD, '--gs-path', _GS_PATH,
+ '--mount', '--mount-dir', '/foo/bar/cow']
options = _ParseCommandLine(argv)
self.assertEqual(options.mount_dir, '/foo/bar/cow')
def testSshIdentityOptionSetsOption(self):
- argv = list(_REGULAR_TO) + ['--private-key', '/foo/bar/key',
- '--board', 'cedar',
+ argv = list(_REGULAR_TO) + ['--board', _TARGET_BOARD,
+ '--private-key', '/foo/bar/key',
'--build-dir', '/path/to/nowhere']
options = _ParseCommandLine(argv)
self.assertEqual(options.private_key, '/foo/bar/key')
@@ -159,12 +149,37 @@
def setUp(self):
self.deploy_mock = self.StartPatcher(DeployChromeMock())
- self.deploy = self._GetDeployChrome(
- list(_REGULAR_TO) + ['--gs-path', _GS_PATH, '--force', '--mount'])
+ self.deploy = self._GetDeployChrome(list(_REGULAR_TO) +
+ ['--board', _TARGET_BOARD, '--gs-path',
+ _GS_PATH, '--force', '--mount'])
self.remote_reboot_mock = \
self.PatchObject(remote_access.RemoteAccess, 'RemoteReboot',
return_value=True)
+
+class TestCheckIfBoardMatches(DeployTest):
+ """Testing checking whether the DUT board matches the target board."""
+
+ def testMatchedBoard(self):
+ """Test the case where the DUT board matches the target board."""
+ self.PatchObject(remote_access.ChromiumOSDevice, 'board', _TARGET_BOARD)
+ self.assertTrue(self.deploy.options.force)
+ self.deploy._CheckBoard()
+ self.deploy.options.force = False
+ self.deploy._CheckBoard()
+
+ def testMismatchedBoard(self):
+ """Test the case where the DUT board does not match the target board."""
+ self.PatchObject(remote_access.ChromiumOSDevice, 'board', 'cedar')
+ self.assertTrue(self.deploy.options.force)
+ self.deploy._CheckBoard()
+ self.deploy.options.force = False
+ self.PatchObject(cros_build_lib, 'BooleanPrompt', return_value=True)
+ self.deploy._CheckBoard()
+ self.PatchObject(cros_build_lib, 'BooleanPrompt', return_value=False)
+ self.assertRaises(deploy_chrome.DeployFailure, self.deploy._CheckBoard)
+
+
class TestDisableRootfsVerification(DeployTest):
"""Testing disabling of rootfs verification and RO mode."""
@@ -199,7 +214,7 @@
def testMountError(self):
"""Test that mount failure doesn't raise an exception by default."""
self.assertFalse(self.deploy._root_dir_is_still_readonly.is_set())
- self.PatchObject(remote_access.RemoteDevice, 'IsDirWritable',
+ self.PatchObject(remote_access.ChromiumOSDevice, 'IsDirWritable',
return_value=False, autospec=True)
self.deploy._MountRootfsAsWritable()
self.assertTrue(self.deploy._root_dir_is_still_readonly.is_set())
@@ -213,7 +228,7 @@
def testMountTempDir(self):
"""Test that mount succeeds if target dir is writable."""
self.assertFalse(self.deploy._root_dir_is_still_readonly.is_set())
- self.PatchObject(remote_access.RemoteDevice, 'IsDirWritable',
+ self.PatchObject(remote_access.ChromiumOSDevice, 'IsDirWritable',
return_value=True, autospec=True)
self.deploy._MountRootfsAsWritable()
self.assertFalse(self.deploy._root_dir_is_still_readonly.is_set())
@@ -276,9 +291,9 @@
def setUp(self):
self.staging_dir = os.path.join(self.tempdir, 'staging')
self.build_dir = os.path.join(self.tempdir, 'build_dir')
- self.common_flags = ['--build-dir', self.build_dir,
- '--board=eve', '--staging-only', '--cache-dir',
- self.tempdir]
+ self.common_flags = ['--board', _TARGET_BOARD,
+ '--build-dir', self.build_dir, '--staging-only',
+ '--cache-dir', self.tempdir]
self.sdk_mock = self.StartPatcher(cros_chrome_sdk_unittest.SDKFetcherMock())
self.PatchObject(
osutils, 'SourceEnvironment', autospec=True,
@@ -320,9 +335,9 @@
self.build_dir = os.path.join(self.tempdir, 'build_dir')
self.deploy_mock = self.StartPatcher(DeployChromeMock())
self.deploy = self._GetDeployChrome(
- list(_REGULAR_TO) + ['--build-dir', self.build_dir,
- '--board=eve', '--staging-only', '--cache-dir',
- self.tempdir, '--sloppy'])
+ list(_REGULAR_TO) + ['--board', _TARGET_BOARD,
+ '--build-dir', self.build_dir, '--staging-only',
+ '--cache-dir', self.tempdir, '--sloppy'])
def getCopyPath(self, source_path):
"""Return a chrome_util.Path or None if not present."""