skills/sbroenne__mcp-server-excel
1
0
Fork 0
mirror of https://github.com/sbroenne/mcp-server-excel.git synced 2026-09-19 07:53:08 +08:00
Code Issues Packages Projects Releases Wiki Activity
Files
v1.4.49
sbroenne__mcp-server-excel/tests
T
History
Stefan Broenner 29adbb3b25 refactor: eliminate dead code and cargo cult patterns (-1,286 lines) (#334) 
* refactor: eliminate cargo cult patterns in PivotTable and Range commands

- Create PivotTableHelpers class with centralized OLAP detection:
  - IsOlapPivotTable() - boolean check with COM cleanup
  - TryGetCubeFields() - returns cubeFields for subsequent operations

- Replace duplicated OLAP detection try-catch blocks across 8 files:
  - PivotTableCommands.Fields.cs
  - PivotTableCommands.Lifecycle.cs
  - PivotTableCommands.cs
  - PivotTableCommands.CalculatedMembers.cs
  - PivotTableCommands.CalculatedFields.cs
  - OlapPivotTableFieldStrategy.cs
  - RegularPivotTableFieldStrategy.cs
  - PivotTableFieldStrategyFactory.cs

- Fix nullable bool comparisons (== true -> is true):
  - RangeCommands.Validation.cs
  - OlapPivotTableFieldStrategy.cs (3 instances)
  - PowerQueryCommands.Helpers.cs

Fixes #333

* refactor: eliminate PivotTable strategy execution boilerplate

- Create ExecuteWithStrategy<TResult> helper method centralizing:
  - PivotTable lookup via CoreLookupHelpers.FindPivotTable
  - Strategy creation via PivotTableFieldStrategyFactory
  - Operation execution with proper COM cleanup in finally block

- Refactor 12 methods to use helper (expression-bodied):
  - Fields: AddRowField, AddColumnField, AddValueField, AddFilterField,
            RemoveField, SetFieldFunction, SetFieldName
  - Grouping: GroupByDate, GroupByNumeric
  - Layout: SetLayout, Subtotals: SetSubtotals, GrandTotals: SetGrandTotals
  - CalculatedFields: CreateCalculatedField
  - Analysis: SetFieldFilter, SortField

- Remove unused 'using Sbroenne.ExcelMcp.ComInterop' from 4 files
- Remove redundant null check in SetGrandTotals (FindPivotTable throws)

Methods kept as-is (require context access or complex logic):
- SetFieldFormat (needs ctx.FormatTranslator)
- ListFields, GetData (complex iteration/extraction logic)
- ListCalculatedFields, DeleteCalculatedField (OLAP-specific paths)

Reduction: ~150 lines of duplicate boilerplate eliminated

* refactor: eliminate redundant table null checks

- Change TableCommands.FindTable to call CoreLookupHelpers.FindTable
  (which throws) instead of TryFindTable (which returns null)

- Remove 22 redundant null checks across 9 TableCommands files:
  - Data.cs (2), Filters.cs (4), Lifecycle.cs (3)
  - NumberFormat.cs (2), Sort.cs (2), Structure.cs (4)
  - StructuredReferences.cs (1), Columns.cs (3), DataModel.cs (1)

- Same pattern fix as PivotTable strategy execution:
  FindTable now throws, so callers don't need to check+throw

Reduction: 88 lines of redundant null check boilerplate eliminated

* refactor: remove dead aggregation helpers and consolidate VBA type mapping

- Remove 3 unused methods from PivotTableCommands.cs (IsValidAggregationForDataType,
  GetValidAggregationsForDataType, GetComAggregationFunction)
- Remove 1 unused method from OlapPivotTableFieldStrategy.cs (GetComAggregationFunction)
- Add GetVbaModuleTypeName() helper to VbaCommands.cs
- Replace 2 inline switch expressions with helper in VbaCommands.Lifecycle.cs

Net reduction: 71 lines (17+/88-)

* refactor: consolidate GetAreaName into PivotTableHelpers

- Add GetAreaName() to PivotTableHelpers.cs as shared utility
- Remove duplicated GetAreaName() from PivotTableCommands.cs (dead code)
- Remove duplicated GetAreaName() from OlapPivotTableFieldStrategy.cs
- Remove duplicated GetAreaName() from RegularPivotTableFieldStrategy.cs
- Update all 8 usages to use PivotTableHelpers.GetAreaName()

Net reduction: 27 lines (26+/53-)

* refactor: consolidate GetAggregationFunctionFromCom into PivotTableHelpers

* refactor: consolidate DetectFieldDataType into PivotTableHelpers

- Move DetectFieldDataType from PivotTableCommands and RegularPivotTableFieldStrategy to PivotTableHelpers
- Update 8 usages across 4 files to use centralized helper
- Eliminates ~50 lines of duplicated data type detection logic

* refactor: consolidate GetFieldUniqueValues into PivotTableHelpers

- Add GetFieldUniqueValues to PivotTableHelpers (identical logic)
- Remove dead code from PivotTableCommands.cs (method was defined but never called)
- Remove duplicate from RegularPivotTableFieldStrategy.cs
- Update 3 callers to use centralized helper
- Eliminates ~40 lines of duplicated/dead code

* refactor: consolidate ParseColor and ParseBorderStyle into FormattingHelpers

* refactor: remove dead WithSessionAsync method

* refactor: remove dead code (-937 lines)

Removed unused code across all layers:

Core:
- IsValidTableName (TableCommands.cs) - never called
- CreatePowerQueryPipeline, PowerQueryConfig, POWER_QUERY_DATA_MODEL_ERROR (ResiliencePipelines.cs)
- RemoveQueryTablesFromSheet, CreateQueryTable (PowerQueryHelpers.cs)
- RemoveQueryConnections, CreateQueryTableConnection, SetQueryLoadToDataModel, CheckQueryDataModelConfiguration (PowerQueryCommands.Helpers.cs)
- GetConnectionNames, RemoveConnections (ConnectionHelpers.cs)
- ExcelConstants.cs - entire file never referenced

ComInterop:
- SpinWaitDelayMs, XlExcel12Binary constants

MCP Server:
- style_names.md, powerquery_targetCellAddress.md - completion files never loaded
- excel_datamodel.md - prompt content never wired up (info already in tool description)

Tests:
- CreateMultipleConnections (ConnectionTestHelper.cs)

* refactor: remove dead completion handler infrastructure (-349 lines)

Removed ExcelCompletionHandler.cs and supporting files which were
written but never wired up to the MCP server (HandleCompletion never called).

Deleted files:
- ExcelCompletionHandler.cs (178 lines) - never registered with MCP
- 7 completion markdown files (border_styles, border_weights, load_destination,
  privacy_level, table_styles, validation_operators, validation_types)

Also cleaned up:
- Removed LoadCompletion/LoadCompletionValues from MarkdownLoader.cs
- Removed unused ListEmbeddedResources debugging method
- Updated mcp-llm-guidance.instructions.md to reflect actual status

* fix: suppress CA1031 warnings for intentional COM interop patterns

---------

Co-authored-by: Stefan Broenner <stefan.broenner@microsoft.comm>
2025-12-25 11:26:30 +01:00
..
ExcelMcp.CLI.Tests
perf: replace CreateUniqueTestFile with fixture-based file creation (#266)
2025-12-04 09:06:26 +00:00
ExcelMcp.ComInterop.Tests
fix: resolve code scanning alerts for generic catch clauses (#319)
2025-12-16 19:12:48 +00:00
ExcelMcp.Core.Tests
refactor: eliminate dead code and cargo cult patterns (-1,286 lines) (#334)
2025-12-25 11:26:30 +01:00
ExcelMcp.McpServer.Tests
feat: Re-add Rename operations + session timeout fix (#330)
2025-12-20 16:32:19 +01:00
Directory.Build.props
refactor: simplify telemetry - always enabled, build-time connection string (#246)
2025-12-01 13:34:39 +00:00
README.md
fix: CodeQL useless-assignment violation in ExcelRangeTool.SetStyleAsync + scripts cleanup (#120)
2025-11-04 06:33:59 +00:00

README.md

ExcelMcp Tests

⚠️ No Traditional Unit Tests: ExcelMcp has no unit tests. Integration tests ARE our unit tests because Excel COM cannot be meaningfully mocked. See docs/ADR-001-NO-UNIT-TESTS.md for full architectural rationale.

Quick Start

# Development (fast feedback - excludes VBA tests)
dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA&Feature!=VBATrust"

# Pre-commit (comprehensive - excludes VBA tests)
dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA&Feature!=VBATrust"

# Session/batch changes (MANDATORY when modifying session/batch code)
dotnet test --filter "RunType=OnDemand"

# VBA tests (manual only - requires VBA trust enabled)
dotnet test --filter "(Feature=VBA|Feature=VBATrust)&RunType!=OnDemand"

Documentation

For complete testing guidance, see:

Test Architecture

tests/
├── ExcelMcp.Core.Tests/      # Core business logic (Unit + Integration)
├── ExcelMcp.McpServer.Tests/ # MCP protocol layer (Unit + Integration)
├── ExcelMcp.CLI.Tests/        # CLI wrapper (Unit + Integration)
└── ExcelMcp.ComInterop.Tests/ # COM utilities (Unit + OnDemand)

Test Categories

Category Speed Requirements Run By Default
Unit Fast (2-5 sec) None ✅ Yes (CI/CD)
Integration Medium (10-20 min) Excel + Windows ✅ Yes (local)
OnDemand Slow (3-5 min) Excel + Windows ❌ No (explicit only)

Feature-Specific Tests

# Test specific feature only
dotnet test --filter "Feature=PowerQuery&RunType!=OnDemand"
dotnet test --filter "Feature=DataModel&RunType!=OnDemand"
dotnet test --filter "Feature=Tables&RunType!=OnDemand"
dotnet test --filter "Feature=PivotTables&RunType!=OnDemand"
dotnet test --filter "Feature=Ranges&RunType!=OnDemand"
dotnet test --filter "Feature=Connections&RunType!=OnDemand"

When to Run Which Tests

Scenario Command
Daily development dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA"
Before commit dotnet test --filter "Category=Integration&RunType!=OnDemand&Feature!=VBA"
Modified session/batch code dotnet test --filter "RunType=OnDemand" (see Rule 3)
VBA development dotnet test --filter "(Feature=VBA|Feature=VBATrust)&RunType!=OnDemand"

VBA Testing

Why VBA Tests Are Excluded by Default

VBA tests are excluded from normal test runs because:

  1. Stable codebase - VBA features are mature with minimal changes
  2. Performance - Excluding VBA tests makes integration tests ~25% faster (10-15 min vs 15-20 min)
  3. Special requirements - VBA tests require VBA trust enabled in Excel settings
  4. Opt-in model - Explicit testing when VBA code changes, rather than every commit

When to Run VBA Tests

Run VBA tests manually when:

  • Modifying VBA-related code (ScriptCommands, VbaTrustDetection)
  • Adding new VBA features
  • Before releasing VBA-related changes
  • Troubleshooting VBA-specific issues

How to Run VBA Tests

# Run ONLY VBA tests
dotnet test --filter "(Feature=VBA|Feature=VBATrust)&RunType!=OnDemand"

# Run ALL tests including VBA (takes longer)
dotnet test --filter "Category=Integration&RunType!=OnDemand"

VBA Test Files

All VBA tests are tagged with [Trait("Feature", "VBA")] or [Trait("Feature", "VBATrust")]:

tests/ExcelMcp.Core.Tests/Integration/Commands/Script/
  - ScriptCommandsTests.cs
  - ScriptCommandsTests.Lifecycle.cs
  - VbaTrustDetectionTests.ScriptCommands.cs
  - VbaTrustDetectionTests.cs

tests/ExcelMcp.CLI.Tests/Integration/Commands/
  - ScriptAndSetupCommandsTests.cs

VBA Trust Setup

VBA tests require VBA trust enabled in Excel:

# Enable VBA trust (required for VBA tests)
Set-ItemProperty -Path "HKCU:\Software\Microsoft\Office\16.0\Excel\Security" -Name "AccessVBOM" -Value 1

# Verify setting
Get-ItemProperty -Path "HKCU:\Software\Microsoft\Office\16.0\Excel\Security" -Name "AccessVBOM"

Security Note: Only enable VBA trust in development environments. Production systems should keep this disabled.

Key Principles

  • ✅ File Isolation - Each test creates unique file (no sharing)
  • ✅ Binary Assertions - Pass OR fail, never "accept both"
  • ✅ Verify Excel State - Always verify actual Excel state after operations
  • ❌ No SaveAsync - Unless testing persistence (see Rule 14)

Getting Help

  • Test failures: Check test output for detailed error messages
  • Excel issues: Ensure Excel 2016+ installed and activated
  • Session/batch issues: Run OnDemand tests to verify cleanup
  • Writing tests: See Testing Strategy
Reference in New Issue View Git Blame Copy Permalink