Skip to content

Conversation

azu
Copy link
Member

@azu azu commented Jul 5, 2025

Summary

  • Add comprehensive configuration options for MCP server to support various use cases
  • Refactor test helpers to reduce over-abstraction and improve maintainability
  • Add comprehensive test coverage for error scenarios and edge cases
  • Improve documentation with detailed MCP server configuration guide

Changes Made

1. Enhanced MCP Server Configuration

  • Added McpServerOptions type with comprehensive configuration options
  • Support for custom config files, working directories, and quiet mode
  • Direct descriptor injection for testing scenarios
  • Backward compatibility maintained with existing API

2. Test Structure Improvements

  • Replaced over-abstracted testFiles object with direct fixture path usage
  • Refactored testServerOptions to more focused helper functions
  • Added test cases for isError: true scenarios (unsupported file extensions)
  • Enhanced edge case testing (empty files, large files, multiple files)

3. Comprehensive Test Coverage

  • Configuration validation tests
  • Error handling scenarios (invalid config files, empty inputs)
  • Backward compatibility verification
  • Edge case handling (unsupported extensions, whitespace-only content)

4. Documentation

  • Added detailed MCP server configuration documentation
  • Usage examples for different configuration scenarios
  • Error handling and backward compatibility notes

Test Plan

  • All existing tests pass
  • New configuration options work as expected
  • Error scenarios are handled gracefully
  • Backward compatibility is maintained
  • Edge cases are properly covered

fix #1625

🤖 Generated with Claude Code

- Add comprehensive configuration options for MCP server
- Refactor test helpers to reduce over-abstraction
- Add test cases for error scenarios (unsupported file extensions)
- Replace testFiles object with direct fixture path usage
- Add detailed documentation for MCP server configuration
- Improve test coverage for edge cases and configuration validation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@Copilot Copilot AI review requested due to automatic review settings July 5, 2025 05:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the MCP server by introducing customizable options, refactoring tests for clarity, and expanding documentation to cover all configuration scenarios.

  • Introduce McpServerOptions to configure custom config files, working directories, quiet mode, and direct descriptor injection.
  • Refactor test helpers and expand test coverage for configuration, edge cases, and error scenarios.
  • Add comprehensive documentation with usage examples, detailed option descriptions, and backward compatibility notes.

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/textlint/src/mcp/server.ts Accepts McpServerOptions in setupServer and connectStdioMcpServer, propagates to makeLinterOptions.
packages/textlint/test/mcp/test-helpers.ts New helper functions for creating server options and resolving fixture/config paths.
packages/textlint/test/mcp/server.test.ts Expanded tests for custom configuration, edge cases, error scenarios, and backward compatibility.
packages/textlint/docs/mcp-server.md New documentation covering MCP server configuration, option details, examples, and error handling.
packages/textlint/test/mcp/fixtures/unsupported.xyz Added fixture with unsupported extension to test error handling.
packages/textlint/test/mcp/fixtures/lint-errors.md Added fixture containing lint errors for rule testing.
packages/textlint/test/mcp/fixtures/large-file.md Added large-file fixture to validate performance handling.
packages/textlint/test/mcp/fixtures/configs/strict-config.json Added strict-config fixture for validating rule enforcement.
packages/textlint/test/mcp/fixtures/configs/minimal-config.json Added minimal-config fixture to test default behavior.
Comments suppressed due to low confidence (3)

