Refactor test cases and enhance MCP tool command logic

Replaced redundant mocks and improved method naming for clarity in test cases. Refactored `execute` logic to ensure proper initialization of input and output handling. Added new tests to validate tool selection and input data handling scenarios, and improved tool listing functionality. Migrated configuration management to a compiler pass, simplifying the bundle setup.

Author: klapaudius

src/Command/TestMcpToolCommand.php

@@ -56,8 +56,8 @@ protected function configure(): void
      */
     public function execute(InputInterface $input, OutputInterface $output): int
     {
-        $this->input = $input;
-        $this->io = new SymfonyStyle($input, $output);
+        $this->input ??= $input;
+        $this->io ??= new SymfonyStyle($input, $output);
 
         // List all tools if --list option is provided
         return $this->input->getOption('list')
@@ -283,7 +283,7 @@ public function askForInputData(array $schema): ?array
     /**
      * List all available tools.
      */
-    public function listAllTools(): int
+    private function listAllTools(): int
     {
         $configuredTools = $this->container->getParameter('klp_mcp_server.tools');
 

src/KlpMcpServerBundle.php

@@ -11,35 +11,6 @@
 
 class KlpMcpServerBundle extends Bundle
 {
-    public function boot(): void
-    {
-        parent::boot();
-        $configPath = $this->container->getParameter('kernel.project_dir').'/config/packages/klp_mcp_server.yaml';
-        $filesystem = new Filesystem;
-
-        // Check if the file already exists
-        if (! $filesystem->exists($configPath)) {
-            $defaultConfig = __DIR__.'/Resources/config/packages/klp_mcp_server.yaml';
-            $filesystem->copy($defaultConfig, $configPath);
-        }
-    }
-
-    public function configurePackage(Package $package): void
-    {
-        /*
-         * This class is a Package Service Provider
-         *
-         * More info: https://github.com/spatie/symfony-package-tools
-         */
-        $package
-            ->name('symfony-mcp-server')
-            ->hasConfigFile('mcp-server')
-            ->hasCommands([
-                MakeMcpToolCommand::class,
-                TestMcpToolCommand::class,
-            ]);
-    }
-
     public function build(ContainerBuilder $container): void
     {
         parent::build($container);

tests/Command/TestMcpToolCommandTest.php

@@ -10,7 +10,7 @@
 use PHPUnit\Framework\Attributes\Small;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
-use stdClass;
+use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -38,15 +38,15 @@ protected function setUp(): void
         $this->ioMock = $this->createMock(SymfonyStyle::class);
 
         $this->command = new TestMcpToolCommand($this->containerMock);
-        $this->command->setApplication($this->createMock(\Symfony\Component\Console\Application::class));
+        $this->command->setApplication($this->createMock(Application::class));
         $this->injectPrivateProperty($this->command, 'input', $this->inputMock);
         $this->injectPrivateProperty($this->command, 'io', $this->ioMock);
     }
 
     /**
      * Tests that an exception is thrown when no tool is provided
      */
-    public function test_get_tool_instance_no_tool_provided_throws_exception(): void
+    public function test_get_tool_instance_no_tool_provided_and_no_tool_configured_throws_exception(): void
     {
         $this->inputMock
             ->method('getArgument')
@@ -59,6 +59,32 @@ public function test_get_tool_instance_no_tool_provided_throws_exception(): void
         $this->invokeGetToolInstanceMethod();
     }
 
+    /**
+     * Tests that an exception is thrown when no tool is provided
+     */
+    public function test_get_tool_instance_no_tool_provided_ask_choice_from_configured_tools(): void
+    {
+        $this->containerMock
+            ->method('getParameter')
+            ->with('klp_mcp_server.tools')
+            ->willReturn([HelloWorldTool::class]);
+        $this->containerMock
+            ->method('get')
+            ->with(HelloWorldTool::class)
+            ->willReturn(new HelloWorldTool);
+        $this->inputMock
+            ->method('getArgument')
+            ->with('tool')
+            ->willReturn(null);
+
+        $this->ioMock
+            ->expects($this->once())
+            ->method('choice')
+            ->with('Select a tool to test', ['hello-world ('.HelloWorldTool::class.')']);
+
+        $this->invokeGetToolInstanceMethod();
+    }
+
     /**
      * Tests that a tool instance is returned when a valid class name is provided
      */
@@ -412,6 +438,12 @@ private function invokePrivateMethod(string $methodName, array $args = []): mixe
      */
     public function test_list_all_tools_when_no_tools_are_configured_displays_warning(): void
     {
+        $this->inputMock
+            ->expects($this->once())
+            ->method('getOption')
+            ->with('list')
+            ->willReturn('list');
+
         $this->containerMock
             ->method('getParameter')
             ->with('klp_mcp_server.tools')
@@ -422,14 +454,19 @@ public function test_list_all_tools_when_no_tools_are_configured_displays_warnin
             ->method('warning')
             ->with('No MCP tools are configured. Add tools in config/package/klp-mcp-server.yaml');
 
-        $this->assertEquals(Command::SUCCESS, $this->command->listAllTools());
+        $this->assertEquals(Command::SUCCESS, $this->command->execute($this->inputMock, $this->outputMock));;
     }
 
     /**
      * Tests that when valid tools are present, they are displayed in a table.
      */
     public function test_list_all_tools_when_valid_tools_present_displays_table(): void
     {
+        $this->inputMock
+            ->expects($this->once())
+            ->method('getOption')
+            ->with('list')
+            ->willReturn('list');
         $tools = [HelloWorldTool::class, VersionCheckTool::class];
         $toolMocks = [
             ['name' => 'Tool1', 'class' => HelloWorldTool::class, 'description' => 'This is tool 1'],
@@ -453,14 +490,19 @@ public function test_list_all_tools_when_valid_tools_present_displays_table(): v
             ->method('table')
             ->with(['Name', 'Class', 'Description'], $toolMocks);
 
-        $this->assertEquals(Command::SUCCESS, $this->command->listAllTools());
+        $this->assertEquals(Command::SUCCESS, $this->command->execute($this->inputMock, $this->outputMock));
     }
 
     /**
      * Tests that when a tool class cannot be loaded, it is gracefully handled.
      */
     public function test_list_all_tools_handles_tool_loading_exceptions_gracefully(): void
     {
+        $this->inputMock
+            ->expects($this->once())
+            ->method('getOption')
+            ->with('list')
+            ->willReturn('list');
         $tools = [HelloWorldTool::class];
 
         $this->containerMock
@@ -477,7 +519,7 @@ public function test_list_all_tools_handles_tool_loading_exceptions_gracefully()
             ->method('warning')
             ->with("Couldn't load tool class: " . HelloWorldTool::class);;
 
-        $this->assertEquals(Command::SUCCESS, $this->command->listAllTools());
+        $this->assertEquals(Command::SUCCESS, $this->command->execute($this->inputMock, $this->outputMock));
     }
 
     /**
@@ -713,4 +755,32 @@ public function test_ask_for_input_data_handle_alpha_for_integer(): void
 
         $this->assertEquals([], $result);
     }
+
+    /**
+     * Tests that askForInputData handles invalid JSON for object property.
+     */
+    public function test_ask_for_input_data_handle_alpha_for_required_integer(): void
+    {
+        $schema = [
+            'properties' => [
+                'age' => [
+                    'type' => 'integer',
+                    'description' => 'How old are you?',
+                ],
+            ],
+            'required' => ['age'],
+        ];
+        $this->ioMock
+            ->method('ask')
+            ->with('Value', false)
+            ->willReturn('twenty');
+        $this->ioMock
+            ->expects($this->once())
+            ->method('warning')
+            ->with('Required field skipped. Using 0.');
+
+        $result = $this->command->askForInputData($schema);
+
+        $this->assertEquals(['age' => 0], $result);
+    }
 }

tests/KlpMcpServerBundleTest.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace KLP\KlpMcpServer\Tests;
+
+use KLP\KlpMcpServer\DependencyInjection\CompilerPass\ToolsDefinitionCompilerPass;
+use KLP\KlpMcpServer\KlpMcpServerBundle;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * This class tests the build method of the KlpMcpServerBundle class.
+ */
+class KlpMcpServerBundleTest extends TestCase
+{
+    /**
+     * Tests that the build method adds the ToolsDefinitionCompilerPass to the container.
+     */
+    public function test_build_adds_compiler_pass()
+    {
+        $bundle = new KlpMcpServerBundle();
+        $containerBuilder = $this->createMock(ContainerBuilder::class);
+
+        $containerBuilder
+            ->expects($this->once())
+            ->method('addCompilerPass')
+            ->with($this->isInstanceOf(ToolsDefinitionCompilerPass::class));
+
+        $bundle->build($containerBuilder);
+    }
+}