Skip to content

Commit 57c413d

Browse files
alzimmermsftalzimmermsft
authored
Fix subscription ID tests when logged in with Azure CLI (#2391)
* Fix subscription ID tests when logged in with Azure CLI * Another round of design iteration
1 parent 368647a commit 57c413d

10 files changed

Lines changed: 90 additions & 48 deletions

File tree

‎core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Areas/Subscription/SubscriptionCommandTests.cs‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Microsoft.Mcp.Core.Helpers;
1212
using Microsoft.Mcp.Core.Models.Command;
1313
using Microsoft.Mcp.Core.Options;
14+
using Microsoft.Mcp.Tests.Helpers;
1415
using NSubstitute;
1516
using Xunit;
1617

@@ -42,7 +43,7 @@ public SubscriptionCommandTests()
4243
public void Validate_WithEnvironmentVariableOnly_PassesValidation()
4344
{
4445
// Arrange
45-
EnvironmentHelpers.SetAzureSubscriptionId("env-subs");
46+
TestEnvironment.SetAzureSubscriptionId("env-subs");
4647

4748
// Act
4849
var parseResult = _commandDefinition.Parse([]);
@@ -55,7 +56,8 @@ public void Validate_WithEnvironmentVariableOnly_PassesValidation()
5556
public async Task ExecuteAsync_WithEnvironmentVariableOnly_CallsServiceWithCorrectSubscription()
5657
{
5758
// Arrange
58-
EnvironmentHelpers.SetAzureSubscriptionId("env-subs");
59+
TestEnvironment.SetAzureSubscriptionId("env-subs");
60+
var subscription = CommandHelper.GetDefaultSubscription()!;
5961

6062
var expectedAccounts = new ResourceQueryResults<StorageAccountInfo>(
6163
[
@@ -65,7 +67,7 @@ public async Task ExecuteAsync_WithEnvironmentVariableOnly_CallsServiceWithCorre
6567

6668
_storageService.GetAccountDetails(
6769
Arg.Is<string?>(s => string.IsNullOrEmpty(s)),
68-
Arg.Is("env-subs"),
70+
Arg.Is(subscription),
6971
Arg.Any<string>(),
7072
Arg.Any<RetryPolicyOptions>(),
7173
Arg.Any<CancellationToken>())
@@ -82,7 +84,7 @@ public async Task ExecuteAsync_WithEnvironmentVariableOnly_CallsServiceWithCorre
8284
// Verify the service was called with the environment variable subscription
8385
_ = _storageService.Received(1).GetAccountDetails(
8486
Arg.Is<string?>(s => string.IsNullOrEmpty(s)),
85-
"env-subs",
87+
subscription,
8688
Arg.Any<string>(),
8789
Arg.Any<RetryPolicyOptions>(),
8890
Arg.Any<CancellationToken>());
@@ -92,7 +94,9 @@ public async Task ExecuteAsync_WithEnvironmentVariableOnly_CallsServiceWithCorre
9294
public async Task ExecuteAsync_WithBothOptionAndEnvironmentVariable_PrefersOption()
9395
{
9496
// Arrange
95-
EnvironmentHelpers.SetAzureSubscriptionId("env-subs");
97+
TestEnvironment.SetAzureSubscriptionId("env-subs");
98+
var ignoredSubscription = CommandHelper.GetDefaultSubscription()!;
99+
var expectedSubscription = "option-subs";
96100

97101
var expectedAccounts = new ResourceQueryResults<StorageAccountInfo>(
98102
[
@@ -102,13 +106,13 @@ public async Task ExecuteAsync_WithBothOptionAndEnvironmentVariable_PrefersOptio
102106

103107
_storageService.GetAccountDetails(
104108
Arg.Is<string?>(s => string.IsNullOrEmpty(s)),
105-
Arg.Is("option-subs"),
109+
Arg.Is(expectedSubscription),
106110
Arg.Any<string>(),
107111
Arg.Any<RetryPolicyOptions>(),
108112
Arg.Any<CancellationToken>())
109113
.Returns(Task.FromResult(expectedAccounts));
110114

111-
var parseResult = _commandDefinition.Parse(["--subscription", "option-subs"]);
115+
var parseResult = _commandDefinition.Parse(["--subscription", expectedSubscription]);
112116

113117
// Act
114118
var response = await _command.ExecuteAsync(_context, parseResult, TestContext.Current.CancellationToken);
@@ -119,13 +123,13 @@ public async Task ExecuteAsync_WithBothOptionAndEnvironmentVariable_PrefersOptio
119123
// Verify the service was called with the option subscription, not the environment variable
120124
_ = _storageService.Received(1).GetAccountDetails(
121125
Arg.Is<string?>(s => string.IsNullOrEmpty(s)),
122-
"option-subs",
126+
expectedSubscription,
123127
Arg.Any<string>(),
124128
Arg.Any<RetryPolicyOptions>(),
125129
Arg.Any<CancellationToken>());
126130
_ = _storageService.DidNotReceive().GetAccountDetails(
127131
Arg.Is<string?>(s => string.IsNullOrEmpty(s)),
128-
"env-subs",
132+
ignoredSubscription,
129133
Arg.Any<string>(),
130134
Arg.Any<RetryPolicyOptions>(),
131135
Arg.Any<CancellationToken>());

‎core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Helpers/CommandHelperTests.cs‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Azure.Mcp.Core.Areas.Group.Commands;
33
using Microsoft.Extensions.Logging;
44
using Microsoft.Mcp.Core.Helpers;
5+
using Microsoft.Mcp.Tests.Helpers;
56
using NSubstitute;
67
using Xunit;
78

@@ -13,35 +14,37 @@ public class CommandHelperTests
1314
public void GetSubscription_EmptySubscriptionParameter_ReturnsEnvironmentValue()
1415
{
1516
// Arrange
16-
EnvironmentHelpers.SetAzureSubscriptionId("env-subs");
17+
TestEnvironment.SetAzureSubscriptionId("env-subs");
18+
var subscription = CommandHelper.GetDefaultSubscription();
1719
var parseResult = GetParseResult(["--subscription", ""]);
1820

1921
// Act
2022
var actual = CommandHelper.GetSubscription(parseResult);
2123

2224
// Assert
23-
Assert.Equal("env-subs", actual);
25+
Assert.Equal(subscription, actual);
2426
}
2527

2628
[Fact]
2729
public void GetSubscription_MissingSubscriptionParameter_ReturnsEnvironmentValue()
2830
{
2931
// Arrange
30-
EnvironmentHelpers.SetAzureSubscriptionId("env-subs");
32+
TestEnvironment.SetAzureSubscriptionId("env-subs");
33+
var subscription = CommandHelper.GetDefaultSubscription();
3134
var parseResult = GetParseResult([]);
3235

3336
// Act
3437
var actual = CommandHelper.GetSubscription(parseResult);
3538

3639
// Assert
37-
Assert.Equal("env-subs", actual);
40+
Assert.Equal(subscription, actual);
3841
}
3942

4043
[Fact]
4144
public void GetSubscription_ValidSubscriptionParameter_ReturnsParameterValue()
4245
{
4346
// Arrange
44-
EnvironmentHelpers.SetAzureSubscriptionId("env-subs");
47+
TestEnvironment.SetAzureSubscriptionId("env-subs");
4548
var parseResult = GetParseResult(["--subscription", "param-subs"]);
4649

4750
// Act
@@ -55,54 +58,78 @@ public void GetSubscription_ValidSubscriptionParameter_ReturnsParameterValue()
5558
public void GetSubscription_ParameterValueContainingSubscription_ReturnsEnvironmentValue()
5659
{
5760
// Arrange
58-
EnvironmentHelpers.SetAzureSubscriptionId("env-subs");
61+
TestEnvironment.SetAzureSubscriptionId("env-subs");
62+
var subscription = CommandHelper.GetDefaultSubscription();
5963
var parseResult = GetParseResult(["--subscription", "Azure subscription 1"]);
6064

6165
// Act
6266
var actual = CommandHelper.GetSubscription(parseResult);
6367

6468
// Assert
65-
Assert.Equal("env-subs", actual);
69+
Assert.Equal(subscription, actual);
6670
}
6771

6872
[Fact]
6973
public void GetSubscription_ParameterValueContainingDefault_ReturnsEnvironmentValue()
7074
{
7175
// Arrange
72-
EnvironmentHelpers.SetAzureSubscriptionId("env-subs");
76+
TestEnvironment.SetAzureSubscriptionId("env-subs");
77+
var subscription = CommandHelper.GetDefaultSubscription();
7378
var parseResult = GetParseResult(["--subscription", "Some default name"]);
7479

7580
// Act
7681
var actual = CommandHelper.GetSubscription(parseResult);
7782

7883
// Assert
79-
Assert.Equal("env-subs", actual);
84+
Assert.Equal(subscription, actual);
8085
}
8186

8287
[Fact]
8388
public void GetSubscription_NoEnvironmentVariableParameterValueContainingDefault_ReturnsParameterValue()
8489
{
8590
// Arrange
91+
TestEnvironment.ClearAzureSubscriptionId();
92+
var subscription = CommandHelper.GetProfileSubscription();
8693
var parseResult = GetParseResult(["--subscription", "Some default name"]);
8794

8895
// Act
8996
var actual = CommandHelper.GetSubscription(parseResult);
9097

9198
// Assert
92-
Assert.Equal("Some default name", actual);
99+
// If-else this test as being logged in with Azure CLI cannot be mocked out at this time.
100+
// So, if the running environment is logged in the subscription will be defaulted to the Azure CLI subscription.
101+
if (subscription != null)
102+
{
103+
Assert.Equal(subscription, actual);
104+
}
105+
else
106+
{
107+
Assert.Equal("Some default name", actual);
108+
}
93109
}
94110

95111
[Fact]
96112
public void GetSubscription_NoEnvironmentVariableParameterValueContainingSubscription_ReturnsParameterValue()
97113
{
98114
// Arrange
115+
TestEnvironment.ClearAzureSubscriptionId();
116+
var subscription = CommandHelper.GetProfileSubscription();
99117
var parseResult = GetParseResult(["--subscription", "Azure subscription 1"]);
100118

101119
// Act
102120
var actual = CommandHelper.GetSubscription(parseResult);
103121

104122
// Assert
105-
Assert.Equal("Azure subscription 1", actual);
123+
// If-else this test as being logged in with Azure CLI cannot be mocked out at this time.
124+
// So, if the running environment is logged in the subscription will be defaulted to the Azure CLI subscription.
125+
if (subscription != null)
126+
{
127+
Assert.Equal(subscription, actual);
128+
}
129+
else
130+
{
131+
Assert.Equal("Azure subscription 1", actual);
132+
}
106133
}
107134

108135
private static ParseResult GetParseResult(params string[] args)

‎core/Microsoft.Mcp.Core/src/Helpers/CommandHelper.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public static bool HasSubscriptionAvailable(CommandResult commandResult)
5252
public static string? GetDefaultSubscription()
5353
{
5454
// Primary: Azure CLI profile (set via 'az account set') - cached to avoid repeated file I/O
55-
var profileDefault = s_profileDefault.Value;
55+
var profileDefault = GetProfileSubscription();
5656
if (!string.IsNullOrEmpty(profileDefault))
5757
{
5858
return profileDefault;
@@ -62,5 +62,7 @@ public static bool HasSubscriptionAvailable(CommandResult commandResult)
6262
return EnvironmentHelpers.GetAzureSubscriptionId();
6363
}
6464

65+
internal static string? GetProfileSubscription() => s_profileDefault.Value;
66+
6567
private static bool IsPlaceholder(string value) => value.Contains("subscription") || value.Contains("default");
6668
}

core/Microsoft.Mcp.Core/src/Helpers/EnvironmentVariableHelpers.cs renamed to core/Microsoft.Mcp.Core/src/Helpers/EnvironmentHelpers.cs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,38 +5,24 @@ namespace Microsoft.Mcp.Core.Helpers;
55

66
public static class EnvironmentHelpers
77
{
8-
private const string AzureSubscriptionIdEnvironmentVariable = "AZURE_SUBSCRIPTION_ID";
8+
internal const string AzureSubscriptionIdEnvironmentVariable = "AZURE_SUBSCRIPTION_ID";
99

1010
public static bool GetEnvironmentVariableAsBool(string envVarName)
11-
{
12-
return Environment.GetEnvironmentVariable(envVarName) switch
11+
=> Environment.GetEnvironmentVariable(envVarName) switch
1312
{
1413
"true" => true,
1514
"True" => true,
1615
"T" => true,
1716
"1" => true,
1817
_ => false
1918
};
20-
}
2119

2220
/// <summary>
2321
/// Gets the Azure subscription ID from the AZURE_SUBSCRIPTION_ID environment variable.
2422
/// </summary>
2523
/// <returns>The subscription ID if available, null otherwise.</returns>
2624
public static string? GetAzureSubscriptionId()
27-
{
28-
return Environment.GetEnvironmentVariable(AzureSubscriptionIdEnvironmentVariable);
29-
}
30-
31-
/// <summary>
32-
/// Sets the AZURE_SUBSCRIPTION_ID environment variable.
33-
/// This method is primarily intended for testing scenarios.
34-
/// </summary>
35-
/// <param name="subscriptionId">The subscription ID to set, or null to clear the variable.</param>
36-
public static void SetAzureSubscriptionId(string? subscriptionId)
37-
{
38-
Environment.SetEnvironmentVariable(AzureSubscriptionIdEnvironmentVariable, subscriptionId);
39-
}
25+
=> Environment.GetEnvironmentVariable(AzureSubscriptionIdEnvironmentVariable);
4026

4127
public static bool IsPlaybackTesting()
4228
{

‎core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Tests/Helpers/TestEnvironment.cs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using Microsoft.Mcp.Core.Helpers;
5+
46
namespace Microsoft.Mcp.Tests.Helpers;
57

68
/// <summary>
@@ -16,5 +18,20 @@ public enum TestMode
1618
public static class TestEnvironment
1719
{
1820
public static bool IsRunningInCi =>
19-
string.Equals(Environment.GetEnvironmentVariable("CI"), "true", StringComparison.OrdinalIgnoreCase) || !string.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TF_BUILD"));
21+
string.Equals(Environment.GetEnvironmentVariable("CI"), "true", StringComparison.OrdinalIgnoreCase) ||
22+
!string.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TF_BUILD"));
23+
24+
/// <summary>
25+
/// Sets the AZURE_SUBSCRIPTION_ID environment variable.
26+
/// </summary>
27+
/// <param name="subscriptionId">The subscription ID to set, or null to clear the variable.</param>
28+
/// <returns>Either the AZURE_SUBSCRIPTION_ID environment variable value that was set or the logged into Azure CLI subscription.</returns>
29+
public static void SetAzureSubscriptionId(string subscriptionId)
30+
=> Environment.SetEnvironmentVariable(EnvironmentHelpers.AzureSubscriptionIdEnvironmentVariable, subscriptionId);
31+
32+
/// <summary>
33+
/// Clears the AZURE_SUBSCRIPTION_ID environment variable.
34+
/// </summary>
35+
public static void ClearAzureSubscriptionId()
36+
=> Environment.SetEnvironmentVariable(EnvironmentHelpers.AzureSubscriptionIdEnvironmentVariable, null);
2037
}

‎tools/Azure.Mcp.Tools.Acr/tests/Azure.Mcp.Tools.Acr.UnitTests/Registry/RegistryListCommandTests.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Microsoft.Mcp.Core.Helpers;
1515
using Microsoft.Mcp.Core.Models.Command;
1616
using Microsoft.Mcp.Core.Options;
17+
using Microsoft.Mcp.Tests.Helpers;
1718
using NSubstitute;
1819
using NSubstitute.ExceptionExtensions;
1920
using Xunit;
@@ -54,7 +55,7 @@ public void Constructor_InitializesCommandCorrectly()
5455
public async Task ExecuteAsync_ValidatesInputCorrectly(string args, bool shouldSucceed)
5556
{
5657
// Ensure environment variable fallback does not interfere with validation tests
57-
EnvironmentHelpers.SetAzureSubscriptionId(null);
58+
TestEnvironment.ClearAzureSubscriptionId();
5859
// Arrange
5960
if (shouldSucceed)
6061
{

‎tools/Azure.Mcp.Tools.ContainerApps/tests/Azure.Mcp.Tools.ContainerApps.UnitTests/ContainerApp/ContainerAppListCommandTests.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Microsoft.Mcp.Core.Helpers;
1515
using Microsoft.Mcp.Core.Models.Command;
1616
using Microsoft.Mcp.Core.Options;
17+
using Microsoft.Mcp.Tests.Helpers;
1718
using NSubstitute;
1819
using NSubstitute.ExceptionExtensions;
1920
using Xunit;
@@ -53,11 +54,11 @@ public void Constructor_InitializesCommandCorrectly()
5354
[InlineData("", false)]
5455
public async Task ExecuteAsync_ValidatesInputCorrectly(string args, bool shouldSucceed)
5556
{
56-
var originalSubscriptionId = Environment.GetEnvironmentVariable("AZURE_SUBSCRIPTION_ID");
57+
var originalSubscriptionId = EnvironmentHelpers.GetAzureSubscriptionId();
5758
try
5859
{
5960
// Ensure environment variable fallback does not interfere with validation tests
60-
EnvironmentHelpers.SetAzureSubscriptionId(null);
61+
TestEnvironment.ClearAzureSubscriptionId();
6162
// Arrange
6263
if (shouldSucceed)
6364
{
@@ -87,7 +88,10 @@ public async Task ExecuteAsync_ValidatesInputCorrectly(string args, bool shouldS
8788
}
8889
finally
8990
{
90-
EnvironmentHelpers.SetAzureSubscriptionId(originalSubscriptionId);
91+
if (originalSubscriptionId != null)
92+
{
93+
TestEnvironment.SetAzureSubscriptionId(originalSubscriptionId);
94+
}
9195
}
9296
}
9397

‎tools/Azure.Mcp.Tools.KeyVault/tests/Azure.Mcp.Tools.KeyVault.UnitTests/Admin/AdminSettingsGetCommandTests.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using Microsoft.Mcp.Core.Helpers;
1414
using Microsoft.Mcp.Core.Models.Command;
1515
using Microsoft.Mcp.Core.Options;
16+
using Microsoft.Mcp.Tests.Helpers;
1617
using NSubstitute;
1718
using NSubstitute.ExceptionExtensions;
1819
using Xunit;
@@ -107,7 +108,7 @@ public async Task ExecuteAsync_ValidatesInputCorrectly(string args, bool shouldS
107108
if (args.Contains("--vault") && !args.Contains("--subscription") && shouldSucceed)
108109
{
109110
// Provide subscription via environment variable
110-
EnvironmentHelpers.SetAzureSubscriptionId(KnownSubscriptionId);
111+
TestEnvironment.SetAzureSubscriptionId(KnownSubscriptionId);
111112
}
112113
else if (!args.Contains("--subscription"))
113114
{

0 commit comments

Comments
 (0)