packages/textlint/src/mcp/server.ts:26

  • When loading the configuration via loadTextlintrc, the provided cwd option isn't passed through—consider including cwd: options?.cwd so config resolution respects the working directory.
        (await loadTextlintrc({

packages/textlint/docs/mcp-server.md:24

  • The documented tool names (getLintFixedFileContent / getLintFixedTextContent) do not match the actual server API (fixFiles and fixText). Update the docs to reflect the correct command names.
- `getLintFixedFileContent`: Get lint-fixed content of files

packages/textlint/docs/mcp-server.md:85

  • The example imports builtInPlugins from textlint, but in code it comes from @textlint/kernel. Adjust the import path in the documentation to match the actual module.
import { builtInPlugins } from "textlint";

azu and others added 10 commits July 5, 2025 14:36
- Remove unnecessary mcp-server.md documentation (internal use only)
- Remove over-abstracted test-helpers.ts file
- Inline helper functions directly in test file for better clarity
- Add proper isError test cases for actual error scenarios
- Fix test expectations to match actual textlint behavior
- Use real configuration files that work with available plugins

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove getFixturePath and getConfigPath helper functions
- Replace all usages with direct path.join(__dirname, "fixtures", filename)
- Eliminate unnecessary abstraction for better clarity and directness

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add comprehensive test suite using configFilePath and node_modulesDir
- Create fake node_modules structure with custom rules and plugins
- Implement test-rule that detects TODO items in documents
- Implement test preset with multiple rules (test-rule and no-exclamation)
- Add mock @textlint/textlint-plugin-text and markdown plugins
- Add test fixtures with-todo.md and with-exclamation.md
- Add configuration files for custom rule and preset testing
- Verify that MCP server works with real textlint configurations
- Follow cli-snapshot pattern for isolated fixture-based testing

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add comprehensive snapshot testing framework for MCP server
- Implement type-safe input.json for MCP tool requests
- Create output.json snapshots with normalized timestamps and paths
- Add utilities for path normalization and timestamp masking
- Support UPDATE_SNAPSHOT=1 environment variable for snapshot updates
- Follow cli-snapshots pattern with directory-based test organization

Test cases included:
- basic-lint-file: Simple file linting with no errors
- custom-rule-todo: Custom rule testing with fixture configuration
- lint-text-basic: Text linting without configuration
- error-unsupported-extension: Error case with unsupported file extension

Benefits:
- Type-safe test input definition with TypeScript
- Deterministic snapshots with masked timestamps/paths
- Easy test case addition by creating new directories
- Clear separation of test data and test logic
- Cross-platform compatibility with path normalization

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Replace 'any' types with 'unknown' in types.ts and snapshot-utils.ts
- Fix string concatenation to use template literals
- Add proper type annotations for object entries and arrays
- Ensure type safety throughout snapshot testing utilities

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Create schemas.ts with TextlintMessageSchema definition
- Import and use TextlintMessageSchema consistently in MCP server
- Fix test failures by ensuring proper schema validation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove testInvalidSchema tool from MCP server
- Remove corresponding tests for the tool
- Simplify test count expectations in test suite

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Enhanced path normalization for Windows/Unix compatibility
- Improved timestamp replacement in JSON strings
- Updated snapshot files to current format

Note: Snapshot tests may still fail on different platforms due to
timestamp variations - this is expected behavior for now.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- snapshot-utils.tsでprefer-templateエラーを修正(文字列連結からテンプレートリテラルに変更)
- JSON文字列内のtimestamp正規化を改善
- content fieldに含まれるJSONのtimestamp値も正しく<timestamp>に置換されるように修正
- MCPスナップショットテストが全てパスするようになった

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@azu azu self-assigned this Jul 5, 2025
@azu azu added the Type: Testing Adding missing tests or correcting existing tests label Jul 5, 2025
azu and others added 15 commits July 5, 2025 23:35
- Windowsの絶対パス(D:\a\textlint\...)を適切に正規化するように改善
- JSON文字列内のパスも正規化されるよう修正
- structuredContentのfilePathも正規化する専用関数を追加
- テストディレクトリパターンをクロスプラットフォーム対応に変更
- スナップショットファイルを更新してWindows CI対応

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Enable TypeScript files (input.ts) for test input definition instead of input.json
- Add SnapshotInputFactory and SnapshotContext types for dynamic configuration
- Eliminate string placeholders like <rule_modules> with dynamic path resolution
- Inject actual paths via context.ruleModulesDir at runtime
- Improve type safety and IDE code completion
- Migrate all 4 existing snapshot tests to input.ts format
- Maintain backward compatibility with both input.json and input.ts support
- Remove legacy input.json files after successful migration

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…ment

- Replace complex regex-based path normalization with simple string replacement
- Use exact path matching with escapeRegExp to avoid cross-platform issues
- Eliminate Windows-specific logic by relying on Node.js path.relative()
- Significantly reduce code complexity while maintaining functionality
- Update snapshots with simplified path normalization
- More reliable across different operating systems and path formats

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Replace complex regex-based path replacement with simple String.replaceAll()
- More reliable across platforms without regex escaping complexity
- Add fallback pattern matching for Windows absolute paths
- Remove unused escapeRegExp function
- Simplify path replacement logic with structured approach
- Better handling of edge cases in path normalization

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Replace complex regex-based path replacement with simple replaceAll
- Use test-case names as placeholders (<test-case>/file.txt) instead of full paths
- This eliminates Windows/Unix path separator differences in JSON content
- Update snapshots with the new test-case-based placeholder format
- Simplify pathReplacer logic from ~100 lines to ~40 lines for better maintainability

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
azu and others added 5 commits July 6, 2025 01:35
- Handle Windows paths that are JSON stringified (D:\path -> D:\\\\path)
- Add jsonStringifyValue helper to handle escaped path patterns
- Support both regular and JSON-escaped path replacements
- Remove debug console logs

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@azu azu enabled auto-merge (squash) July 5, 2025 23:24
@azu azu merged commit c3ddad1 into master Jul 5, 2025
17 checks passed
@azu azu deleted the 1625 branch July 5, 2025 23:28
@github-actions github-actions bot mentioned this pull request Jul 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Testing Adding missing tests or correcting existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve MCP Server Testing: Add Configuration Options and Test Coverage
1 participant