ubuntu-server-iso-testing-dev team mailing list archive

[Jean-Baptiste Lallement <jean-baptiste@xxxxxxxxxx>]

Review: Needs Fixing

* The right place for the dns configuration setup would be in a setUp method of the class DnsServerTest
* The following code can be more concise with readlines() instead of read() + splitlines()
content = resolver.read()
      for line in content.splitlines():
* You fixed indentation but introduced broken indentation lines 92 to 95, it should be 8, 12 and 16 spaces instead of 6,8 and 10 respectively
* ns_address = line.split(' ')[1] will fail if there is more than 1 space between the name of the option and the IP, split() without explicit separator, consecutive spaces will be considered as 1 separator.
* If there is more than 1 nameserver in resolv.conf the last entry will be used. Is it ok since the resolver uses them in order ?
* If there is no nameserver entry, /etc/bind/named.conf.options will be rewritten with a 'None' in it which will fail. The setup should be aborted in this case because the test cannot pass without it.
* Catch the returncode of subprocess.call(['sudo', 'service', 'bind9', 'restart']) or use check_call and catch the exception, and cancel if it failed. If the DNS doesn't start properly the test will fail.
* The test changes the configuration of the DNS server of the host it is running on and it should be documented.

-- 
https://code.launchpad.net/~hggdh2/ubuntu-server-iso-testing/fix-dns-test/+merge/128983
Your team Ubuntu Server Iso Testing Developers is subscribed to branch lp:ubuntu-server-iso-testing.