| Message ID | 1488799839-3963-5-git-send-email-andrei.otcheretianski@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Mon, Mar 06, 2017 at 01:30:39PM +0200, Andrei Otcheretianski wrote: > When saving a configuration, many fields are only saved only if > they are set to a value different than the default value. > So if the test sets a field to its default value and than saves > the configuration, this value will not be saved to the config file > and the test will fail. This requires this test to be updated if > default values are changed. This is done on purpose. In most cases, there should be clear justification for changing a default value and as such, it is convenient to have a test case clearly highlight such changes so that no accidental changes get accepted. > Fix that by only verifying that fields that are set to a non-default > value are written to the saved configuration. What is your use case for this? The current behavior seems more useful to me.
> On Mon, Mar 06, 2017 at 01:30:39PM +0200, Andrei Otcheretianski wrote: > > When saving a configuration, many fields are only saved only if they > > are set to a value different than the default value. > > So if the test sets a field to its default value and than saves the > > configuration, this value will not be saved to the config file and the > > test will fail. This requires this test to be updated if default > > values are changed. > > This is done on purpose. In most cases, there should be clear justification for > changing a default value and as such, it is convenient to have a test case clearly > highlight such changes so that no accidental changes get accepted. I agree that changing default values shouldn't be accidental, but this test doesn't really verify it. The test will fail only if the value used by the test is by chance equals to the default one. So maybe it's better to have a separate test that verifies the default values? We can prepare such test if you want. > > > Fix that by only verifying that fields that are set to a non-default > > value are written to the saved configuration. > > What is your use case for this? The current behavior seems more useful to me. In our internal tree we have different defaults and one of them is failing this test. So I wondered that it may be useful to have this specific test more robust to such changes. Andrei > > -- > Jouni Malinen PGP id EFC895FA
diff --git a/tests/hwsim/test_wpas_config.py b/tests/hwsim/test_wpas_config.py index 12cb6e8..52ecbf7 100644 --- a/tests/hwsim/test_wpas_config.py +++ b/tests/hwsim/test_wpas_config.py @@ -121,7 +121,7 @@ config_checks = [ ("ap_scan", "0"), ("fst_priority", "5"), ("fst_llt", "7"), ("openssl_ciphers", "DEFAULT") ] -def check_config(config): +def check_config(config, exclude=[]): with open(config, "r") as f: data = f.read() if "ctrl_interface=DIR=/var/run/wpa_supplicant GROUP=" not in data: @@ -133,10 +133,17 @@ def check_config(config): if "network={" not in data: raise Exception("Missing network") for field, value in config_checks: - if "\n" + field + "=" + value + "\n" not in data: + if field not in exclude and "\n" + field + "=" + value + "\n" not in data: raise Exception("Missing value: " + field) return data +def check_defaults(dev): + exclude = [] + for field, value in config_checks: + if dev.request("GET " + field) == value: + exclude.append(field) + return exclude + def test_wpas_config_file(dev, apdev, params): """wpa_supplicant config file parsing/writing""" config = os.path.join(params['logdir'], 'wpas_config_file.conf') @@ -172,6 +179,8 @@ def test_wpas_config_file(dev, apdev, params): wpas.interface_add("wlan5", config=config) + exclude = check_defaults(wpas) + id = wpas.add_network() wpas.set_network_quoted(id, "ssid", "foo") wpas.set_network_quoted(id, "psk", "12345678") @@ -213,7 +222,7 @@ def test_wpas_config_file(dev, apdev, params): raise Exception("Failed to save configuration file") wpas.interface_remove("wlan5") - data1 = check_config(config) + data1 = check_config(config, exclude) wpas.interface_add("wlan5", config=config) if len(wpas.list_networks()) != 1: @@ -223,7 +232,7 @@ def test_wpas_config_file(dev, apdev, params): if "OK" not in wpas.request("SAVE_CONFIG"): raise Exception("Failed to save configuration file") - data2 = check_config(config) + data2 = check_config(config, exclude) if data1 != data2: logger.debug(data1)