Include tests to the python program that can be executed by github workflow

[funbeedev]

Current Behaviour

Execution of the python program in the github workflow will fail.

Changes Requested

Include tests in the python program.
Setup workflow to execute these tests.

[Stedders]

I'll look at this, it will help with #5 as well, under https://github.com/Stedders/AutomateGitRepoSetup/tree/Stedders.

[funbeedev]

Great, thanks. Please issue a PR per major change you are making.
Also I can see in your fork the code formatting is changed in some parts but please keep the original formats and only change parts of the code that you are adding functionality to.
Note, you can ignore any linting errors to do with line numbers too long.

[Stedders]

Hey @funbeedev

I've gone through and reverted the changes to the main file, it is difficult to unit test the code but I have committed a change that shows how it could be done.

On the latest commit of my branch I have added a test folder with a pytest script, this excutes the main start_program_flow function, the git workflow is setup to run pytest on every push.

I would strongly recommend refactoring the code with the following changes...

1. Allows the config_file path to be variable and passed into start_program_flow() - allows us to generate configurations separate to the root directory
2. Don't read the configuration in the module scope - as it stands the config is loaded when the module is loaded from the running dir, this results in all variables being defined at start
3. Add arguments to the functions so they are not all reading from a single object in the outer scope - allows for more specificof the functions

As it stands I won't create the PR as the code fails flake8 linting (start_program_flow() is not in the linting scope as it is checking the code statically), this can be rectified but would need the config.ini file in the route of the directory to be manipulated during running, which feels wrong.

[funbeedev]

@Stedders Thanks, I'll review your changes and suggestions and get back to you soon.
Btw, you can still issue the PR if you want even though everything might not be working. It makes it easier for me to pull your changes.
Also you can make changes to a PR after its issued by just pushing more changes to your fork!

[funbeedev]

@Stedders
In response to your suggestions on refactoring:

1. Allows the config_file path to be variable and passed into start_program_flow() - allows us to generate configurations separate to the root directory

Is this just for more flexibility? How do you suggest the path of the config file be passed in?

2. Don't read the configuration in the module scope - as it stands the config is loaded when the module is loaded from the running dir, this results in all variables being defined at start

Do you just mean to load the config within a function instead?

3. Add arguments to the functions so they are not all reading from a single object in the outer scope - allows for more specificof the functions

I assume this relates to your point 2 and is about avoid use of global variables. I agree this aspect of the code needs to be improved! I'll create an issue for this.

[Stedders]

Is this just for more flexibility? How do you suggest the path of the config file be passed in?

I would recommend that we add a param start_program_flow(config_file)

1. If you want the path to always be config.ini then the __main__section will call start_program_flow('config.ini') to set it
2. Testing can call start_program_flow('test-1.ini'), start_program_flow('test-2.ini'), etc...
3. In the future you could add it as an argument to the program that can be passed through

Do you just mean to load the config within a function instead?

Yes, there are a couple of options...

1. Use a global that can be accessed by all functions
2. Pass the config items to the functions separately

I would prefer the latter, as it allows for more granular testing - for example commit_files(msg, repo_directory) can be tested multiple times with different messages - normal, blank, None, Japanese characters, etc... very quickly and specifically

I assume this relates to your point 2 and is about avoid use of global variables. I agree this aspect of the code needs to be improved! I'll create an issue for this.

👍 it feels like the better way of doing it.

[funbeedev]

@Stedders
Sorry the delay in responding.

Those are all great suggestions, thank you! I've made issues based on your suggestions and they will be addressed soon.

[Stedders]

@funbeedev - I'm happy to hold on the unit testing, in addition I can pick up any other little bits you need help with (either coding or reviewing).

[funbeedev]

@Stedders Thanks, your help is appreciated!
See PR #16. This removes use of globals. Could you review the changes?

[funbeedev]

@Stedders I've made your suggested changes now and it's merged into the repo. For the relevant changes see PR #16 and #17.
Do you want to pull these changes and update the test in your PR?
Thanks!