Clean unit tests by anannya03 · Pull Request #2567 · microsoft/mcp

anannya03 · 2026-05-03T03:30:42Z

What does this PR do?

Several unit tests across the MCP Tools codebase (e.g., UpdateWorkbooksCommandTests, UpdateAlertsCommandTests, etc.) currently assert that tool descriptions or summaries exactly match hard-coded strings. These tests frequently fail when a description is reworded or improved for clarity, even though the tool’s behavior remains correct.

Any test that asserts the exact implementation of the unit under test is just repeating the logic of the unit in another place and doesn't really test anything.

This cleans up the tests and asserts that each description is less than 1024 characters.

GitHub issue number?

#788

Pre-merge Checklist

Copilot

Pull request overview

This PR updates unit tests across MCP tools to avoid brittle assertions on exact tool/command description text, and instead validate descriptions using a maximum-length constraint (<= 1024 characters), aligning with issue #788.

Changes:

Replaced many Assert.Contains(...) checks on descriptions with Length <= 1024 assertions.
Removed or relaxed “description contains required info” assertions in several test suites.
Updated a core tool-loading unit test to stop matching the Azure tool description text.

Reviewed changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/PathListCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/OneLakeWorkspaceListCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/OneLakeItemListCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/OneLakeItemDataListCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/FileWriteCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/FileReadCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/DirectoryDeleteCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/DirectoryCreateCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.OneLake/tests/Commands/BlobListCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Fabric.Mcp.Tools.Core/tests/Commands/ItemCreateCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Workbooks/tests/Azure.Mcp.Tools.Workbooks.UnitTests/UpdateWorkbooksCommandTests.cs	Replaces description content checks with max-length; relaxes “required info” test.
tools/Azure.Mcp.Tools.Workbooks/tests/Azure.Mcp.Tools.Workbooks.UnitTests/ShowWorkbooksCommandTests.cs	Replaces description substring check with max-length.
tools/Azure.Mcp.Tools.Workbooks/tests/Azure.Mcp.Tools.Workbooks.UnitTests/ListWorkbooksCommandTests.cs	Replaces description substring checks with max-length; relaxes “required info” test.
tools/Azure.Mcp.Tools.Workbooks/tests/Azure.Mcp.Tools.Workbooks.UnitTests/CreateWorkbooksCommandTests.cs	Replaces description substring checks with max-length; relaxes “required info” test.
tools/Azure.Mcp.Tools.VirtualDesktop/tests/Azure.Mcp.Tools.VirtualDesktop.UnitTests/SessionHost/SessionHostUserSessionListCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.UnitTests/Server/ServerGetCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.UnitTests/Server/ServerCreateCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.UnitTests/FirewallRule/FirewallRuleDeleteCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.UnitTests/FirewallRule/FirewallRuleCreateCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.UnitTests/ElasticPool/ElasticPoolListCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.UnitTests/Database/DatabaseRenameCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.UnitTests/Database/DatabaseGetCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.UnitTests/Database/DatabaseCreateCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Speech/tests/Azure.Mcp.Tools.Speech.UnitTests/SpeechSetupTests.cs	Removes group/subgroup description substring checks.
tools/Azure.Mcp.Tools.Speech/tests/Azure.Mcp.Tools.Speech.UnitTests/Options/SpeechOptionDefinitionsTests.cs	Changes option description assertions (currently to an invalid symbol).
tools/Azure.Mcp.Tools.ServiceBus/tests/Azure.Mcp.Tools.ServiceBus.UnitTests/ServiceBusSetupTests.cs	Removes detailed description content checks (currently to an invalid symbol).
tools/Azure.Mcp.Tools.Pricing/tests/Azure.Mcp.Tools.Pricing.UnitTests/PricingGetCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Postgres/tests/Azure.Mcp.Tools.Postgres.UnitTests/PostgresListCommandTests.cs	Removes description assertions entirely.
tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/MySqlListCommandTests.cs	Removes description assertions entirely.
tools/Azure.Mcp.Tools.Monitor/tests/Azure.Mcp.Tools.Monitor.UnitTests/WebTests/WebTestsGetCommandTests.cs	Replaces description substring checks with max-length; relaxes “required info” test.
tools/Azure.Mcp.Tools.Monitor/tests/Azure.Mcp.Tools.Monitor.UnitTests/WebTests/WebTestsCreateOrUpdateCommandTests.cs	Replaces description substring checks with max-length; relaxes “required info” test.
tools/Azure.Mcp.Tools.Monitor/tests/Azure.Mcp.Tools.Monitor.UnitTests/Metrics/MetricsQueryCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Monitor/tests/Azure.Mcp.Tools.Monitor.UnitTests/Metrics/MetricsDefinitionsCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Grafana/tests/Azure.Mcp.Tools.Grafana.UnitTests/WorkspaceListCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.FoundryExtensions/tests/Azure.Mcp.Tools.FoundryExtensions.UnitTests/OpenAiChatCompletionsCreateCommandTests.cs	Replaces multiple description substring checks with non-null/non-empty + max-length.
tools/Azure.Mcp.Tools.EventHubs/tests/Azure.Mcp.Tools.EventHubs.UnitTests/Namespace/NamespaceUpdateCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.EventGrid/tests/Azure.Mcp.Tools.EventGrid.UnitTests/Events/EventsPublishCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Cosmos/tests/Azure.Mcp.Tools.Cosmos.UnitTests/CosmosListCommandTests.cs	Replaces multiple description substring checks with non-null/non-empty + max-length.
tools/Azure.Mcp.Tools.Compute/tests/Azure.Mcp.Tools.Compute.UnitTests/Disk/DiskUpdateCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Compute/tests/Azure.Mcp.Tools.Compute.UnitTests/Disk/DiskGetCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Compute/tests/Azure.Mcp.Tools.Compute.UnitTests/Disk/DiskDeleteCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.Compute/tests/Azure.Mcp.Tools.Compute.UnitTests/Disk/DiskCreateCommandTests.cs	Replaces description substring check with a max-length assertion.
tools/Azure.Mcp.Tools.CloudArchitect/tests/Azure.Mcp.Tools.CloudArchitect.UnitTests/Design/DesignCommandTests.cs	Removes many brittle description-content assertions; adds max-length.
tools/Azure.Mcp.Tools.AzureBestPractices/tests/Azure.Mcp.Tools.AzureBestPractices.UnitTests/AIAppBestPracticesCommandTests.cs	Removes description assertions entirely.
core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Areas/Server/Commands/ToolLoading/SingleProxyToolLoaderTests.cs	Removes brittle Azure tool description match; adds a max-length assertion (currently to an invalid symbol).

