Merge lp:~terceiro/lava-dispatcher/fix-1032467 into lp:lava-dispatcher

[Antonio Terceiro]

when tester_hostname has an empty value, use the default value of `linaro`.

[Andy Doan (doanac)]

in the past we've avoided creating actual device configs. I see why you are doing it here, but I'm not sure the pros/cons. I'll defer to mwhudson.

[Michael Hudson-Doyle (mwhudson)]

Andy, the device config is in lava_dispatcher/tests/test-config/devices/ -- so an installation of the dispatcher still won't have a device configured. This seems fine.

I think this test is OK -- it's not perfect, because the test is not self-contained: you need to go and look at the qemu01.conf file to assess whether the test is correct. A better test would create a config file with contents specified by the test code itself, verify the code and then delete the config file again (possibly all using a helper to make it nice). But this is being picky, I'd be fine to land the code as is.

But, can you remove the hostname from the config file? I think I'd like to start squashing the meme that you need to repeat the hostname like that.

review: Approve

[Antonio Terceiro (terceiro)]

Michael Hudson-Doyle escreveu:
> Review: Approve
>
> Andy, the device config is in
> lava_dispatcher/tests/test-config/devices/ -- so an installation of
> the dispatcher still won't have a device configured. This seems fine.
>
> I think this test is OK -- it's not perfect, because the test is not
> self-contained: you need to go and look at the qemu01.conf file to
> assess whether the test is correct. A better test would create a
> config file with contents specified by the test code itself, verify
> the code and then delete the config file again (possibly all using a
> helper to make it nice). But this is being picky, I'd be fine to land
> the code as is.
>
> But, can you remove the hostname from the config file? I think I'd
> like to start squashing the meme that you need to repeat the hostname
> like that.

I like self-contained tests, so I will work a solution to create the
config file on the fly - let's see.

--
Antonio Terceiro
Software Engineer - Linaro
http://www.linaro.org

[lp:~terceiro/lava-dispatcher/fix-1032467]

380. By Antonio Terceiro on 2012-09-12

make test self-contained

[Michael Hudson-Doyle (mwhudson)]

Antonio Terceiro <email address hidden> writes:

> I like self-contained tests, so I will work a solution to create the
> config file on the fly - let's see.

I like what you did there :-)

Cheers,
mwh

=== modified file 'lava_dispatcher/config.py'
--- lava_dispatcher/config.py	2012-08-08 22:30:52 +0000
+++ lava_dispatcher/config.py	2012-09-11 21:04:19 +0000
@@ -80,7 +80,10 @@
         self.config_dir = config_dir
     def get(self, key, default=_sentinel):
         try:
-            return self.cp.get("DEFAULT", key)
+            val = self.cp.get("DEFAULT", key)
+            if default is not _sentinel and val == '':
+                val = default
+            return val
         except NoOptionError:
             if default is not _sentinel:
                 return default

=== added file 'lava_dispatcher/tests/test-config/devices/qemu01.conf'
--- lava_dispatcher/tests/test-config/devices/qemu01.conf	1970-01-01 00:00:00 +0000
+++ lava_dispatcher/tests/test-config/devices/qemu01.conf	2012-09-11 21:04:19 +0000
@@ -0,0 +1,2 @@
+device_type = qemu
+hostname = qemu01

=== modified file 'lava_dispatcher/tests/test_config.py'
--- lava_dispatcher/tests/test_config.py	2011-11-28 20:27:03 +0000
+++ lava_dispatcher/tests/test_config.py	2012-09-11 21:04:19 +0000
@@ -22,6 +22,7 @@
 
 from lava_dispatcher.config import get_config, get_device_config
 from lava_dispatcher.utils import string_to_list
+from lava_dispatcher.client.base import LavaClient
 
 test_config_dir = os.path.join(os.path.dirname(__file__), 'test-config')
 print test_config_dir
@@ -48,3 +49,7 @@
         lava_server_ip = server_config.get("LAVA_SERVER_IP")
         self.assertEqual(expected, lava_server_ip)
 
+    def test_default_value_for_tester_hostname(self):
+        config = get_device_config("qemu01", test_config_dir)
+        client = LavaClient(None, config)
+        self.assertEqual('linaro', client.tester_hostname)