Adding unit tests for utilities.py

[PhillypHenning]

Adds to #286

Description

Adding unit tests for the utilities.py file

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

It is the testing 👍
python3 -m unittest scripts/tests/unit/test_utilities.py

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation

[runforesight]

Foresight Summary

    
Major Impacts

CI duration(37 seconds) has decreased 2 minutes 30 seconds compared to main branch avg(3 minutes 7 seconds).

View More Details

✅  CI workflow has finished in 37 seconds (2 minutes 30 seconds less than main branch avg.) and finished at 4th Apr, 2023.

---

Job	Failed Steps	Tests
tests-unit	-     🔗	N/A	See Details
lint-pylint	-     🔗	N/A	See Details
lint-black	-     🔗	N/A	See Details

---

🔎 See details on Foresight

^{*You can configure Foresight comments in your organization settings page.}

[arm4b]

Nice!

Yes, these are really unit tests as you're testing a specific function.

Move them under the /unit/ dir.
Also, rename utilities_test.py -> test_utilities.py.

See https://github.com/bitovi/bitops/tree/main/scripts/tests/unit existing structure.

[PhillypHenning]

So the BITOPS_RUN_MODE = 'testing' doesn't work but it does need to.
I'm not exactly sure how to accomplish this yet, but I also spent 2 minutes thinking about it.

Other then that, these were created using OpenAI. The prompt goes as followed:

Let's define a persona called PythonTestWriter. The PythonTestWriter is an expert in python. The PythonTestWriter seeks to create comprehensive unit tests for functions. The PythonTestWriter will prioritize using python default libraries such as unittest.

Using the PythonTestWriter, please write unit tests for the following function:
"""
FUNCTION
"""

[PhillypHenning]

These still need to be cleaned up. The first one that should be cleaned up is the run_cmd as I believe it will close a few of the handle_hook test failures

Ran 9 tests in 0.019s

FAILED (failures=4)

[arm4b]

How Has This Been Tested?

It is the testing 👍
python3 -m unittest scripts/tests/unit/test_utilities.py

See https://github.com/bitovi/bitops/tree/main/scripts/tests#running-the-tests which gives an example command with all the configuration/environment.

[arm4b]

Unit Tests ideally shouldn't write to a file for logging.

I saw you tried to disable the logging in another PR, but this should do it in a more native way.

This fixes the Unit Tests run in this PR without relying on a chain of PRs.

[arm4b]

Allowing to test our stuff triggered a lot of the changes.

As we're moving the exception handling outside of the function, - it's better to also log errors outside of the function.

Example pattern:

# Function definition should raise an exception
def example():
  if ok:
    do thing
    logger.debug("Some debugging information in the function is OK")
  else:
    raise ExampleException("Custom error here goes outside as an Exception")

# Usage of the function in the main code
# should catch the exception
# and handle the error logging and return codes, eg. full error handling. 
try:
  example()
except ExampleException as e:
  # error handling is moved outside of the function
  logger.error("An error occurred while trying to run an example: " + e)
  sys.exit(1)

[arm4b]

Updated it to be ready for merge.
Many things needed more work after the AI touch.

@PhillypHenning Let me know in case of any questions and let's merge it.

[PhillypHenning]

Error message for deploy and install should be similar.

[arm4b]

This one comes from the Exception, eg. when something wrong happens on the Popen system side like a timeout, permissions issue, etc.

What would be the more expected error message to you?
Feel free to push directly to the branch or you can also propose a code suggestion (https://egghead.io/lessons/github-add-suggestions-in-a-github-pr-review) for better clarity.

[PhillypHenning]

run_cmd try catch blocks exit with 1xx, should we consider making this a 1xx exit as well? What denotes a 2 vs 101

[arm4b]

That's how you added it here initially: eaeba19#diff-ec9057b7cfb0579064835fe003bf275eb479b2910d976987366051d5d7593c9dR24, so that's more a question to you.

There is no difference to me, unless we're returning the non-zero error code.

[arm4b]

Updated in 92ffbd1

Suggested change

	sys.exit(2)
	sys.exit(101)