From e2d23d902a0501d5ef49ed3ca5458e171a0a5d5e Mon Sep 17 00:00:00 2001 From: David-Leon Pohl Date: Tue, 25 Oct 2016 07:18:24 +0200 Subject: [PATCH] Unittests for ddwrt device tracker and bugfix (#3996) * BUG Message data cannot be changed thus use voluptuous to ensure format * Pilight daemon expects JSON serializable data Thus dict is needed and not a mapping proxy. * Add explanation why dict as message data is needed * Use more obvious voluptuous validation scheme * Pylint: Trailing whitespace * Pilight sensor component * Python 3.4 compatibility * D202 * Use pytest-caplog and no unittest.TestCase * Fix setup/teardown of unittests * Activate coverage testing * Bugfix whitelist filter and use bugfixed pilight library * Use newest pilight library that has a bugfix * Add unittest for pilight hub component * PEP257 for docstrings * Bugfix setting device name from host name and small cleanup - Init with connection error handling is more clear - Comments clean-up * PEP257 * New unittest with full coverage * Upload missing testfixtures * D209 * Handle double quotes in reply * Formatting --- .coveragerc | 1 - .../components/device_tracker/ddwrt.py | 34 ++- tests/components/device_tracker/test_ddwrt.py | 244 ++++++++++++++++++ tests/fixtures/Ddwrt_Status_Lan.txt | 18 ++ tests/fixtures/Ddwrt_Status_Wireless.txt | 13 + 5 files changed, 290 insertions(+), 20 deletions(-) create mode 100644 tests/components/device_tracker/test_ddwrt.py create mode 100644 tests/fixtures/Ddwrt_Status_Lan.txt create mode 100644 tests/fixtures/Ddwrt_Status_Wireless.txt diff --git a/.coveragerc b/.coveragerc index 039fa184e1f..cf5c0a36684 100644 --- a/.coveragerc +++ b/.coveragerc @@ -142,7 +142,6 @@ omit = homeassistant/components/device_tracker/bluetooth_tracker.py homeassistant/components/device_tracker/bluetooth_le_tracker.py homeassistant/components/device_tracker/bt_home_hub_5.py - homeassistant/components/device_tracker/ddwrt.py homeassistant/components/device_tracker/fritz.py homeassistant/components/device_tracker/icloud.py homeassistant/components/device_tracker/luci.py diff --git a/homeassistant/components/device_tracker/ddwrt.py b/homeassistant/components/device_tracker/ddwrt.py index 4dc6229566c..9ccb15a1707 100644 --- a/homeassistant/components/device_tracker/ddwrt.py +++ b/homeassistant/components/device_tracker/ddwrt.py @@ -35,9 +35,10 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ # pylint: disable=unused-argument def get_scanner(hass, config): """Validate the configuration and return a DD-WRT scanner.""" - scanner = DdWrtDeviceScanner(config[DOMAIN]) - - return scanner if scanner.success_init else None + try: + return DdWrtDeviceScanner(config[DOMAIN]) + except ConnectionError: + return None # pylint: disable=too-many-instance-attributes @@ -53,13 +54,13 @@ class DdWrtDeviceScanner(object): self.lock = threading.Lock() self.last_results = {} - self.mac2name = {} # Test the router is accessible url = 'http://{}/Status_Wireless.live.asp'.format(self.host) data = self.get_ddwrt_data(url) - self.success_init = data is not None + if not data: + raise ConnectionError('Cannot connect to DD-Wrt router') def scan_devices(self): """Scan for new devices and return a list with found device IDs.""" @@ -83,14 +84,15 @@ class DdWrtDeviceScanner(object): if not dhcp_leases: return None - # Remove leading and trailing single quotes. - cleaned_str = dhcp_leases.strip().strip('"') - elements = cleaned_str.split('","') - num_clients = int(len(elements)/5) + # Remove leading and trailing quotes and spaces + cleaned_str = dhcp_leases.replace( + "\"", "").replace("\'", "").replace(" ", "") + elements = cleaned_str.split(',') + num_clients = int(len(elements) / 5) self.mac2name = {} for idx in range(0, num_clients): - # This is stupid but the data is a single array - # every 5 elements represents one hosts, the MAC + # The data is a single array + # every 5 elements represents one host, the MAC # is the third element and the name is the first. mac_index = (idx * 5) + 2 if mac_index < len(elements): @@ -105,9 +107,6 @@ class DdWrtDeviceScanner(object): Return boolean if scanning successful. """ - if not self.success_init: - return False - with self.lock: _LOGGER.info('Checking ARP') @@ -123,11 +122,8 @@ class DdWrtDeviceScanner(object): if not active_clients: return False - # This is really lame, instead of using JSON the DD-WRT UI - # uses its own data format for some reason and then - # regex's out values so I guess I have to do the same, - # LAME!!! - + # The DD-WRT UI uses its own data format and then + # regex's out values so this is done here too # Remove leading and trailing single quotes. clean_str = active_clients.strip().strip("'") elements = clean_str.split("','") diff --git a/tests/components/device_tracker/test_ddwrt.py b/tests/components/device_tracker/test_ddwrt.py new file mode 100644 index 00000000000..5e0a90d3bbe --- /dev/null +++ b/tests/components/device_tracker/test_ddwrt.py @@ -0,0 +1,244 @@ +"""The tests for the DD-WRT device tracker platform.""" +import os +import unittest +from unittest import mock +import logging +import requests +import requests_mock + +from homeassistant import config +from homeassistant.bootstrap import setup_component +from homeassistant.components import device_tracker +from homeassistant.const import ( + CONF_PLATFORM, CONF_HOST, CONF_PASSWORD, CONF_USERNAME) +from homeassistant.components.device_tracker import DOMAIN +from homeassistant.util import slugify + +from tests.common import ( + get_test_home_assistant, assert_setup_component, load_fixture) + +TEST_HOST = '127.0.0.1' +_LOGGER = logging.getLogger(__name__) + + +class TestDdwrt(unittest.TestCase): + """Tests for the Ddwrt device tracker platform.""" + + hass = None + + def setup_method(self, _): + """Setup things to be run when tests are started.""" + self.hass = get_test_home_assistant() + self.hass.config.components = ['zone'] + + def teardown_method(self, _): + """Stop everything that was started.""" + try: + os.remove(self.hass.config.path(device_tracker.YAML_DEVICES)) + except FileNotFoundError: + pass + + @mock.patch('homeassistant.components.device_tracker.ddwrt._LOGGER.error') + def test_login_failed(self, mock_error): + """Create a Ddwrt scanner with wrong credentials.""" + with requests_mock.Mocker() as mock_request: + mock_request.register_uri( + 'GET', r'http://%s/Status_Wireless.live.asp' % TEST_HOST, + status_code=401) + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) + + self.assertTrue( + 'Failed to authenticate' in + str(mock_error.call_args_list[-1])) + + @mock.patch('homeassistant.components.device_tracker.ddwrt._LOGGER.error') + def test_invalid_response(self, mock_error): + """Test error handling when response has an error status.""" + with requests_mock.Mocker() as mock_request: + mock_request.register_uri( + 'GET', r'http://%s/Status_Wireless.live.asp' % TEST_HOST, + status_code=444) + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) + + self.assertTrue( + 'Invalid response from ddwrt' in + str(mock_error.call_args_list[-1])) + + @mock.patch('homeassistant.components.device_tracker._LOGGER.error') + @mock.patch('homeassistant.components.device_tracker.' + 'ddwrt.DdWrtDeviceScanner.get_ddwrt_data', return_value=None) + def test_no_response(self, data_mock, error_mock): + """Create a Ddwrt scanner with no response in init, should fail.""" + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) + self.assertTrue( + 'Error setting up platform' in + str(error_mock.call_args_list[-1])) + + @mock.patch('homeassistant.components.device_tracker.ddwrt.requests.get', + side_effect=requests.exceptions.Timeout) + @mock.patch('homeassistant.components.device_tracker.ddwrt._LOGGER.error') + def test_get_timeout(self, mock_error, mock_request): + """Test get Ddwrt data with request time out.""" + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) + + self.assertTrue( + 'Connection to the router timed out' in + str(mock_error.call_args_list[-1])) + + def test_scan_devices(self): + """Test creating device info (MAC, name) from response. + + The created known_devices.yaml device info is compared + to the DD-WRT Lan Status request response fixture. + This effectively checks the data parsing functions. + """ + with requests_mock.Mocker() as mock_request: + mock_request.register_uri( + 'GET', r'http://%s/Status_Wireless.live.asp' % TEST_HOST, + text=load_fixture('Ddwrt_Status_Wireless.txt')) + mock_request.register_uri( + 'GET', r'http://%s/Status_Lan.live.asp' % TEST_HOST, + text=load_fixture('Ddwrt_Status_Lan.txt')) + + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) + + path = self.hass.config.path(device_tracker.YAML_DEVICES) + devices = config.load_yaml_config_file(path) + for device in devices: + self.assertIn( + devices[device]['mac'], + load_fixture('Ddwrt_Status_Lan.txt')) + self.assertIn( + slugify(devices[device]['name']), + load_fixture('Ddwrt_Status_Lan.txt')) + + def test_device_name_no_data(self): + """Test creating device info (MAC only) when no response.""" + with requests_mock.Mocker() as mock_request: + mock_request.register_uri( + 'GET', r'http://%s/Status_Wireless.live.asp' % TEST_HOST, + text=load_fixture('Ddwrt_Status_Wireless.txt')) + mock_request.register_uri( + 'GET', r'http://%s/Status_Lan.live.asp' % TEST_HOST, text=None) + + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) + + path = self.hass.config.path(device_tracker.YAML_DEVICES) + devices = config.load_yaml_config_file(path) + for device in devices: + _LOGGER.error(devices[device]) + self.assertIn( + devices[device]['mac'], + load_fixture('Ddwrt_Status_Lan.txt')) + + def test_device_name_no_dhcp(self): + """Test creating device info (MAC) when missing dhcp response.""" + with requests_mock.Mocker() as mock_request: + mock_request.register_uri( + 'GET', r'http://%s/Status_Wireless.live.asp' % TEST_HOST, + text=load_fixture('Ddwrt_Status_Wireless.txt')) + mock_request.register_uri( + 'GET', r'http://%s/Status_Lan.live.asp' % TEST_HOST, + text=load_fixture('Ddwrt_Status_Lan.txt'). + replace('dhcp_leases', 'missing')) + + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) + + path = self.hass.config.path(device_tracker.YAML_DEVICES) + devices = config.load_yaml_config_file(path) + for device in devices: + _LOGGER.error(devices[device]) + self.assertIn( + devices[device]['mac'], + load_fixture('Ddwrt_Status_Lan.txt')) + + def test_update_no_data(self): + """Test error handling of no response when active devices checked.""" + with requests_mock.Mocker() as mock_request: + mock_request.register_uri( + 'GET', r'http://%s/Status_Wireless.live.asp' % TEST_HOST, + # First request has to work to setup connection + [{'text': load_fixture('Ddwrt_Status_Wireless.txt')}, + # Second request to get active devices fails + {'text': None}]) + mock_request.register_uri( + 'GET', r'http://%s/Status_Lan.live.asp' % TEST_HOST, + text=load_fixture('Ddwrt_Status_Lan.txt')) + + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) + + def test_update_wrong_data(self): + """Test error handling of bad response when active devices checked.""" + with requests_mock.Mocker() as mock_request: + mock_request.register_uri( + 'GET', r'http://%s/Status_Wireless.live.asp' % TEST_HOST, + text=load_fixture('Ddwrt_Status_Wireless.txt'). + replace('active_wireless', 'missing')) + mock_request.register_uri( + 'GET', r'http://%s/Status_Lan.live.asp' % TEST_HOST, + text=load_fixture('Ddwrt_Status_Lan.txt')) + + with assert_setup_component(1): + assert setup_component( + self.hass, DOMAIN, {DOMAIN: { + CONF_PLATFORM: 'ddwrt', + CONF_HOST: TEST_HOST, + CONF_USERNAME: 'fake_user', + CONF_PASSWORD: '0' + }}) diff --git a/tests/fixtures/Ddwrt_Status_Lan.txt b/tests/fixtures/Ddwrt_Status_Lan.txt new file mode 100644 index 00000000000..b61d92c365e --- /dev/null +++ b/tests/fixtures/Ddwrt_Status_Lan.txt @@ -0,0 +1,18 @@ +{lan_mac::AA:BB:CC:DD:EE:F0} +{lan_ip::192.168.1.1} +{lan_ip_prefix::192.168.1.} +{lan_netmask::255.255.255.0} +{lan_gateway::0.0.0.0} +{lan_dns::8.8.8.8} +{lan_proto::dhcp} +{dhcp_daemon::DNSMasq} +{dhcp_start::100} +{dhcp_num::50} +{dhcp_lease_time::1440} +{dhcp_leases:: 'device_1','192.168.1.113','AA:BB:CC:DD:EE:00','1 day 00:00:00','113','device_2','192.168.1.201','AA:BB:CC:DD:EE:01','Static','201'} +{pptp_leases::} +{pppoe_leases::} +{arp_table:: 'device_1','192.168.1.113','AA:BB:CC:DD:EE:00','13','device_2','192.168.1.201','AA:BB:CC:DD:EE:01','1'} +{uptime:: 12:28:48 up 132 days, 18:02, load average: 0.15, 0.19, 0.21} +{ipinfo:: IP: 192.168.0.108} + diff --git a/tests/fixtures/Ddwrt_Status_Wireless.txt b/tests/fixtures/Ddwrt_Status_Wireless.txt new file mode 100644 index 00000000000..5343fea9904 --- /dev/null +++ b/tests/fixtures/Ddwrt_Status_Wireless.txt @@ -0,0 +1,13 @@ +{wl_mac::AA:BB:CC:DD:EE:FF} +{wl_ssid::WIFI_SSD} +{wl_channel::10} +{wl_radio::Radio is On} +{wl_xmit::Auto} +{wl_rate::72 Mbps} +{wl_ack::} +{active_wireless::'AA:BB:CC:DD:EE:00','eth1','3:13:14','72M','24M','HT20','-9','-92','83','1048','AA:BB:CC:DD:EE:01','eth1','10:48:22','72M','72M','HT20','-40','-92','52','664'} +{active_wds::} +{packet_info::SWRXgoodPacket=173673555;SWRXerrorPacket=27;SWTXgoodPacket=311344396;SWTXerrorPacket=3107;} +{uptime:: 12:29:23 up 132 days, 18:03, load average: 0.16, 0.19, 0.20} +{ipinfo:: IP: 192.168.0.108} +