Comments suppressed due to low confidence (2)

tools/Azure.Mcp.Tools.Workbooks/tests/Azure.Mcp.Tools.Workbooks.UnitTests/UpdateWorkbooksCommandTests.cs:48

This test name says it validates required information, but it now only asserts Command.Description is non-null (which is already covered by the constructor test via CommandDefinition.Description). Consider either removing this test or updating it to assert a non-trivial invariant (e.g., not empty and/or <= 1024 chars) that isn’t already verified elsewhere.
tools/Azure.Mcp.Tools.Postgres/tests/Azure.Mcp.Tools.Postgres.UnitTests/PostgresListCommandTests.cs:290
This change removes the only assertions about Command.Description, so the test suite no longer verifies that the command has a non-empty description (or that it meets the <= 1024 char requirement described in the PR). Consider adding a stable check such as non-null/non-empty and/or length <= 1024 instead of dropping the description validation entirely.

    [Fact]
    public void Name_IsCorrect()
    {
        Assert.Equal("list", Command.Name);
    }

}

@@ -42,8 +42,6 @@ public void Description_ContainsRequiredInformation()
    {
        var description = Command.Description;
        Assert.NotNull(description);
-        Assert.Contains("workbook", description, StringComparison.OrdinalIgnoreCase);
-        Assert.Contains("subscription", description, StringComparison.OrdinalIgnoreCase);
    }



jongio

This PR has the right intent - removing brittle exact-string assertions is a good goal. However, CI is failing with 18 compile errors and there are a few structural issues to fix before this can move forward.

Build failures: CommandDefinition isn't available in every test class. It's a property on CommandUnitTestsBase<TCommand, TService>, but several modified tests don't inherit from that base:

ServiceBusSetupTests - standalone class testing serviceBusGroup.Description (a CommandGroup, not a command)
SpeechOptionDefinitionsTests - standalone class testing option.Description (an OptionDefinition, not a command)
SingleProxyToolLoaderTests - testing azureTool.Description from a tool listing result

These need different treatment since there's no CommandDefinition in scope.

Hollow test methods: Several Description_ContainsRequiredInformation methods are left with only Assert.NotNull(description) and no other assertion (e.g., in WebTestsCreateOrUpdateCommandTests, WebTestsGetCommandTests, CreateWorkbooksCommandTests, ListWorkbooksCommandTests, UpdateWorkbooksCommandTests). Either delete these methods entirely or add a meaningful assertion like the length check.

Semantic validation gap: The removed Assert.Contains checks weren't exact matches - they validated domain relevance (e.g., a Cosmos command description mentions "accounts"). Consider keeping a lightweight semantic check alongside the length check. Something like Assert.NotEmpty(Command.Description) plus one domain keyword is still useful and not brittle.

Suggested path forward:

Fix the compile errors by handling non-CommandUnitTestsBase tests differently
Delete empty test methods rather than leaving them as no-ops
Consider keeping one Assert.Contains per test for the primary domain noun (optional - up to the team)

