Are my Unit tests written correctly to test my code functions

Question

Trying to write Unit tests to test my Python code project.

I have a gestioner.py file which checks for the User input and see if they are valid (from an allowed list) or not, (from a 'danger_list') and then also checks if the entered command has any suspicious charecters entered to prevent security attacks.

Now I want to add Unit tests to my functions code in the Gestioner.py

Was following the writeup in Stackoverflow question: Writing unit tests in Python: How do I start?

and then py.test documentation (https://docs.pytest.org/en/latest/contents.html)

and I've written following test_gestioner.py to start my unit tests as below:

And all my three tests seeming Pasing, all the time. But I doubt if my these tests are really and correctly testing my code because when I expect the test to fail still that test Passes.

Am I doing this Unit testing of my code correctly ? it appears to me that I am not, so please suggest where am I doing mistake in my unit tests ?

Here are is my main script, Gestioner.py

import sys
import re


class CmdGestioner:
    _allowed_list = ['fichier', 'binfile', 'prep', 'rep' ]
    _danger_list = ['gen', 'regen'] 

    def __init__(self):
        None

    def chk_cmds(self, mycmd):
        print('chk_cmds: lets look term at {}'.format(mycmd))
        regex = re.compile(r'[&`%;$><!#]')
        msg = bool(regex.search(mycmd))
        print('My command: ' +mycmd)
        if msg is True:
            print(msg)
            print('Found a suspicious entry in the command : {}'.format(mycmd))
            return -1
        else:
            print('Command seems clean {}'.format(mycmd))
            return 0


    def is_valid(self, in_command):
        valid = True


        cmd_mtg_str = ''.join(str(elem) for elem in in_command)



        for term in cmd_mtg_str.split():

            if term.strip('--') not in self._allowed_list:
                if term.strip('--') in self._forbidden_list:
                    valid = False
                    print('Found a forbidden Sec_Service command '+term)
                else:
                    print ('%s not found in list so need to runs checks on this!' % (term))
                    checkResult = self.chk_cmds(term)
                    if checkResult == -1:
                        valid = False
            else:
                print ('Found in list so term %s is valid' % (term))
        return valid




test_command = ' '.join(str(elem) for elem in sys.argv[1:])
cmd = CmdGestioner()

status = cmd.is_valid(test_command)
print ('\nStatus for command: %s is %s' % (test_command,status))

and my unit test file (test_gestioner.py)

import sys
import re
import unittest
import pytest
from gestioner import CmdGestioner

cmd = CmdGestioner()


def test_chk_cmds_valid():
    mycmd = "--binfile testbinfile"
    cmd.chk_cmds(mycmd)
    assert 'Status for command: ' +mycmd +' is True'


def test_chk_cmds_danger():
    mycmd = "--gen genf"
    cmd.chk_cmds(mycmd)
    assert 'Status for command: ' +mycmd +' is False'


def test_chk_cmds_attacks():
    mycmd = ";ls"
    cmd.chk_cmds(mycmd)
    assert 'Found a suspicious entry in the command : ' .format(mycmd)
    assert 'Status for command: ' +mycmd +' is False

My Unit tests being executed:

PS C:\Work\Questions_forums\SecService_messaging> py.test -v                                                                                                                                                                                       ================================================================================== test session starts =============================================================================================================================
platform win32 -- Python 3.7.4, pytest-5.0.1, py-1.8.0, pluggy-0.12.0 -- c:\users\SecUser\appdata\local\programs\python\python37\python.exe
cachedir: .pytest_cache
metadata: {'Python': '3.7.4', 'Platform': 'Windows-10-10.0.18362-SP0', 'Packages': {'pytest': '5.0.1', 'py': '1.8.0', 'pluggy': '0.12.0'}, 'Plugins': {'html': '1.21.1', 'metadata': '1.8.0'}}
rootdir: C:\Work\Questions_forums\SecService_messaging
plugins: html-1.21.1, metadata-1.8.0
collected 3 items

test_gestioner.py::test_chk_cmds_valid PASSED                                                                                                                                                                                                                           [ 33%]
test_gestioner.py::test_chk_cmds_danger PASSED                                                                                                                                                                                                                          [ 66%]
test_gestioner.py::test_chk_cmds_attacks PASSED                                                                                                                                                                                                                         [100%]

========================================================================================================================== 3 passed in 0.03 seconds ==========================================================================================================================

Answer 1

No, your unittests are incorrectly written. The chk_cmd method doesn't return a string, it prints to standard out, but your assert statements are looking for the result of your chk_cmd to equal what was printed. Instead, the assert statement is checking if the string equals true which any string of one charter or more is considered true so these tests should always pass. For the assert statements to work as they're written you'd need to return the expected string from the chk_cmd method or test something else. For example

assert cmd.is_valid('rep')

assert cmd.chk_cmds(mycmd) == -1

assert cmd.chk_cmd(mycmd) == 0

It looks like you may come from a C programming background with the -1 and 0 return values. If you rewrote those values as True and False it would improve your code and your assertions. The following refactor:

class CmdGestioner:
    _allowed_list = ['fichier', 'binfile', 'prep', 'rep' ]
    _danger_list = ['gen', 'regen']

    def chk_cmds(self, mycmd):
        regex = re.compile(r'[&`%;$><!#]')
        msg = bool(regex.search(mycmd))
        if msg is True:
            return False
        else:
            return True


    def is_valid(self, in_command):
        valid = True
        cmd_mtg_str = ''.join(str(elem) for elem in in_command)
        for term in cmd_mtg_str.split():
            if term.strip('--') not in self._allowed_list:
                if term.strip('--') in self._danger_list:
                    valid = False
                else:
                    if self.chk_cmds(term):
                        valid = False
        return valid

is both more readable (print statements omitted to see the proposed changes more clearly) and it allows assertions to be rewritten as

assert cmd.chk_cmds(mycmd)

Because the method now returns true or false you don't have to add a conditional statement to the assertion.

The way you have your current assertions should always evaluate to true since you're checking the string and not standard out.