Refactoring

Refacotring to use _pure functions_ operating on data structures
rather than depending on io implementation. This makes unit testing
easier and more extensible.

Signed-off-by: Piotr <piotrzan@gmail.com>

Author: Piotr1215

killercoda_cli/cli.py

@@ -5,25 +5,25 @@
 import difflib
 import sys
 
-def get_tree_output():
+def get_tree_structure():
     # Get the current tree structure as a string
     result = subprocess.run(['tree'], stdout=subprocess.PIPE)
     return result.stdout.decode('utf-8')
 
-def print_diff(old_tree, new_tree):
+def generate_diff(old_tree, new_tree):
     # Use difflib to print a diff of the two tree outputs
     diff = difflib.unified_diff(
         old_tree.splitlines(keepends=True),
         new_tree.splitlines(keepends=True),
         fromfile='Before changes',
         tofile='After changes',
     )
-    print(''.join(diff), end="")
+    return ''.join(diff)
 
 # 1. Traverse the current directory and build a dictionary mapping step numbers to paths
-def get_current_steps_dict():
+def get_current_steps_dict(directory_items):
     steps_dict = {}
-    for item in os.listdir('.'):
+    for item in directory_items:
         if item.startswith('step') and (os.path.isdir(item) or item.endswith('.md')):
             # Extract the step number from the name
             try:
@@ -34,20 +34,17 @@ def get_current_steps_dict():
     return steps_dict
 
 # 2. Take input from the user for the new step's name and the desired step number
-def get_user_input(steps_dict):
-    step_title = input("Enter the title for the new step: ")
+def get_user_input(steps_dict, step_title_input, step_number_input):
+    step_title = step_title_input
     highest_step_num = max(steps_dict.keys(), default=0)
 
     while True:
-        try:
-            step_number = int(input(f"Enter the step number to insert the new step at (1-{highest_step_num+1}): "))
-            if 1 <= step_number <= highest_step_num + 1:
-                break
-            else:
-                print(f"Please enter a valid step number between 1 and {highest_step_num+1}.")
-        except ValueError:
-            print("That's not a valid number. Please try again.")
-    
+        step_number = int(step_number_input)
+        if 1 <= step_number <= highest_step_num + 1:
+            break
+        else:
+            raise ValueError(f"Invalid step number: {step_number_input}. Please enter a valid step number between 1 and {highest_step_num+1}.")
+
     return step_title, step_number
 
 # 3. Determine the renaming and shifting required based on user input
@@ -65,61 +62,61 @@ def plan_renaming(steps_dict, insert_step_num):
     renaming_plan.reverse()
     return renaming_plan
 
-def execute_renaming_plan(renaming_plan):
+def calculate_renaming_operations(renaming_plan):
     # Execute the renaming plan
+    file_operations = []
     for old_name, new_name in renaming_plan:
         # Make the new directory if it doesn't exist
-        os.makedirs(new_name, exist_ok=True)
+        file_operations.append(('makedirs', new_name))
         # If it's a directory, we need to check for background.sh and foreground.sh
         if os.path.isdir(old_name):
             # Check and move background.sh if it exists
             old_background = f"{old_name}/background.sh"
             new_background = f"{new_name}/background.sh"
             if os.path.isfile(old_background):
-                os.rename(old_background, new_background)
+                file_operations.append(('rename', old_background, new_background))
             # Check and move foreground.sh if it exists
             old_foreground = f"{old_name}/foreground.sh"
             new_foreground = f"{new_name}/foreground.sh"
             if os.path.isfile(old_foreground):
-                os.rename(old_foreground, new_foreground)
+                file_operations.append(('rename', old_foreground, new_foreground))
             # Rename the step markdown file
             old_step_md = f"{old_name}/step{old_name.replace('step', '')}.md"
             new_step_md = f"{new_name}/step{new_name.replace('step', '')}.md"
             if os.path.isfile(old_step_md):
-                os.rename(old_step_md, new_step_md)
+                file_operations.append(('rename', old_step_md, new_step_md))
         else:
             # If it's just a markdown file without a directory
             new_step_md = f"{new_name}.md"
-            os.rename(old_name, new_step_md)
+            file_operations.append(('rename', old_name, new_step_md))
+    return file_operations
 
-def add_new_step_file(insert_step_num, step_title):
+def calculate_new_step_file_operations(insert_step_num, step_title):
     # Add the new step folder and files
     new_step_folder = f"step{insert_step_num}"
     new_step_md = f"{new_step_folder}/step{insert_step_num}.md"
     new_step_background = f"{new_step_folder}/background.sh"
     new_step_foreground = f"{new_step_folder}/foreground.sh"
 
-    os.makedirs(new_step_folder, exist_ok=True)
+    file_operations = [('makedirs', new_step_folder)]
 
     # Write the step markdown file
-    with open(new_step_md, 'w') as md_file:
-        md_file.write(f"# {step_title}\n")
+    file_operations.append(('write_file', new_step_md, f"# {step_title}\n"))
 
     # Write a simple echo command to the background and foreground scripts
     script_content = f"#!/bin/sh\necho \"{step_title} script\"\n"
 
