u1-test-utils

Merge lp:~vila/u1-test-utils/sudo-opt-in into lp:u1-test-utils

  1. sudo-opt-in
  2. Merge into trunk
Proposed by Vincent Ladeuil
Status: Merged
Approved by: Vincent Ladeuil
Approved revision: 95
Merged at revision: 96
Proposed branch: lp:~vila/u1-test-utils/sudo-opt-in
Merge into: lp:u1-test-utils
Diff against target: 102 lines (+17/-14)
2 files modified
setup_vm/tests/__init__.py (+11/-8)
setup_vm/tests/test_setup_vm.py (+6/-6)
To merge this branch: bzr merge lp:~vila/u1-test-utils/sudo-opt-in
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Leo Arias (community) code review Approve
Review via email: mp+179952@code.launchpad.net

Commit message

Tests requiring sudo should use an opt-in mechanism, nobody should blindly accept running them.

Description of the change

sudo access is hard to guess but it doesn't imply the user want to use it during tests.

This use ~/.setup_vm.use_sudo_for_tests as an opt-in mechanism instead.

To post a comment you must log in.
Revision history for this message
Leo Arias (elopio) :
review: Approve (code review)
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'setup_vm/tests/__init__.py'
--- setup_vm/tests/__init__.py 2013-08-08 10:10:01 +0000
+++ setup_vm/tests/__init__.py 2013-08-13 16:04:05 +0000
@@ -133,29 +133,32 @@
133qemu_img_feature = ExecutableFeature('qemu-img')133qemu_img_feature = ExecutableFeature('qemu-img')
134134
135135
136class _SudoFeature(Feature):136class _UseSudoForTestsFeature(Feature):
137 """User has sudo access.137 """User has sudo access.
138138
139 There is no direct way to test for sudo access other than trying to use139 There is no direct way to test for sudo access other than trying to use
140 it. This is not something we can do in automated tests as it requires user140 it. This is not something we can do in automated tests as it requires user
141 input.141 input.
142142
143 Instead, we look for '~/.sudo_as_admin_succesful' as a proof that the user143 Whatever trick is used to guess whether or not the user *can* sudo won't
144 succeeded in the past.144 tell us if she agrees to run the sudo tests. Instead, this should rely on
145145 an opt-in mechanism so each user decides whether or not she wants to run
146 This is mostly useful to avoid attempting to use sudo on jenkins where it's146 the tests.
147 unlikely to be configured.
148 """147 """
149148
150 def _probe(self):149 def _probe(self):
151 path = os.path.expanduser('~/.sudo_as_admin_successful')150 # I.e. if you want to run the tests that requires sudo issue:
151 # $ touch ~/.setup_vm.use_sudo_for_tests
152 # if you don't, issue:
153 # $ rm -f ~/.setup_vm.use_sudo_for_tests
154 path = os.path.expanduser('~/.setup_vm.use_sudo_for_tests')
152 return os.path.exists(path)155 return os.path.exists(path)
153156
154 def feature_name(self):157 def feature_name(self):
155 return 'sudo access'158 return 'sudo access'
156159
157160
158sudo_feature = _SudoFeature()161use_sudo_for_tests_feature = _UseSudoForTestsFeature()
159162
160163
161class _SshFeature(ExecutableFeature):164class _SshFeature(ExecutableFeature):
162165
=== modified file 'setup_vm/tests/test_setup_vm.py'
--- setup_vm/tests/test_setup_vm.py 2013-08-09 08:06:27 +0000
+++ setup_vm/tests/test_setup_vm.py 2013-08-13 16:04:05 +0000
@@ -169,7 +169,7 @@
169# level and be done -- vila 2013-08-06169# level and be done -- vila 2013-08-06
170170
171 def setUp(self):171 def setUp(self):
172 tests.requires_feature(self, tests.sudo_feature)172 tests.requires_feature(self, tests.use_sudo_for_tests_feature)
173 # Downloading real isos or images is too long for tests, instead, we173 # Downloading real isos or images is too long for tests, instead, we
174 # fake it by downloading a small but known to exist file: MD5SUMS174 # fake it by downloading a small but known to exist file: MD5SUMS
175 super(TestDownload, self).setUp()175 super(TestDownload, self).setUp()
@@ -637,7 +637,7 @@
637class TestSeedImage(TestCaseWithHome):637class TestSeedImage(TestCaseWithHome):
638638
639 def setUp(self):639 def setUp(self):
640 tests.requires_feature(self, tests.sudo_feature)640 tests.requires_feature(self, tests.use_sudo_for_tests_feature)
641 tests.requires_feature(self, tests.qemu_img_feature)641 tests.requires_feature(self, tests.qemu_img_feature)
642 super(TestSeedImage, self).setUp()642 super(TestSeedImage, self).setUp()
643 self.conf = svm.VmStack('foo')643 self.conf = svm.VmStack('foo')
@@ -662,7 +662,7 @@
662class TestImageFromCloud(TestCaseWithHome):662class TestImageFromCloud(TestCaseWithHome):
663663
664 def setUp(self):664 def setUp(self):
665 tests.requires_feature(self, tests.sudo_feature)665 tests.requires_feature(self, tests.use_sudo_for_tests_feature)
666 tests.requires_feature(self, tests.qemu_img_feature)666 tests.requires_feature(self, tests.qemu_img_feature)
667 super(TestImageFromCloud, self).setUp()667 super(TestImageFromCloud, self).setUp()
668 self.conf = svm.VmStack('foo')668 self.conf = svm.VmStack('foo')
@@ -696,7 +696,7 @@
696class TestImageWithBacking(TestCaseWithHome):696class TestImageWithBacking(TestCaseWithHome):
697697
698 def setUp(self):698 def setUp(self):
699 tests.requires_feature(self, tests.sudo_feature)699 tests.requires_feature(self, tests.use_sudo_for_tests_feature)
700 (download_cache_dir,700 (download_cache_dir,
701 reference_cloud_image_name) = requires_known_reference_image(self)701 reference_cloud_image_name) = requires_known_reference_image(self)
702 super(TestImageWithBacking, self).setUp()702 super(TestImageWithBacking, self).setUp()
@@ -870,7 +870,7 @@
870class TestInstallWithSeed(TestCaseWithHome):870class TestInstallWithSeed(TestCaseWithHome):
871871
872 def setUp(self):872 def setUp(self):
873 tests.requires_feature(self, tests.sudo_feature)873 tests.requires_feature(self, tests.use_sudo_for_tests_feature)
874 (download_cache,874 (download_cache,
875 reference_cloud_image_name) = requires_known_reference_image(self)875 reference_cloud_image_name) = requires_known_reference_image(self)
876 super(TestInstallWithSeed, self).setUp()876 super(TestInstallWithSeed, self).setUp()
@@ -907,7 +907,7 @@
907class TestInstallWithBacking(TestCaseWithHome):907class TestInstallWithBacking(TestCaseWithHome):
908908
909 def setUp(self):909 def setUp(self):
910 tests.requires_feature(self, tests.sudo_feature)910 tests.requires_feature(self, tests.use_sudo_for_tests_feature)
911 (download_cache_dir,911 (download_cache_dir,
912 reference_cloud_image_name) = requires_known_reference_image(self)912 reference_cloud_image_name) = requires_known_reference_image(self)
913 super(TestInstallWithBacking, self).setUp()913 super(TestInstallWithBacking, self).setUp()

Subscribers

People subscribed via source and target branches

to all changes: