From 633095aee1704cc6cced1dc78ea897dac32417f2 Mon Sep 17 00:00:00 2001 From: diginc Date: Wed, 2 Nov 2016 22:13:05 -0500 Subject: [PATCH 1/9] switch to consistent style --- automated install/basic-install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh index 14d271ef..534b194d 100755 --- a/automated install/basic-install.sh +++ b/automated install/basic-install.sh @@ -1149,6 +1149,6 @@ main() { echo "::: The install log is located at: /etc/pihole/install.log" } -if [[ -z "$PHTEST" ]] ; then +if [[ "${PHTEST}" != "TRUE" ]] ; then main "$@" fi From 0d7e06a141ac0bde23f478f878be21e8b7d6cebd Mon Sep 17 00:00:00 2001 From: diginc Date: Wed, 2 Nov 2016 22:13:39 -0500 Subject: [PATCH 2/9] remove old pytest output file --- test/shellcheck_failing_output.txt | 76 ------------------------------ 1 file changed, 76 deletions(-) delete mode 100644 test/shellcheck_failing_output.txt diff --git a/test/shellcheck_failing_output.txt b/test/shellcheck_failing_output.txt deleted file mode 100644 index c741a8e9..00000000 --- a/test/shellcheck_failing_output.txt +++ /dev/null @@ -1,76 +0,0 @@ -============================= test session starts ============================== -platform linux2 -- Python 2.7.6, pytest-2.9.2, py-1.4.31, pluggy-0.3.1 -- /usr/bin/python -cachedir: .cache -rootdir: /home/a/opensource/pi-hole, inifile: -plugins: cov-2.3.0, bdd-2.17.0, xdist-1.14, testinfra-1.4.0 -collecting ... collected 7 items - -test/test_000_build_containers.py::test_build_pihole_image[test/debian.Dockerfile-pytest_pihole:debian] PASSED -test/test_000_build_containers.py::test_build_pihole_image[test/centos.Dockerfile-pytest_pihole:centos] PASSED -test/test_automated_install.py::test_setupVars_are_sourced_to_global_scope[debian] PASSED -test/test_automated_install.py::test_setupVars_are_sourced_to_global_scope[centos] PASSED -test/test_automated_install.py::test_setupVars_saved_to_file[debian] PASSED -test/test_automated_install.py::test_setupVars_saved_to_file[centos] PASSED -test/test_shellcheck.py::test_scripts_pass_shellcheck FAILED - -=================================== FAILURES =================================== -_________________________ test_scripts_pass_shellcheck _________________________ - - def test_scripts_pass_shellcheck(): - ''' Make sure shellcheck does not find anything wrong with our shell scripts ''' - shellcheck = "find . -name 'update.sh' | while read file; do shellcheck \"$file\"; done;" - results = run_local(shellcheck) - print results.stdout -> assert '' == results.stdout -E assert '' == '\nIn ./advanced/Scripts/upda...vent glob interpretation.\n\n' -E + -E + In ./advanced/Scripts/update.sh line 24: -E + while [ "$(ps a | awk '{print $1}' | grep "${pid}")" ]; do -E + ^-- SC2143: Instead of [ -n $(foo | grep bar) ], use foo | grep -q bar . -E + -E + -E + In ./advanced/Scripts/update.sh line 57: -E + git clone -q --depth 1 "${2}" "${1}" > /dev/null & spinner $! -E + ^-- SC2086: Double quote to prevent globbing and word splitting. -E Detailed information truncated (27 more lines), use "-vv" to show - -test/test_shellcheck.py:13: AssertionError ------------------------------ Captured stdout call ----------------------------- - -In ./advanced/Scripts/update.sh line 24: - while [ "$(ps a | awk '{print $1}' | grep "${pid}")" ]; do - ^-- SC2143: Instead of [ -n $(foo | grep bar) ], use foo | grep -q bar . - - -In ./advanced/Scripts/update.sh line 57: - git clone -q --depth 1 "${2}" "${1}" > /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./advanced/Scripts/update.sh line 65: - git stash -q > /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./advanced/Scripts/update.sh line 66: - git pull -q > /dev/null & spinner $! - ^-- SC2086: Double quote to prevent globbing and word splitting. - - -In ./advanced/Scripts/update.sh line 107: -if [[ ${piholeVersion} == ${piholeVersionLatest} && ${webVersion} == ${webVersionLatest} ]]; then - ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation. - ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation. - - -In ./advanced/Scripts/update.sh line 112: -elif [[ ${piholeVersion} == ${piholeVersionLatest} && ${webVersion} != ${webVersionLatest} ]]; then - ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation. - - -In ./advanced/Scripts/update.sh line 120: -elif [[ ${piholeVersion} != ${piholeVersionLatest} && ${webVersion} == ${webVersionLatest} ]]; then - ^-- SC2053: Quote the rhs of = in [[ ]] to prevent glob interpretation. - - -===================== 1 failed, 6 passed in 24.01 seconds ====================== From 05e114173db93f9395866995c004138d1a9e56db Mon Sep 17 00:00:00 2001 From: diginc Date: Wed, 2 Nov 2016 23:25:13 -0500 Subject: [PATCH 3/9] update comments, add configureFirewall test * Comments to clarify some of the existing tests * mock_command to allow recording of calls and mocking return calls in bash * new configureFirewall test (only the first one of it's many paths) --- test/test_automated_install.py | 41 ++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/test/test_automated_install.py b/test/test_automated_install.py index 458536eb..b720b94d 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -10,8 +10,9 @@ SETUPVARS = { } def test_setupVars_are_sourced_to_global_scope(Pihole): - ''' currently update_dialogs sources setupVars with a dot, - then various other functions use the variables ''' + ''' currently update_dialogs sources setupVars with a dot, + then various other functions use the variables. + This confirms the sourced variables are in scope between functions ''' setup_var_file = 'cat < /etc/pihole/setupVars.conf\n' for k,v in SETUPVARS.iteritems(): setup_var_file += "{}={}\n".format(k, v) @@ -64,6 +65,42 @@ def test_setupVars_saved_to_file(Pihole): for k,v in SETUPVARS.iteritems(): assert "{}={}".format(k, v) in output +def test_configureFirewall_firewalld_no_errors(Pihole): + ''' confirms firewalld rules are applied when appopriate ''' + mock_command('firewall-cmd', '0', Pihole) + configureFirewall = Pihole.run(''' + bash -c " + PHTEST=TRUE + source /opt/pihole/basic-install.sh + configureFirewall + " ''') + expected_stdout = '::: Configuring firewalld for httpd and dnsmasq.' + assert expected_stdout in configureFirewall.stdout + firewall_calls = Pihole.run('cat /var/log/firewall-cmd').stdout + assert 'firewall-cmd --state' in firewall_calls + assert 'firewall-cmd --permanent --add-port=80/tcp' in firewall_calls + assert 'firewall-cmd --permanent --add-port=53/tcp' in firewall_calls + assert 'firewall-cmd --permanent --add-port=53/udp' in firewall_calls + assert 'firewall-cmd --reload' in firewall_calls + + +# Helper functions +def mock_command(script, result, container): + ''' Allows for setup of commands we don't really want to have to run for real in unit tests ''' + ''' TODO: support array of results that enable the results to change over multiple executions of a command ''' + full_script_path = '/usr/local/bin/{}'.format(script) + mock_script = dedent('''\ + #!/bin/bash -e + echo "\$0 \$@" >> /var/log/{script} + exit {retcode} + '''.format(script=script, retcode=result)) + container.run(''' + cat < {script}\n{content}\nEOF + chmod +x {script} + '''.format(script=full_script_path, content=mock_script)) + print container.run('cat {}'.format(full_script_path)).stdout + + def run_script(Pihole, script, file="/test.sh"): _write_test_script(Pihole, script, file=file) result = Pihole.run(file) From 699e29934521587066f001800f09e76badc0a1d4 Mon Sep 17 00:00:00 2001 From: diginc Date: Wed, 2 Nov 2016 23:40:50 -0500 Subject: [PATCH 4/9] add a comment about bash vs dash. future refact needed --- test/test_automated_install.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_automated_install.py b/test/test_automated_install.py index b720b94d..6a3daf60 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -110,5 +110,8 @@ def run_script(Pihole, script, file="/test.sh"): def _write_test_script(Pihole, script, file): ''' Running the test script blocks directly can behave differently with regard to global vars ''' ''' this is a cheap work around to that until all functions no longer rely on global variables ''' + ''' found out why, dash: is the default in testinfra run() for Docker + Should try and convert the tests using this to firewalld style test + or override the run() function to use bash instead ''' Pihole.run('cat < {file}\n{script}\nEOF'.format(file=file, script=script)) Pihole.run('chmod +x {}'.format(file)) From a5a067d50fb15c5d1d03f6d0c70bb587c21c11db Mon Sep 17 00:00:00 2001 From: diginc Date: Wed, 2 Nov 2016 23:58:54 -0500 Subject: [PATCH 5/9] switching testinfra's Docker run from dash to bash --- test/conftest.py | 14 ++++++++++++++ test/test_automated_install.py | 3 +-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 407d00dc..e99e47a3 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -8,6 +8,20 @@ check_output = testinfra.get_backend( @pytest.fixture def Pihole(Docker): ''' used to contain some script stubbing, now pretty much an alias ''' + def run_bash(self, command, *args, **kwargs): + cmd = self.get_command(command, *args) + if self.user is not None: + out = self.run_local( + "docker exec -u %s %s /bin/bash -c %s", + self.user, self.name, cmd) + else: + out = self.run_local( + "docker exec %s /bin/bash -c %s", self.name, cmd) + out.command = self.encode(cmd) + return out + + funcType = type(Docker.run) + Docker.run = funcType(run_bash, Docker, testinfra.backend.docker.DockerBackend) return Docker @pytest.fixture diff --git a/test/test_automated_install.py b/test/test_automated_install.py index 6a3daf60..45fecfd0 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -69,11 +69,10 @@ def test_configureFirewall_firewalld_no_errors(Pihole): ''' confirms firewalld rules are applied when appopriate ''' mock_command('firewall-cmd', '0', Pihole) configureFirewall = Pihole.run(''' - bash -c " PHTEST=TRUE source /opt/pihole/basic-install.sh configureFirewall - " ''') + ''') expected_stdout = '::: Configuring firewalld for httpd and dnsmasq.' assert expected_stdout in configureFirewall.stdout firewall_calls = Pihole.run('cat /var/log/firewall-cmd').stdout From d2f815bba7c122d22bdfaf70e2ad2e4272e39fe6 Mon Sep 17 00:00:00 2001 From: diginc Date: Thu, 3 Nov 2016 00:02:28 -0500 Subject: [PATCH 6/9] no longer need to write bash test scripts --- test/test_automated_install.py | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/test/test_automated_install.py b/test/test_automated_install.py index 45fecfd0..19d87fe8 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -20,15 +20,15 @@ def test_setupVars_are_sourced_to_global_scope(Pihole): Pihole.run(setup_var_file) script = dedent('''\ - #!/bin/bash -e + set -e printSetupVars() { # Currently debug test function only echo "Outputting sourced variables" - echo "PIHOLE_INTERFACE=\${PIHOLE_INTERFACE}" - echo "IPV4_ADDRESS=\${IPV4_ADDRESS}" - echo "IPV6_ADDRESS=\${IPV6_ADDRESS}" - echo "PIHOLE_DNS_1=\${PIHOLE_DNS_1}" - echo "PIHOLE_DNS_2=\${PIHOLE_DNS_2}" + echo "PIHOLE_INTERFACE=${PIHOLE_INTERFACE}" + echo "IPV4_ADDRESS=${IPV4_ADDRESS}" + echo "IPV6_ADDRESS=${IPV6_ADDRESS}" + echo "PIHOLE_DNS_1=${PIHOLE_DNS_1}" + echo "PIHOLE_DNS_2=${PIHOLE_DNS_2}" } update_dialogs() { . /etc/pihole/setupVars.conf @@ -50,7 +50,7 @@ def test_setupVars_saved_to_file(Pihole): Pihole.run(set_setup_vars).stdout script = dedent('''\ - #!/bin/bash -e + set -e echo start TERM=xterm PHTEST=TRUE @@ -100,17 +100,7 @@ def mock_command(script, result, container): print container.run('cat {}'.format(full_script_path)).stdout -def run_script(Pihole, script, file="/test.sh"): - _write_test_script(Pihole, script, file=file) - result = Pihole.run(file) +def run_script(Pihole, script): + result = Pihole.run(script) assert result.rc == 0 return result - -def _write_test_script(Pihole, script, file): - ''' Running the test script blocks directly can behave differently with regard to global vars ''' - ''' this is a cheap work around to that until all functions no longer rely on global variables ''' - ''' found out why, dash: is the default in testinfra run() for Docker - Should try and convert the tests using this to firewalld style test - or override the run() function to use bash instead ''' - Pihole.run('cat < {file}\n{script}\nEOF'.format(file=file, script=script)) - Pihole.run('chmod +x {}'.format(file)) From 5b54b9cb11173a0ffcab78d089da47c65236d77b Mon Sep 17 00:00:00 2001 From: diginc Date: Thu, 3 Nov 2016 00:05:19 -0500 Subject: [PATCH 7/9] update Pihole fixture comment --- test/conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/conftest.py b/test/conftest.py index e99e47a3..5960cc24 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -7,7 +7,8 @@ check_output = testinfra.get_backend( @pytest.fixture def Pihole(Docker): - ''' used to contain some script stubbing, now pretty much an alias ''' + ''' used to contain some script stubbing, now pretty much an alias. + Also provides bash as the default run function shell ''' def run_bash(self, command, *args, **kwargs): cmd = self.get_command(command, *args) if self.user is not None: From c2930b0ca5fff156924d19ff71e6fc2ea76fd3bc Mon Sep 17 00:00:00 2001 From: Adam Hill Date: Thu, 3 Nov 2016 08:34:44 -0500 Subject: [PATCH 8/9] remove the debug print in mock_command --- test/test_automated_install.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test_automated_install.py b/test/test_automated_install.py index 19d87fe8..a3c80666 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -97,8 +97,6 @@ def mock_command(script, result, container): cat < {script}\n{content}\nEOF chmod +x {script} '''.format(script=full_script_path, content=mock_script)) - print container.run('cat {}'.format(full_script_path)).stdout - def run_script(Pihole, script): result = Pihole.run(script) From b9f3493dbc13dffb72172118abb5bf8f8312dd04 Mon Sep 17 00:00:00 2001 From: diginc Date: Thu, 3 Nov 2016 22:34:04 -0500 Subject: [PATCH 9/9] move PH_TRUE to Dockerfiles to DRY --- automated install/basic-install.sh | 2 +- test/centos.Dockerfile | 2 ++ test/debian.Dockerfile | 3 ++- test/test_automated_install.py | 2 -- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh index 534b194d..491f7b6d 100755 --- a/automated install/basic-install.sh +++ b/automated install/basic-install.sh @@ -1149,6 +1149,6 @@ main() { echo "::: The install log is located at: /etc/pihole/install.log" } -if [[ "${PHTEST}" != "TRUE" ]] ; then +if [[ "${PH_TEST}" != true ]] ; then main "$@" fi diff --git a/test/centos.Dockerfile b/test/centos.Dockerfile index 9af7eb4d..00543b67 100644 --- a/test/centos.Dockerfile +++ b/test/centos.Dockerfile @@ -11,4 +11,6 @@ ENV PATH /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$SCRIPTDIR RUN true && \ chmod +x $SCRIPTDIR/* +ENV PH_TEST true + #sed '/# Start the installer/Q' /opt/pihole/basic-install.sh > /opt/pihole/stub_basic-install.sh && \ diff --git a/test/debian.Dockerfile b/test/debian.Dockerfile index b80d6155..931c0ba7 100644 --- a/test/debian.Dockerfile +++ b/test/debian.Dockerfile @@ -8,8 +8,9 @@ ADD . $GITDIR RUN cp $GITDIR/advanced/Scripts/*.sh $GITDIR/gravity.sh $GITDIR/pihole $GITDIR/automated\ install/*.sh $SCRIPTDIR/ ENV PATH /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$SCRIPTDIR - RUN true && \ chmod +x $SCRIPTDIR/* +ENV PH_TEST true + #sed '/# Start the installer/Q' /opt/pihole/basic-install.sh > /opt/pihole/stub_basic-install.sh && \ diff --git a/test/test_automated_install.py b/test/test_automated_install.py index a3c80666..ee3beeee 100644 --- a/test/test_automated_install.py +++ b/test/test_automated_install.py @@ -53,7 +53,6 @@ def test_setupVars_saved_to_file(Pihole): set -e echo start TERM=xterm - PHTEST=TRUE source /opt/pihole/basic-install.sh {} finalExports @@ -69,7 +68,6 @@ def test_configureFirewall_firewalld_no_errors(Pihole): ''' confirms firewalld rules are applied when appopriate ''' mock_command('firewall-cmd', '0', Pihole) configureFirewall = Pihole.run(''' - PHTEST=TRUE source /opt/pihole/basic-install.sh configureFirewall ''')