-    with open(new_step_background, 'w') as bg_file:
-        bg_file.write(script_content)
-    with open(new_step_foreground, 'w') as fg_file:
-        fg_file.write(script_content)
+    file_operations.append(('write_file', new_step_background, script_content))
+    file_operations.append(('write_file', new_step_foreground, script_content))
 
-    os.chmod(new_step_background, 0o755)
-    os.chmod(new_step_foreground, 0o755)
+    file_operations.append(('chmod', new_step_background, 0o755))
+    file_operations.append(('chmod', new_step_foreground, 0o755))
 
-def update_index_json(steps_dict, insert_step_num, step_title, index_file):
+    return file_operations
+
+def calculate_index_json_updates(steps_dict, insert_step_num, step_title, current_index_data):
     # Load the index.json file
-    with open(index_file, 'r') as file:
-        data = json.load(file)
+    data = current_index_data
 
     # Create new step entry
     new_step_data = {
@@ -136,11 +133,10 @@ def update_index_json(steps_dict, insert_step_num, step_title, index_file):
         step = data['details']['steps'][i]
         step_number = i + 1  # Convert to 1-based index
         step["text"] = f"step{step_number}/step{step_number}.md"
-        step["background"] = f"step{step_number}/background{step_number}.sh"
+        step["background"] = f"step{step_number}/background.sh"
 
     # Write the updated data back to index.json
-    with open(index_file, 'w') as file:
-        json.dump(data, file, ensure_ascii=False, indent=4)
+    return data
 
 def display_help():
     help_text = """
@@ -168,27 +164,64 @@ def main():
     if len(sys.argv) > 1 and sys.argv[1] in ['-h', '--help']:
         display_help()
         sys.exit()
-    old_tree_output = get_tree_output()
-    steps_dict = get_current_steps_dict()
+    old_tree_structure = get_tree_structure()
+    directory_items = os.listdir('.')
+    steps_dict = get_current_steps_dict(directory_items)
     if not steps_dict:
         print("No step files or directories found. Please run this command in a directory containing step files or directories.")
         sys.exit(1)
-    step_title, insert_step_num = get_user_input(steps_dict)
+    step_title_input = input("Enter the title for the new step: ")
+    highest_step_num = max(steps_dict.keys(), default=0)
+    while True:
+        try:
+            step_number_input = input(f"Enter the step number to insert the new step at (1-{highest_step_num+1}): ")
+            insert_step_num = int(step_number_input)
+            if 1 <= insert_step_num <= highest_step_num + 1:
+                break
+            else:
+                print(f"Please enter a valid step number between 1 and {highest_step_num+1}.")
+        except ValueError:
+            print("That's not a valid number. Please try again.")
+    step_title, insert_step_num = get_user_input(steps_dict, step_title_input, step_number_input)
     renaming_plan = plan_renaming(steps_dict, insert_step_num)
 
-    # Execute the renaming plan
-    execute_renaming_plan(renaming_plan)
-
-    # Add the new step
-    add_new_step_file(insert_step_num, step_title)
-
-    # Update the index.json
-    index_file = 'index.json'
-    update_index_json(steps_dict, insert_step_num, step_title, index_file)
-    new_tree_output = get_tree_output()
+    # Calculate the file operations for the renaming plan
+    file_operations = calculate_renaming_operations(renaming_plan)
+    # Execute the file operations
+    for operation in file_operations:
+        if operation[0] == 'makedirs':
+            os.makedirs(operation[1], exist_ok=True)
+        elif operation[0] == 'rename':
+            os.rename(operation[1], operation[2])
+
+    # Calculate the file operations for the new step
+    new_step_operations = calculate_new_step_file_operations(insert_step_num, step_title)
+    # Execute the file operations for the new step
+    for operation in new_step_operations:
+        if operation[0] == 'makedirs':
+            os.makedirs(operation[1], exist_ok=True)
+        elif operation[0] == 'write_file':
+            with open(operation[1], 'w') as file:
+                file.write(operation[2])
+        elif operation[0] == 'chmod':
+            os.chmod(operation[1], operation[2])
+
+    # Read the current index.json data
+    index_file_path = 'index.json'
+    with open(index_file_path, 'r') as index_file:
+        current_index_data = json.load(index_file)
+
+    # Calculate the updates to the index.json data
+    updated_index_data = calculate_index_json_updates(steps_dict, insert_step_num, step_title, current_index_data)
+
+    # Write the updated index.json data back to the file
+    with open(index_file_path, 'w') as index_file:
+        json.dump(updated_index_data, index_file, ensure_ascii=False, indent=4)
+    new_tree_structure = get_tree_structure()
     # Print out the new file structure for confirmation
-    print("\nNew file structure:")
-    print_diff(old_tree_output, new_tree_output)
+    tree_diff = generate_diff(old_tree_structure, new_tree_structure)
+    print("\nFile structure changes:")
+    print(tree_diff, end="")
 
 if __name__ == "__main__":
     main()

tests/test_basic.py

@@ -0,0 +1,110 @@
+#!/usr/bin/env python3
+import unittest
+from unittest import TestCase
+from unittest.mock import patch, MagicMock
+from killercoda_cli import cli
+
+class TestCLI(unittest.TestCase):
+    def test_get_tree_structure(self):
+        # Mock the subprocess.run to return a predefined tree output
+        mock_output = """.
+        ├── step1
+        │   ├── background.sh
+        │   └── step1.md
+        └── step2
+            ├── background.sh
+            └── step2.md
+        """
+        with patch('subprocess.run') as mock_run:
+            mock_run.return_value.stdout = mock_output.encode('utf-8')
+            tree_structure = cli.get_tree_structure()
+            expected_output = mock_output
+            assert tree_structure == expected_output, "The tree structure output was not as expected."
+
+    def test_generate_diff(self):
+        old_tree = "old\nstructure\n"
+        new_tree = "new\nstructure\n"
+        expected_diff = "Expected diff output"
+        with patch('difflib.unified_diff', return_value=expected_diff) as mock_diff:
+            diff = cli.generate_diff(old_tree, new_tree)
+            mock_diff.assert_called_once()
+            assert diff == expected_diff, "The generated diff output was not as expected."
+
+    def test_get_current_steps_dict(self):
+        directory_items = ['step1', 'step2', 'step3.md', 'not_a_step', 'stepX']
+        expected_dict = cli.get_current_steps_dict(directory_items)
+        steps_dict = cli.get_current_steps_dict(directory_items)
+        assert steps_dict == expected_dict, "The steps dictionary did not match the expected output."
+
+    def test_get_user_input(self):
+        steps_dict = {1: 'step1', 2: 'step2', 3: 'step3.md'}
+        step_title_input = "New Step Title"
+        step_number_input = "4"
+        expected_output = (step_title_input, int(step_number_input))
+        user_input = cli.get_user_input(steps_dict, step_title_input, step_number_input)
+        assert user_input == expected_output, "The user input did not match the expected output."
+
+    def test_plan_renaming(self):
+        steps_dict = {1: 'step1', 2: 'step2', 3: 'step3.md'}
+        insert_step_num = 2
+        expected_plan = [('step3.md', 'step4'), ('step2', 'step3')]
+        renaming_plan = cli.plan_renaming(steps_dict, insert_step_num)
+        assert renaming_plan == expected_plan, "The renaming plan did not match the expected output."
+
+    def test_calculate_renaming_operations(self):
+        renaming_plan = [('step2', 'step3'), ('step1', 'step2')]
+        expected_operations = [
+            ('makedirs', 'step3'),
+            ('rename', 'step2/background.sh', 'step3/background.sh'),
+            ('rename', 'step2/foreground.sh', 'step3/foreground.sh'),
+            ('rename', 'step2/step2.md', 'step3/step3.md'),
+            ('makedirs', 'step2'),
+            ('rename', 'step1/background.sh', 'step2/background.sh'),
+            ('rename', 'step1/foreground.sh', 'step2/foreground.sh'),
+            ('rename', 'step1/step1.md', 'step2/step2.md')
+        ]
+        with patch('os.path.isdir', return_value=True), \
+             patch('os.path.isfile', return_value=True):
+            operations = cli.calculate_renaming_operations(renaming_plan)
+            assert operations == expected_operations, "The calculated file operations did not match the expected output."
+
+    def test_calculate_new_step_file_operations(self):
+        insert_step_num = 4
+        step_title = "New Step"
+        expected_operations = [
+            ('makedirs', 'step4'),
+            ('write_file', 'step4/step4.md', '# New Step\n'),
+            ('write_file', 'step4/background.sh', '#!/bin/sh\necho "New Step script"\n'),
+            ('write_file', 'step4/foreground.sh', '#!/bin/sh\necho "New Step script"\n'),
+            ('chmod', 'step4/background.sh', 0o755),
+            ('chmod', 'step4/foreground.sh', 0o755)
+        ]
+        operations = cli.calculate_new_step_file_operations(insert_step_num, step_title)
+        assert operations == expected_operations, "The new step file operations did not match the expected output."
+
+    def test_calculate_index_json_updates(self):
+        steps_dict = {1: 'step1', 2: 'step2', 3: 'step3.md'}
+        insert_step_num = 2
+        step_title = "New Step"
+        current_index_data = {
+            "details": {
+                "steps": [
+                    {"title": "Step 1", "text": "step1/step1.md", "background": "step1/background.sh"},
+                    {"title": "Step 2", "text": "step2/step2.md", "background": "step2/background.sh"}
+                ]
+            }
+        }
+        expected_data = {
+            "details": {
+                "steps": [
+                    {"title": "Step 1", "text": "step1/step1.md", "background": "step1/background.sh"},
+                    {"title": "New Step", "text": "step2/step2.md", "background": "step2/background.sh"},
+                    {"title": "Step 2", "text": "step3/step3.md", "background": "step3/background.sh"}
+                ]
+            }
+        }
+        updated_data = cli.calculate_index_json_updates(steps_dict, insert_step_num, step_title, current_index_data)
+        assert updated_data == expected_data, "The updated index.json data did not match the expected output."
+
+if __name__ == '__main__':
+    unittest.main()