From b0ab34e8d2e00287894ec50192cfeaa8eab57639 Mon Sep 17 00:00:00 2001
From: Travis Ens <ens.travis@gmail.com>
Date: Thu, 24 Dec 2020 00:40:46 -0500
Subject: Fixed comparison with None, properly unset environment variables by
 deleting them, updated the test to check this behaviour, refactored test code
 by removing redundant variables and improving readability.

---
 qutebrowser/config/configdata.yml |  9 ++++---
 qutebrowser/config/qtargs.py      |  9 +++----
 tests/unit/config/test_qtargs.py  | 52 ++++++++++++++++++++++-----------------
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml
index 5788dc4ab..362cd9b72 100644
--- a/qutebrowser/config/configdata.yml
+++ b/qutebrowser/config/configdata.yml
@@ -160,13 +160,16 @@ qt.environ:
   type:
     name: Dict
     keytype: String
-    valtype: String
+    valtype: 
+      name: String
+      none_ok: true
     none_ok: true
   default: {}
   restart: true
   desc: >-
-    Additional environment variables to set. Set to `None` to unset an environment
-    variable.
+    Additional environment variables to set. 
+
+    Setting an environment variable to null/None will unset it.
 
 force_software_rendering:
   renamed: qt.force_software_rendering
diff --git a/qutebrowser/config/qtargs.py b/qutebrowser/config/qtargs.py
index dde8e1131..d6375f331 100644
--- a/qutebrowser/config/qtargs.py
+++ b/qutebrowser/config/qtargs.py
@@ -260,9 +260,8 @@ def init_envvars() -> None:
                    else 'QT_AUTO_SCREEN_SCALE_FACTOR')
         os.environ[env_var] = '1'
 
-    for var in config.val.qt.environ:
-        val = config.val.qt.environ[var]
-        if val == 'None':
-            os.environ[var] = ''
-        else:
+    for var, val in config.val.qt.environ.items():
+        if val is None and var in os.environ:
+            del os.environ[var]
+        elif val is not None:
             os.environ[var] = val
diff --git a/tests/unit/config/test_qtargs.py b/tests/unit/config/test_qtargs.py
index 4abf69139..bf60cb8fd 100644
--- a/tests/unit/config/test_qtargs.py
+++ b/tests/unit/config/test_qtargs.py
@@ -428,38 +428,46 @@ class TestEnvVars:
 
         assert os.environ[envvar] == expected
 
-    @pytest.mark.parametrize('config_opt, config_val, init_val, envvar, expected', [
-        ('qt.environ', {'QT_SCALE_FACTOR': '2'},
-         [None], ['QT_SCALE_FACTOR'], ['2']),
-        #Test changing an environment variable
-        ('qt.environ', {'QT_SCALE_FACTOR': '4'},
-         ['2'], ['QT_SCALE_FACTOR'], ['4']),
-        ('qt.environ', {'QT_SCALE_FACTOR': '4'},
-         ['unset'], ['QT_SCALE_FACTOR'], ['4']),
-        #Test unsetting an environment variable
-        ('qt.environ', {'QT_SCALE_FACTOR': 'None'},
-         ['3'], ['QT_SCALE_FACTOR'], ['']),
-        #Test setting multiple environment variables
-        ('qt.environ', {'QT_SCALE_FACTOR': '3', 'QT_PLUGIN_PATH': '/tmp/',
-                        'QT_NEWVAR': 'newval'},
-         [None, None, None], ['QT_SCALE_FACTOR', 'QT_PLUGIN_PATH', 'QT_NEWVAR'],
-         ['3', '/tmp/', 'newval']),
+    @pytest.mark.parametrize('init_val, config_val', [
+        (   # Test setting a variable
+            {'QT_SCALE_FACTOR': None},
+            {'QT_SCALE_FACTOR': '2'},
+        ),
+        (   # Test changing a set variable
+            {'QT_SCALE_FACTOR': '2'},
+            {'QT_SCALE_FACTOR': '4'},
+        ),
+        (   # Test setting an unset variable
+            {'QT_SCALE_FACTOR': 'unset'},
+            {'QT_SCALE_FACTOR': '3'},
+        ),
+        (   # Test unsetting a variable
+            {'QT_SCALE_FACTOR': '3'},
+            {'QT_SCALE_FACTOR': None},
+        ),
+        (   # Test setting multiple variables
+            {'QT_SCALE_FACTOR': None, 'QT_PLUGIN_PATH': None, 'QT_NEWVAR': None},
+            {'QT_SCALE_FACTOR': '3', 'QT_PLUGIN_PATH': '/tmp/', 'QT_NEWVAR': 'newval'},
+        )
     ])
     def test_environ_settings(self, monkeypatch, config_stub,
-                              config_opt, config_val, init_val, envvar, expected):
-        """Test extra environment settings."""
-        for (var, val) in zip(envvar, init_val):
+                              init_val, config_val):
+        """Test setting environment variables using qt.environ."""
+        for var, val in init_val.items():
             if val is not None:
                 if val == 'unset':
                     monkeypatch.delenv(var)
                 else:
                     monkeypatch.setenv(var, val)
 
-        config_stub.set_obj(config_opt, config_val)
+        config_stub.val.qt.environ = config_val
         qtargs.init_envvars()
 
-        for (var, result) in zip(envvar, expected):
-            assert os.environ[var] == result
+        for var, result in config_val.items():
+            if result is None:
+                assert var not in os.environ
+            else:
+                assert os.environ[var] == result
 
     @pytest.mark.parametrize('new_qt', [True, False])
     def test_highdpi(self, monkeypatch, config_stub, new_qt):
-- 
cgit v1.2.3-54-g00ecf