srnagar · 2026-05-05T22:38:21Z

        Assert.Equal("create", Command.Name);
-        Assert.Contains("managed disk", Command.Description, StringComparison.OrdinalIgnoreCase);
        Assert.NotEqual(Guid.Empty.ToString(), Command.Id);
+        Assert.False(string.IsNullOrEmpty(Command.Description));


Same here and in a few other places - using Assert.NotEmpty will have better error message description when tests fail.

srnagar · 2026-05-05T22:41:37Z

-        Assert.Contains("Publish custom events to Event Grid topics", Command.Description);
        Assert.Equal("Publish Events to Event Grid Topic", Command.Title);
+        Assert.False(string.IsNullOrEmpty(CommandDefinition.Description));
+        Assert.Contains("Event Grid topics", Command.Description, StringComparison.OrdinalIgnoreCase);


Description may or may not use plural topics. So, this test can fail when the description gets updated.

jongio

Looks much better - the build failures are fixed and the test pattern is consistent now. Two things I noticed in this push:

PathListCommandTests.cs - the GetCommand_ReturnsValidCommand method lost Assert.NotEmpty(CommandDefinition.Options) when the description checks were added. That Options assertion was unrelated to description validation and shouldn't have been removed.
PricingGetCommand.cs - this is a production source change (shortening the command description). Not opposed to it, but it's scope creep for a PR titled "Clean unit tests." Consider splitting it out or updating the PR description to note the source change.

Otherwise the pattern of NotNull + NotEmpty + Length <= 1024 is clean and consistent across all files. CI is green.

anannya03 · 2026-05-13T23:19:20Z

Hi @jongio,

PricingGetCommand.cs - this is a production source change (shortening the command description). Not opposed to it, but it's scope creep for a PR titled "Clean unit tests." Consider splitting it out or updating the PR description to note the source change.

The length of the Pricing tool's description exceeded 1024 characters, so this PR also tackles that to ensure unit tests are passing. I have updated the PR description to include this change.

Copilot AI review requested due to automatic review settings May 3, 2026 03:30

github-project-automation Bot added this to Azure MCP Server May 3, 2026

anannya03 requested review from a team, JacksonWeber, JonathanCrd, achyuth-ms, anuchandy, conniey, ericshape, jeremyfrosti, jongio, kk-src, lzchen, matteing, mattkohnms, ramnov, srnagar, vladimisms and xiangyan99 as code owners May 3, 2026 03:30

github-project-automation Bot moved this to Untriaged in Azure MCP Server May 3, 2026

anannya03 requested review from XiaofuHuang, fanyang-mono, g2vinay, jairmyree, msalaman, rads-1996, rajkumar-rangaraj, weng5e and xiang17 as code owners May 3, 2026 03:30

github-actions Bot added tools-Grafana Azure Managed Grafana tools-Monitor tools-MySQL tools-Postgres PostgreSQL tools-Price tools-ServiceBus tools-Speech tools-SQL tools-VirtualDesktop Azure Virtual Desktop service tools-Workbooks labels May 3, 2026

Copilot started reviewing on behalf of anannya03 May 3, 2026 03:31 View session

Copilot AI reviewed May 3, 2026

View reviewed changes

anannya03 marked this pull request as draft May 3, 2026 05:31

jongio reviewed May 5, 2026

View reviewed changes

srnagar reviewed May 5, 2026

View reviewed changes

jongio reviewed May 13, 2026

View reviewed changes

anannya03 added 9 commits May 13, 2026 16:10

Clean tests- draft 1

e68d287

Clean unit tests

cac6ed7

Fix errors

6b5f371

null fix

f2ab092

more fixes

1046be2

Addres feedback - 1

a9b4caa

feedback - 1

e35f7ef

replaced isnullorempty with individual null/empty assert

f1b7492

unit test fix

dab02b2

anannya03 force-pushed the clean_unit_tests branch from 059e116 to dab02b2 Compare May 13, 2026 23:12

anannya03 marked this pull request as ready for review May 13, 2026 23:12

addresses feedback

bc1601a

fixed the pathlistcommandtests file

b2c80d5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean unit tests#2567

Clean unit tests#2567
anannya03 wants to merge 11 commits into
microsoft:mainfrom
anannya03:clean_unit_tests

anannya03 commented May 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jongio left a comment

Uh oh!

Uh oh!

Uh oh!

srnagar May 5, 2026

Uh oh!

srnagar May 5, 2026

Uh oh!

jongio left a comment

Uh oh!

anannya03 commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

anannya03 commented May 3, 2026

What does this PR do?

GitHub issue number?

Pre-merge Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jongio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

srnagar May 5, 2026

Choose a reason for hiding this comment

Uh oh!

srnagar May 5, 2026

Choose a reason for hiding this comment

Uh oh!

jongio left a comment

Choose a reason for hiding this comment

Uh oh!

anannya03 commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants