Split new function out into a separte utility script and add a test for it. Can be used in future to organise re/commonly-used code

Signed-off-by: Adam Warner <me@adamwarner.co.uk>

advanced/Scripts/utils.sh

@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+# Pi-hole: A black hole for Internet advertisements
+# (c) 2017 Pi-hole, LLC (https://pi-hole.net)
+# Network-wide ad blocking via your own hardware.
+#
+# Script to hold utility functions for use in other scripts
+#
+# This file is copyright under the latest version of the EUPL.
+# Please see LICENSE file for your rights under this license.
+
+# Basic Housekeeping rules
+#  - Functions must be self contained
+#  - Functions must be added in alphabetical order
+#  - Functions must be documented
+#  - New functions must have a test added for them in test/test_any_utils.py
+
+#######################
+# Takes three arguments key, value, and file.
+# Checks the target file for the existence of the key
+#   - If it exists, it changes the value
+#   - If it does not exist, it adds the value
+#
+# Example usage:
+# addOrEditKeyValuePair "BLOCKING_ENABLED" "true" "/etc/pihole/setupVars.conf"
+#######################
+addOrEditKeyValPair() {
+  local key="${1}"
+  local value="${2}"
+  local file="${3}"
+  if grep -q "^${key}=" "${file}"; then
+    sed -i "/^${key}=/c\\${key}=${value}" "${file}"
+  else
+    echo "${key}=${value}" >> "${file}"
+  fi
+}

pihole

@@ -21,6 +21,9 @@ readonly FTL_PID_FILE="/run/pihole-FTL.pid"
 readonly colfile="${PI_HOLE_SCRIPT_DIR}/COL_TABLE"
 source "${colfile}"
 
+readonly utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+source "${utilsfile}"
+
 webpageFunc() {
   source "${PI_HOLE_SCRIPT_DIR}/webpage.sh"
   main "$@"
@@ -170,16 +173,6 @@ restartDNS() {
   fi
 }
 
-addOrEditKeyValPair() {
-  local key="${1}"
-  local value="${2}"
-  if grep -q "^${key}=" "${setupVars}"; then
-    sed -i "/^${key}=/c\\${key}=${value}" "${setupVars}"
-  else
-    echo "${key}=${value}" >> "${setupVars}" 
-  fi
-}
-
 piholeEnable() {
   if [[ "${2}" == "-h" ]] || [[ "${2}" == "--help" ]]; then
     echo "Usage: pihole disable [time]
@@ -233,7 +226,7 @@ Time:
       fi
 
       local str="Pi-hole Disabled"
-      addOrEditKeyValPair "BLOCKING_ENABLED" "false"
+      addOrEditKeyValPair "BLOCKING_ENABLED" "false" "${setupVars}"
     fi
   else
     # Enable Pi-hole
@@ -245,7 +238,7 @@ Time:
     echo -e "  ${INFO} Enabling blocking"
     local str="Pi-hole Enabled"
 
-    addOrEditKeyValPair "BLOCKING_ENABLED" "true"
+    addOrEditKeyValPair "BLOCKING_ENABLED" "true" "${setupVars}"
   fi
 
   restartDNS reload-lists
@@ -268,7 +261,7 @@ Options:
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
     sed -i 's/^log-queries/#log-queries/' /etc/dnsmasq.d/01-pihole.conf
-    addOrEditKeyValPair "QUERY_LOGGING" "false"
+    addOrEditKeyValPair "QUERY_LOGGING" "false" "${setupVars}"
     if [[ "${2}" != "noflush" ]]; then
       # Flush logs
       "${PI_HOLE_BIN_DIR}"/pihole -f
@@ -278,7 +271,7 @@ Options:
   elif [[ "${1}" == "on" ]]; then
     # Enable logging
     sed -i 's/^#log-queries/log-queries/' /etc/dnsmasq.d/01-pihole.conf
-    addOrEditKeyValPair "QUERY_LOGGING" "true"
+    addOrEditKeyValPair "QUERY_LOGGING" "true" "${setupVars}"
     echo -e "  ${INFO} Enabling logging..."
     local str="Logging has been enabled!"
   else

test/test_any_utils.py

@@ -0,0 +1,16 @@
+def test_key_val_replacement_works(host):
+    ''' Confirms addOrEditKeyValPair provides the expected output '''
+    host.run('''
+    setupvars=./testoutput
+    source /opt/pihole/utils.sh
+    addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput"
+    addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
+    addOrEditKeyValPair "KEY_ONE" "value3" "./testoutput"
+    addOrEditKeyValPair "KEY_FOUR" "value4" "./testoutput"
+    cat ./testoutput
+    ''')
+    output = host.run('''
+    cat ./testoutput
+    ''')
+    expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\n'
+    assert expected_stdout == output.stdout

test/tox.centos_7.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _centos_7.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py ./test_centos_fedora_common_support.py ./test_centos_common_support.py ./test_centos_7_support.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py ./test_centos_fedora_common_support.py ./test_centos_common_support.py ./test_centos_7_support.py

test/tox.centos_8.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _centos_8.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py ./test_centos_fedora_common_support.py ./test_centos_common_support.py ./test_centos_8_support.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py ./test_centos_fedora_common_support.py ./test_centos_common_support.py ./test_centos_8_support.py

test/tox.debian_10.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _debian_10.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py

test/tox.debian_11.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _debian_11.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py

test/tox.debian_9.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _debian_9.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py

test/tox.fedora_33.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _fedora_33.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py ./test_centos_fedora_common_support.py ./test_fedora_support.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py ./test_centos_fedora_common_support.py ./test_fedora_support.py

test/tox.fedora_34.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _fedora_34.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py ./test_centos_fedora_common_support.py ./test_fedora_support.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py ./test_centos_fedora_common_support.py ./test_fedora_support.py

test/tox.ubuntu_16.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _ubuntu_16.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py

test/tox.ubuntu_18.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _ubuntu_18.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py

test/tox.ubuntu_20.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _ubuntu_20.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py

test/tox.ubuntu_21.ini

@@ -5,4 +5,4 @@ envlist = py38
 whitelist_externals = docker
 deps = -rrequirements.txt
 commands = docker build -f _ubuntu_21.Dockerfile -t pytest_pihole:test_container ../
-           pytest {posargs:-vv -n auto} ./test_automated_install.py
+           pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py