Merge lp:~gmb/maas-test/fix-up-proxy-config-dir-for-tests into lp:maas-test
- fix-up-proxy-config-dir-for-tests
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 78 |
Proposed branch: | lp:~gmb/maas-test/fix-up-proxy-config-dir-for-tests |
Merge into: | lp:maas-test |
Diff against target: |
321 lines (+50/-42) 6 files modified
maastest/cases.py (+2/-1) maastest/kvmfixture.py (+9/-2) maastest/proxyfixture.py (+10/-9) maastest/tests/test_kvmfixture.py (+3/-8) maastest/tests/test_proxyfixture.py (+24/-22) packages.txt (+2/-0) |
To merge this branch: | bzr merge lp:~gmb/maas-test/fix-up-proxy-config-dir-for-tests |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email: mp+196918@code.launchpad.net |
Commit message
Description of the change
This branch makes the config_dir of LocalProxyFixture, and the ssh_key_dir of KVMFixture configurable. It also adds a command-line option, --config-dir, which sets both of the above (there's an argument for having ssh_key_dir separate, I suppose; I'm happy to do that if it's a better fit).
- 77. By Graham Binns
-
Fixed a typo.
- 78. By Graham Binns
-
Removed TODO that was TODONE.
Julian Edwards (julian-edwards) wrote : | # |
On Wednesday 27 Nov 2013 16:28:29 you wrote:
> Review: Needs Information
>
> Looks good, but I'm concerned about punting everything to the user.
>
>
> [1]
>
> + # maas-test config
> + parser.
> + '--config-dir', type=text_type,
> + default=
> + help="The directory in which to store maas-test config and cache "
> + "files. Defaults to %(default)s")
>
> I think we should use the xdg stuff I mentioned to rvb yesterday, and
> not have yet another configuration option. Or is there a compelling
> use-case for allowing this? This feels like getting the user to choose
> because we can't make a decision.
+1 to removing config where sane and and possible.
Graham Binns (gmb) wrote : | # |
On 27 November 2013 22:51, Julian Edwards <email address hidden> wrote:
>> I think we should use the xdg stuff I mentioned to rvb yesterday, and
>> not have yet another configuration option. Or is there a compelling
>> use-case for allowing this? This feels like getting the user to choose
>> because we can't make a decision.
I saw that whizz past in a merge proposal and promptly forgot about
it. That makes perfect sense to me.
Julian Edwards (julian-edwards) wrote : | # |
On Thursday 28 Nov 2013 07:21:34 you wrote:
> On 27 November 2013 22:51, Julian Edwards <email address hidden>
wrote:
> >> I think we should use the xdg stuff I mentioned to rvb yesterday, and
> >> not have yet another configuration option. Or is there a compelling
> >> use-case for allowing this? This feels like getting the user to choose
> >> because we can't make a decision.
>
> I saw that whizz past in a merge proposal and promptly forgot about
> it. That makes perfect sense to me.
For the record, I didn't say that, Gavin did :)
Graham Binns (gmb) wrote : | # |
On 28 November 2013 07:24, Julian Edwards <email address hidden> wrote:
> For the record, I didn't say that, Gavin did :)
Yeah, trimming fail.
- 79. By Graham Binns
-
Removed config-dir option stuff; we'll use xdg instead.
- 80. By Graham Binns
-
XDG conversion.
- 81. By Graham Binns
-
Merged trunk.
- 82. By Graham Binns
-
Added dependencies.
- 83. By Graham Binns
-
Trimmed some lint.
- 84. By Graham Binns
-
Removed a TODO.
Gavin Panella (allenap) wrote : | # |
Tip top. One more small thing:
[2]
- os.path.
+ BaseDirectory.
No need for the leading . on .maas-test.
This appears elsewhere too.
Also, fwiw, any number of components can passed to save_config_path(),
for example:
would create a directory at $XDG_CONFIG_
Graham Binns (gmb) wrote : | # |
On 28 November 2013 10:03, Gavin Panella <email address hidden> wrote:
> Review: Approve
>
> Tip top. One more small thing:
>
>
> [2]
>
> - os.path.
> + BaseDirectory.
>
> No need for the leading . on .maas-test.
>
> This appears elsewhere too.
On it.
> Also, fwiw, any number of components can passed to save_config_path(),
> for example:
>
> BaseDirectory.
>
> would create a directory at $XDG_CONFIG_
Ah, but we don't want to create a directory there (I've improved the
code so that's more obvious). vm_ssh_id_rsa is the name of the
keyfile, so we need to make sure the directory is there and then
create the path to the file from it. As I said, I've improved the
clarity.
- 85. By Graham Binns
-
Fixed an assortment of wonderful things.
- 86. By Graham Binns
-
Removed unused imports.
Gavin Panella (allenap) wrote : | # |
> Ah, but we don't want to create a directory there (I've improved the
> code so that's more obvious).
I did understand that - your code was clear - I was just pointing it out
for future reference.
Graham Binns (gmb) wrote : | # |
On 28 November 2013 14:25, Gavin Panella <email address hidden> wrote:
> I did understand that - your code was clear - I was just pointing it out
> for future reference.
Okay, cool. Thanks for the clarification.
Preview Diff
1 | === modified file 'maastest/cases.py' | |||
2 | --- maastest/cases.py 2013-11-27 17:22:21 +0000 | |||
3 | +++ maastest/cases.py 2013-11-28 14:16:14 +0000 | |||
4 | @@ -129,11 +129,12 @@ | |||
5 | 129 | "URL: http://%s/MAAS/\n" | 129 | "URL: http://%s/MAAS/\n" |
6 | 130 | "Username: %s\n" | 130 | "Username: %s\n" |
7 | 131 | "Password: %s\n" | 131 | "Password: %s\n" |
9 | 132 | "SSH login: ssh -i ~/.maas-test/vm_ssh_id_rsa %s\n" | 132 | "SSH login: ssh -i %s %s\n" |
10 | 133 | % ( | 133 | % ( |
11 | 134 | self.machine.ip_address(), | 134 | self.machine.ip_address(), |
12 | 135 | self.maas.admin_user, | 135 | self.maas.admin_user, |
13 | 136 | self.maas.admin_password, | 136 | self.maas.admin_password, |
14 | 137 | self.machine.ssh_private_key_file, | ||
15 | 137 | self.machine.identity(), | 138 | self.machine.identity(), |
16 | 138 | )) | 139 | )) |
17 | 139 | 140 | ||
18 | 140 | 141 | ||
19 | === modified file 'maastest/kvmfixture.py' | |||
20 | --- maastest/kvmfixture.py 2013-11-27 13:36:27 +0000 | |||
21 | +++ maastest/kvmfixture.py 2013-11-28 14:16:14 +0000 | |||
22 | @@ -34,6 +34,7 @@ | |||
23 | 34 | import netaddr | 34 | import netaddr |
24 | 35 | from six import text_type | 35 | from six import text_type |
25 | 36 | from testtools.content import text_content | 36 | from testtools.content import text_content |
26 | 37 | from xdg import BaseDirectory | ||
27 | 37 | 38 | ||
28 | 38 | # Maximum wait for the virtual machine to complete its cloud-init | 39 | # Maximum wait for the virtual machine to complete its cloud-init |
29 | 39 | # initialization. It can take quite a while, because it performs an | 40 | # initialization. It can take quite a while, because it performs an |
30 | @@ -67,7 +68,7 @@ | |||
31 | 67 | """ | 68 | """ |
32 | 68 | def __init__(self, series, architecture, proxy_url, | 69 | def __init__(self, series, architecture, proxy_url, |
33 | 69 | direct_interface=None, direct_network=None, name=None, | 70 | direct_interface=None, direct_network=None, name=None, |
35 | 70 | archive=None): | 71 | archive=None, ssh_key_dir=None): |
36 | 71 | """Initialise a KVM-based Virtual Machine fixture. | 72 | """Initialise a KVM-based Virtual Machine fixture. |
37 | 72 | 73 | ||
38 | 73 | The VM's network is configured like this: | 74 | The VM's network is configured like this: |
39 | @@ -103,6 +104,10 @@ | |||
40 | 103 | :param archive: An optional repository name. If provided, the | 104 | :param archive: An optional repository name. If provided, the |
41 | 104 | repository will be added onto the VM. | 105 | repository will be added onto the VM. |
42 | 105 | :type archive: string | 106 | :type archive: string |
43 | 107 | :param ssh_key_dir: The directory in which to store the SSH keys | ||
44 | 108 | for connecting to this KVM instance. If None, this defaults | ||
45 | 109 | to the default XDG config directory. | ||
46 | 110 | :type ssh_key_dir: string. | ||
47 | 106 | """ | 111 | """ |
48 | 107 | super(KVMFixture, self).__init__() | 112 | super(KVMFixture, self).__init__() |
49 | 108 | self.command_count = itertools.count(1) | 113 | self.command_count = itertools.count(1) |
50 | @@ -124,8 +129,10 @@ | |||
51 | 124 | if name is None: | 129 | if name is None: |
52 | 125 | name = text_type(uuid1()) | 130 | name = text_type(uuid1()) |
53 | 126 | self.name = name | 131 | self.name = name |
54 | 132 | if ssh_key_dir is None: | ||
55 | 133 | ssh_key_dir = BaseDirectory.save_config_path('maas-test') | ||
56 | 127 | self.ssh_private_key_file = os.path.join( | 134 | self.ssh_private_key_file = os.path.join( |
58 | 128 | os.path.expanduser('~'), '.maas-test', 'vm_ssh_id_rsa') | 135 | ssh_key_dir, 'vm_ssh_id_rsa') |
59 | 129 | self.running = False | 136 | self.running = False |
60 | 130 | 137 | ||
61 | 131 | def setUp(self): | 138 | def setUp(self): |
62 | 132 | 139 | ||
63 | === modified file 'maastest/proxyfixture.py' | |||
64 | --- maastest/proxyfixture.py 2013-11-27 13:36:27 +0000 | |||
65 | +++ maastest/proxyfixture.py 2013-11-28 14:16:14 +0000 | |||
66 | @@ -23,10 +23,9 @@ | |||
67 | 23 | from maastest import utils | 23 | from maastest import utils |
68 | 24 | from netaddr import IPNetwork | 24 | from netaddr import IPNetwork |
69 | 25 | import netifaces | 25 | import netifaces |
70 | 26 | from xdg import BaseDirectory | ||
71 | 26 | 27 | ||
75 | 27 | # TODO: '.maas-test' shouln't be harcoded like that, it should | 28 | DISK_CACHE_ROOT = '%(config_dir)s/proxy_cache/' |
73 | 28 | # be a parameter passed to LocalProxyFixture(). | ||
74 | 29 | DISK_CACHE_ROOT = '~/.maas-test/proxy_cache/' | ||
76 | 30 | 29 | ||
77 | 31 | PROXY_CONFIG_ITEMS = { | 30 | PROXY_CONFIG_ITEMS = { |
78 | 32 | 'daemonise': 'true', | 31 | 'daemonise': 'true', |
79 | @@ -59,7 +58,7 @@ | |||
80 | 59 | 58 | ||
81 | 60 | #TODO: port should be determined at runtime from the set of | 59 | #TODO: port should be determined at runtime from the set of |
82 | 61 | # available ports rather than being an arbitrary one as it is here. | 60 | # available ports rather than being an arbitrary one as it is here. |
84 | 62 | def __init__(self, port=42676): | 61 | def __init__(self, config_dir=None, port=42676): |
85 | 63 | """Create a `LocalProxyFixture` object. | 62 | """Create a `LocalProxyFixture` object. |
86 | 64 | 63 | ||
87 | 65 | :param url: The URL of the proxy to use. If None, LocalProxyFixture | 64 | :param url: The URL of the proxy to use. If None, LocalProxyFixture |
88 | @@ -68,13 +67,15 @@ | |||
89 | 68 | :param port: The port on which to start a local caching proxy. | 67 | :param port: The port on which to start a local caching proxy. |
90 | 69 | If URL is specified, this parameter is ignored. | 68 | If URL is specified, this parameter is ignored. |
91 | 70 | :type port: int | 69 | :type port: int |
92 | 70 | :param config_dir: The directory in which to put the proxy's | ||
93 | 71 | config, pid file and cache. | ||
94 | 72 | :type config_dir: string | ||
95 | 71 | """ | 73 | """ |
96 | 72 | super(LocalProxyFixture, self).__init__() | 74 | super(LocalProxyFixture, self).__init__() |
97 | 73 | self.port = port | 75 | self.port = port |
102 | 74 | # TODO: '.maas-test' shouln't be harcoded like that, it should | 76 | if config_dir is None: |
103 | 75 | # be a parameter passed to LocalProxyFixture(). | 77 | config_dir = BaseDirectory.save_config_path('maas-test') |
104 | 76 | self.config_dir = os.path.join( | 78 | self.config_dir = config_dir |
101 | 77 | os.path.expanduser('~'), '.maas-test') | ||
105 | 78 | self.pid_file = os.path.join(self.config_dir, 'proxy.pid') | 79 | self.pid_file = os.path.join(self.config_dir, 'proxy.pid') |
106 | 79 | self._network_address = None | 80 | self._network_address = None |
107 | 80 | 81 | ||
108 | @@ -132,7 +133,7 @@ | |||
109 | 132 | logging.info("Done starting proxy.") | 133 | logging.info("Done starting proxy.") |
110 | 133 | 134 | ||
111 | 134 | def create_cache_root(self): | 135 | def create_cache_root(self): |
113 | 135 | cache_root = os.path.expanduser(DISK_CACHE_ROOT) | 136 | cache_root = DISK_CACHE_ROOT % {'config_dir': self.config_dir} |
114 | 136 | try: | 137 | try: |
115 | 137 | os.makedirs(cache_root) | 138 | os.makedirs(cache_root) |
116 | 138 | except OSError as error: | 139 | except OSError as error: |
117 | 139 | 140 | ||
118 | === modified file 'maastest/tests/test_kvmfixture.py' | |||
119 | --- maastest/tests/test_kvmfixture.py 2013-11-27 17:44:58 +0000 | |||
120 | +++ maastest/tests/test_kvmfixture.py 2013-11-28 14:16:14 +0000 | |||
121 | @@ -13,8 +13,6 @@ | |||
122 | 13 | __all__ = [] | 13 | __all__ = [] |
123 | 14 | 14 | ||
124 | 15 | import itertools | 15 | import itertools |
125 | 16 | import os | ||
126 | 17 | import os.path | ||
127 | 18 | import random | 16 | import random |
128 | 19 | from random import randint | 17 | from random import randint |
129 | 20 | import textwrap | 18 | import textwrap |
130 | @@ -49,9 +47,7 @@ | |||
131 | 49 | # KVMFixture.generate_ssh_key() generates an ssh key in ~/.maas-test, | 47 | # KVMFixture.generate_ssh_key() generates an ssh key in ~/.maas-test, |
132 | 50 | # creating that directory if necessary. Set up a fake home directory | 48 | # creating that directory if necessary. Set up a fake home directory |
133 | 51 | # so it can do what it needs to. | 49 | # so it can do what it needs to. |
137 | 52 | home_dir = self.useFixture(fixtures.TempDir()).path | 50 | self.home_dir = self.useFixture(fixtures.TempDir()).path |
135 | 53 | self.patch( | ||
136 | 54 | os.path, 'expanduser', mock.MagicMock(return_value=home_dir)) | ||
138 | 55 | 51 | ||
139 | 56 | def patch_run_command(self, return_value=0, stdout=b'', stderr=b''): | 52 | def patch_run_command(self, return_value=0, stdout=b'', stderr=b''): |
140 | 57 | """Patch out `kvmfixture` calls to `run_command`. | 53 | """Patch out `kvmfixture` calls to `run_command`. |
141 | @@ -84,7 +80,7 @@ | |||
142 | 84 | series, architecture, name=name, | 80 | series, architecture, name=name, |
143 | 85 | direct_interface=direct_interface, | 81 | direct_interface=direct_interface, |
144 | 86 | direct_network=direct_network, archive=archive, | 82 | direct_network=direct_network, archive=archive, |
146 | 87 | proxy_url=proxy_url) | 83 | proxy_url=proxy_url, ssh_key_dir=self.home_dir) |
147 | 88 | # Hack to get fixture.addDetail to work without actually calling | 84 | # Hack to get fixture.addDetail to work without actually calling |
148 | 89 | # fixture.setUp(). | 85 | # fixture.setUp(). |
149 | 90 | fixture._details = {} | 86 | fixture._details = {} |
150 | @@ -288,7 +284,6 @@ | |||
151 | 288 | self.logger.output.strip().split("\n")) | 284 | self.logger.output.strip().split("\n")) |
152 | 289 | 285 | ||
153 | 290 | def test_generate_ssh_key_sets_up_temporary_key_pair(self): | 286 | def test_generate_ssh_key_sets_up_temporary_key_pair(self): |
154 | 291 | home_dir = os.path.expanduser('~') | ||
155 | 292 | fixture = self.make_KVMFixture() | 287 | fixture = self.make_KVMFixture() |
156 | 293 | fixtures.Fixture.setUp(fixture) | 288 | fixtures.Fixture.setUp(fixture) |
157 | 294 | 289 | ||
158 | @@ -297,7 +292,7 @@ | |||
159 | 297 | self.assertThat(fixture.ssh_private_key_file, FileExists()) | 292 | self.assertThat(fixture.ssh_private_key_file, FileExists()) |
160 | 298 | self.assertThat(fixture.get_ssh_public_key_file(), FileExists()) | 293 | self.assertThat(fixture.get_ssh_public_key_file(), FileExists()) |
161 | 299 | self.assertThat( | 294 | self.assertThat( |
163 | 300 | fixture.ssh_private_key_file, StartsWith(home_dir + '/')) | 295 | fixture.ssh_private_key_file, StartsWith(self.home_dir + '/')) |
164 | 301 | 296 | ||
165 | 302 | self.assertThat( | 297 | self.assertThat( |
166 | 303 | fixture.ssh_private_key_file, | 298 | fixture.ssh_private_key_file, |
167 | 304 | 299 | ||
168 | === modified file 'maastest/tests/test_proxyfixture.py' | |||
169 | --- maastest/tests/test_proxyfixture.py 2013-11-26 22:51:09 +0000 | |||
170 | +++ maastest/tests/test_proxyfixture.py 2013-11-28 14:16:14 +0000 | |||
171 | @@ -39,24 +39,23 @@ | |||
172 | 39 | 'netmask': '255.255.255.0', | 39 | 'netmask': '255.255.255.0', |
173 | 40 | }]} | 40 | }]} |
174 | 41 | self.patch(netifaces, 'ifaddresses', mock_ifaddresses) | 41 | self.patch(netifaces, 'ifaddresses', mock_ifaddresses) |
175 | 42 | self.temp_dir = self.useFixture(TempDir()).path | ||
176 | 42 | 43 | ||
177 | 43 | def get_random_port_number(self): | 44 | def get_random_port_number(self): |
178 | 44 | return random.randint(1, 65535) | 45 | return random.randint(1, 65535) |
179 | 45 | 46 | ||
180 | 46 | def test_init_sets_properties(self): | 47 | def test_init_sets_properties(self): |
181 | 47 | port = self.get_random_port_number() | 48 | port = self.get_random_port_number() |
185 | 48 | proxy_fixture = LocalProxyFixture(port=port) | 49 | proxy_fixture = LocalProxyFixture(port=port, config_dir=self.temp_dir) |
183 | 49 | expected_config_path = os.path.join( | ||
184 | 50 | os.path.expanduser('~'), '.maas-test') | ||
186 | 51 | self.assertEqual(port, proxy_fixture.port) | 50 | self.assertEqual(port, proxy_fixture.port) |
188 | 52 | self.assertEqual(expected_config_path, proxy_fixture.config_dir) | 51 | self.assertEqual(self.temp_dir, proxy_fixture.config_dir) |
189 | 53 | self.assertEqual( | 52 | self.assertEqual( |
191 | 54 | os.path.join(expected_config_path, 'proxy.pid'), | 53 | os.path.join(self.temp_dir, 'proxy.pid'), |
192 | 55 | proxy_fixture.pid_file) | 54 | proxy_fixture.pid_file) |
193 | 56 | 55 | ||
194 | 57 | def test_get_url_returns_proxy_url(self): | 56 | def test_get_url_returns_proxy_url(self): |
195 | 58 | port = self.get_random_port_number() | 57 | port = self.get_random_port_number() |
197 | 59 | proxy_fixture = LocalProxyFixture(port=port) | 58 | proxy_fixture = LocalProxyFixture(config_dir=self.temp_dir, port=port) |
198 | 60 | self.assertEqual( | 59 | self.assertEqual( |
199 | 61 | "http://192.168.0.1:%s" % port, | 60 | "http://192.168.0.1:%s" % port, |
200 | 62 | proxy_fixture.get_url()) | 61 | proxy_fixture.get_url()) |
201 | @@ -65,8 +64,9 @@ | |||
202 | 65 | self.patch(LocalProxyFixture, 'create_cache_root', mock.MagicMock()) | 64 | self.patch(LocalProxyFixture, 'create_cache_root', mock.MagicMock()) |
203 | 66 | self.patch(LocalProxyFixture, 'start_proxy', mock.MagicMock()) | 65 | self.patch(LocalProxyFixture, 'start_proxy', mock.MagicMock()) |
204 | 67 | 66 | ||
207 | 68 | random_port = self.get_random_port_number() | 67 | port = self.get_random_port_number() |
208 | 69 | with LocalProxyFixture(port=random_port) as fixture: | 68 | config_dir = self.temp_dir |
209 | 69 | with LocalProxyFixture(config_dir=config_dir, port=port) as fixture: | ||
210 | 70 | pass | 70 | pass |
211 | 71 | 71 | ||
212 | 72 | self.assertEqual( | 72 | self.assertEqual( |
213 | @@ -79,7 +79,8 @@ | |||
214 | 79 | def test_start_proxy_starts_proxy(self): | 79 | def test_start_proxy_starts_proxy(self): |
215 | 80 | mock_run_command = mock.MagicMock() | 80 | mock_run_command = mock.MagicMock() |
216 | 81 | self.patch(utils, 'run_command', mock_run_command) | 81 | self.patch(utils, 'run_command', mock_run_command) |
218 | 82 | proxy_fixture = LocalProxyFixture(port=self.get_random_port_number()) | 82 | proxy_fixture = LocalProxyFixture( |
219 | 83 | config_dir=self.temp_dir, port=self.get_random_port_number()) | ||
220 | 83 | 84 | ||
221 | 84 | proxy_fixture.start_proxy() | 85 | proxy_fixture.start_proxy() |
222 | 85 | 86 | ||
223 | @@ -90,26 +91,27 @@ | |||
224 | 90 | mock_run_command.mock_calls[-1]) | 91 | mock_run_command.mock_calls[-1]) |
225 | 91 | 92 | ||
226 | 92 | def test_create_cache_root_creates_directory(self): | 93 | def test_create_cache_root_creates_directory(self): |
231 | 93 | temp_dir = self.useFixture(TempDir()).path | 94 | proxy_fixture = LocalProxyFixture( |
232 | 94 | test_cache_path = temp_dir + "path" | 95 | config_dir=self.temp_dir, port=self.get_random_port_number()) |
229 | 95 | self.patch(proxyfixture, 'DISK_CACHE_ROOT', test_cache_path) | ||
230 | 96 | proxy_fixture = LocalProxyFixture(port=self.get_random_port_number()) | ||
233 | 97 | 96 | ||
234 | 98 | proxy_fixture.create_cache_root() | 97 | proxy_fixture.create_cache_root() |
237 | 99 | 98 | expected_cache_path = proxyfixture.DISK_CACHE_ROOT % { | |
238 | 100 | self.assertThat(test_cache_path, DirExists()) | 99 | 'config_dir': self.temp_dir} |
239 | 100 | self.assertThat(expected_cache_path, DirExists()) | ||
240 | 101 | 101 | ||
241 | 102 | def test_create_cache_root_ignores_existing_directory(self): | 102 | def test_create_cache_root_ignores_existing_directory(self): |
242 | 103 | temp_dir = self.useFixture(TempDir()).path | 103 | temp_dir = self.useFixture(TempDir()).path |
243 | 104 | self.patch(proxyfixture, 'DISK_CACHE_ROOT', temp_dir) | 104 | self.patch(proxyfixture, 'DISK_CACHE_ROOT', temp_dir) |
245 | 105 | proxy_fixture = LocalProxyFixture(port=self.get_random_port_number()) | 105 | proxy_fixture = LocalProxyFixture( |
246 | 106 | config_dir=self.temp_dir, port=self.get_random_port_number()) | ||
247 | 106 | 107 | ||
248 | 107 | proxy_fixture.create_cache_root() | 108 | proxy_fixture.create_cache_root() |
249 | 108 | 109 | ||
250 | 109 | self.assertThat(temp_dir, DirExists()) | 110 | self.assertThat(temp_dir, DirExists()) |
251 | 110 | 111 | ||
252 | 111 | def test_get_network_address(self): | 112 | def test_get_network_address(self): |
254 | 112 | proxy_fixture = LocalProxyFixture(port=self.get_random_port_number()) | 113 | proxy_fixture = LocalProxyFixture( |
255 | 114 | config_dir=self.temp_dir, port=self.get_random_port_number()) | ||
256 | 113 | proxy_adddress = proxy_fixture._get_network_address() | 115 | proxy_adddress = proxy_fixture._get_network_address() |
257 | 114 | self.assertEqual('192.168.0.1', proxy_adddress.ip.format()) | 116 | self.assertEqual('192.168.0.1', proxy_adddress.ip.format()) |
258 | 115 | self.assertEqual(24, proxy_adddress.prefixlen) | 117 | self.assertEqual(24, proxy_adddress.prefixlen) |
259 | @@ -120,7 +122,7 @@ | |||
260 | 120 | mock_read_file = mock.MagicMock() | 122 | mock_read_file = mock.MagicMock() |
261 | 121 | mock_read_file.return_value = bytes(self.getUniqueInteger()) | 123 | mock_read_file.return_value = bytes(self.getUniqueInteger()) |
262 | 122 | self.patch(utils, 'read_file', mock_read_file) | 124 | self.patch(utils, 'read_file', mock_read_file) |
264 | 123 | proxy_fixture = LocalProxyFixture() | 125 | proxy_fixture = LocalProxyFixture(config_dir=self.temp_dir) |
265 | 124 | proxy_fixture.kill_running_proxy() | 126 | proxy_fixture.kill_running_proxy() |
266 | 125 | self.assertEqual( | 127 | self.assertEqual( |
267 | 126 | mock.call(['kill', mock_read_file.return_value], check_call=True), | 128 | mock.call(['kill', mock_read_file.return_value], check_call=True), |
268 | @@ -132,7 +134,7 @@ | |||
269 | 132 | mock_read_file = mock.MagicMock() | 134 | mock_read_file = mock.MagicMock() |
270 | 133 | mock_read_file.side_effect = Exception(self.getUniqueString()) | 135 | mock_read_file.side_effect = Exception(self.getUniqueString()) |
271 | 134 | self.patch(utils, 'read_file', mock_read_file) | 136 | self.patch(utils, 'read_file', mock_read_file) |
273 | 135 | proxy_fixture = LocalProxyFixture() | 137 | proxy_fixture = LocalProxyFixture(config_dir=self.temp_dir) |
274 | 136 | proxy_fixture.kill_running_proxy() | 138 | proxy_fixture.kill_running_proxy() |
275 | 137 | mock_log_error.assert_has_calls([ | 139 | mock_log_error.assert_has_calls([ |
276 | 138 | mock.call("Unable to kill proxy:"), | 140 | mock.call("Unable to kill proxy:"), |
277 | @@ -147,7 +149,7 @@ | |||
278 | 147 | mock_read_file = mock.MagicMock() | 149 | mock_read_file = mock.MagicMock() |
279 | 148 | mock_read_file.return_value = bytes(self.getUniqueInteger()) | 150 | mock_read_file.return_value = bytes(self.getUniqueInteger()) |
280 | 149 | self.patch(utils, 'read_file', mock_read_file) | 151 | self.patch(utils, 'read_file', mock_read_file) |
282 | 150 | proxy_fixture = LocalProxyFixture() | 152 | proxy_fixture = LocalProxyFixture(config_dir=self.temp_dir) |
283 | 151 | proxy_fixture.kill_running_proxy() | 153 | proxy_fixture.kill_running_proxy() |
284 | 152 | mock_log_error.assert_has_calls([ | 154 | mock_log_error.assert_has_calls([ |
285 | 153 | mock.call("Unable to kill proxy:"), | 155 | mock.call("Unable to kill proxy:"), |
286 | @@ -162,7 +164,7 @@ | |||
287 | 162 | mock_kill_running_proxy = mock.MagicMock() | 164 | mock_kill_running_proxy = mock.MagicMock() |
288 | 163 | 165 | ||
289 | 164 | port = self.get_random_port_number() | 166 | port = self.get_random_port_number() |
291 | 165 | proxy_fixture = LocalProxyFixture(port) | 167 | proxy_fixture = LocalProxyFixture(config_dir=self.temp_dir, port=port) |
292 | 166 | self.patch( | 168 | self.patch( |
293 | 167 | proxy_fixture, 'kill_running_proxy', mock_kill_running_proxy) | 169 | proxy_fixture, 'kill_running_proxy', mock_kill_running_proxy) |
294 | 168 | 170 | ||
295 | @@ -174,5 +176,5 @@ | |||
296 | 174 | mock_exists = mock.MagicMock() | 176 | mock_exists = mock.MagicMock() |
297 | 175 | mock_exists.return_value = True | 177 | mock_exists.return_value = True |
298 | 176 | self.patch(os.path, 'exists', mock_exists) | 178 | self.patch(os.path, 'exists', mock_exists) |
300 | 177 | proxy_fixture = LocalProxyFixture() | 179 | proxy_fixture = LocalProxyFixture(config_dir=self.temp_dir) |
301 | 178 | self.assertRaises(Exception, proxy_fixture.setUp) | 180 | self.assertRaises(Exception, proxy_fixture.setUp) |
302 | 179 | 181 | ||
303 | === modified file 'packages.txt' | |||
304 | --- packages.txt 2013-11-26 13:10:43 +0000 | |||
305 | +++ packages.txt 2013-11-28 14:16:14 +0000 | |||
306 | @@ -11,6 +11,7 @@ | |||
307 | 11 | python-testresources | 11 | python-testresources |
308 | 12 | python-testtools | 12 | python-testtools |
309 | 13 | python-virtualenv | 13 | python-virtualenv |
310 | 14 | python-xdg | ||
311 | 14 | python3-fixtures | 15 | python3-fixtures |
312 | 15 | python3-lxml | 16 | python3-lxml |
313 | 16 | python3-mock | 17 | python3-mock |
314 | @@ -18,6 +19,7 @@ | |||
315 | 18 | python3-six | 19 | python3-six |
316 | 19 | python3-testresources | 20 | python3-testresources |
317 | 20 | python3-testtools | 21 | python3-testtools |
318 | 22 | python3-xdg | ||
319 | 21 | qemu-kvm | 23 | qemu-kvm |
320 | 22 | uvtool | 24 | uvtool |
321 | 23 | uvtool-libvirt | 25 | uvtool-libvirt |
Looks good, but I'm concerned about punting everything to the user.
[1]
+ # maas-test config add_argument( "~/.maas- test",
+ parser.
+ '--config-dir', type=text_type,
+ default=
+ help="The directory in which to store maas-test config and cache "
+ "files. Defaults to %(default)s")
I think we should use the xdg stuff I mentioned to rvb yesterday, and
not have yet another configuration option. Or is there a compelling
use-case for allowing this? This feels like getting the user to choose
because we can't make a decision.