Skip to content

Commit 3349423

Browse files
yugangw-msftyugangw-msft
committed
address review feedback
1 parent 507dd02 commit 3349423

File tree

4 files changed

+38
-19
lines changed

4 files changed

+38
-19
lines changed

‎src/ResourceManager/Profile/Commands.Profile.Test/AzureRMProfileTests.cs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@ private static RMProfileClient SetupTestEnvironment(List<string> tenants, params
4646
mock.MoqClients = true;
4747
AzureSession.ClientFactory = mock;
4848
var context = new AzureContext(new AzureSubscription()
49-
{
50-
Account = DefaultAccount,
51-
Environment = EnvironmentName.AzureCloud,
52-
Id = DefaultSubscription,
53-
Name = DefaultSubscriptionName
54-
},
49+
{
50+
Account = DefaultAccount,
51+
Environment = EnvironmentName.AzureCloud,
52+
Id = DefaultSubscription,
53+
Name = DefaultSubscriptionName
54+
},
5555
new AzureAccount() { Id = DefaultAccount, Type = AzureAccount.AccountType.User },
5656
AzureEnvironment.PublicEnvironments[EnvironmentName.AzureCloud],
5757
new AzureTenant() { Domain = DefaultDomain, Id = DefaultTenant });
@@ -69,7 +69,8 @@ public void MultipleTenantsAndSubscriptionsSucceed()
6969
var firstList = new List<string> { DefaultSubscription.ToString(), secondsubscriptionInTheFirstTenant};
7070
var secondList = new List<string> { Guid.NewGuid().ToString()};
7171
var thirdList = new List<string> { DefaultSubscription.ToString(), secondsubscriptionInTheFirstTenant };
72-
var client = SetupTestEnvironment(tenants, firstList, secondList, thirdList);
72+
var fourthList = new List<string> { DefaultSubscription.ToString(), secondsubscriptionInTheFirstTenant };
73+
var client = SetupTestEnvironment(tenants, firstList, secondList, thirdList, fourthList);
7374
var subResults = new List<AzureSubscription>(client.ListSubscriptions());
7475
Assert.Equal(3, subResults.Count);
7576
var tenantResults = client.ListTenants();
@@ -79,6 +80,10 @@ public void MultipleTenantsAndSubscriptionsSucceed()
7980
AzureSubscription subValue;
8081
Assert.True(client.TryGetSubscriptionById(DefaultTenant.ToString(), DefaultSubscription.ToString(), out subValue));
8182
Assert.Equal(DefaultSubscription.ToString(), subValue.Id.ToString());
83+
Assert.True(client.TryGetSubscriptionByName(DefaultTenant.ToString(),
84+
MockSubscriptionClientFactory.GetSubscriptionNameFromId(DefaultSubscription.ToString()),
85+
out subValue));
86+
Assert.Equal(DefaultSubscription.ToString(), subValue.Id.ToString());
8287
}
8388

8489
[Fact]
@@ -88,7 +93,8 @@ public void SingleTenantAndSubscriptionSucceeds()
8893
var tenants = new List<string> {DefaultTenant.ToString()};
8994
var firstList = new List<string> {DefaultSubscription.ToString()};
9095
var secondList = firstList;
91-
var client = SetupTestEnvironment(tenants, firstList, secondList);
96+
var thirdList = firstList;
97+
var client = SetupTestEnvironment(tenants, firstList, secondList, thirdList);
9298
var subResults = new List<AzureSubscription>(client.ListSubscriptions());
9399
Assert.Equal(1, subResults.Count);
94100
var tenantResults = client.ListTenants();
@@ -98,6 +104,10 @@ public void SingleTenantAndSubscriptionSucceeds()
98104
AzureSubscription subValue;
99105
Assert.True(client.TryGetSubscriptionById(DefaultTenant.ToString(), DefaultSubscription.ToString(), out subValue));
100106
Assert.Equal(DefaultSubscription.ToString(), subValue.Id.ToString());
107+
Assert.True(client.TryGetSubscriptionByName(DefaultTenant.ToString(),
108+
MockSubscriptionClientFactory.GetSubscriptionNameFromId(DefaultSubscription.ToString()),
109+
out subValue));
110+
Assert.Equal(DefaultSubscription.ToString(), subValue.Id.ToString());
101111
}
102112

103113
[Fact]
@@ -113,6 +123,7 @@ public void SubscriptionNotFoundDoesNotThrow()
113123
Assert.Equal(1, subResults.Count);
114124
AzureSubscription subValue;
115125
Assert.False(client.TryGetSubscriptionById(DefaultTenant.ToString(), DefaultSubscription.ToString(), out subValue));
126+
Assert.False(client.TryGetSubscriptionByName("random-tenant", "random-subscription", out subValue));
116127
}
117128

118129
[Fact]
@@ -132,10 +143,11 @@ public void NoSubscriptionsInListDoesNotThrow()
132143
{
133144
var tenants = new List<string> { DefaultTenant.ToString() };
134145
var subscriptions = new List<string> () ;
135-
var client = SetupTestEnvironment(tenants, subscriptions);
146+
var client = SetupTestEnvironment(tenants, subscriptions, subscriptions);
136147
Assert.Equal(0, client.ListSubscriptions().Count());
137148
AzureSubscription subValue;
138149
Assert.False(client.TryGetSubscriptionById(DefaultTenant.ToString(), DefaultSubscription.ToString(), out subValue));
150+
Assert.False(client.TryGetSubscriptionByName(DefaultTenant.ToString(), "random-name", out subValue));
139151
}
140152

141153
[Fact]

‎src/ResourceManager/Profile/Commands.Profile.Test/MockSubscriptionClientFactory.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ public MockSubscriptionClientFactory(List<string> tenants, Queue<List<string>> s
4444
}
4545
}
4646

47+
public static string GetSubscriptionNameFromId(string id)
48+
{
49+
return "Sub-" + id;
50+
}
51+
4752
public SubscriptionClient GetSubscriptionClient()
4853
{
4954
var tenantMock = new Mock<ITenantOperations>();
@@ -72,7 +77,7 @@ public SubscriptionClient GetSubscriptionClient()
7277
result.Subscription =
7378
new Subscription
7479
{
75-
DisplayName = "Returned Subscription",
80+
DisplayName = GetSubscriptionNameFromId(subId),
7681
Id = subId,
7782
State = "Active",
7883
SubscriptionId = subId
@@ -100,7 +105,7 @@ public SubscriptionClient GetSubscriptionClient()
100105
sub =>
101106
new Subscription
102107
{
103-
DisplayName = "Contoso Subscription",
108+
DisplayName = GetSubscriptionNameFromId(sub),
104109
Id = sub,
105110
State = "Active",
106111
SubscriptionId = sub

‎src/ResourceManager/Profile/Commands.Profile/Context/SetAzureRMContext.cs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,9 @@ protected override void ProcessRecord()
6767
}
6868

6969
var profileClient = new RMProfileClient(AzureRmProfileProvider.Instance.Profile);
70-
if (!string.IsNullOrWhiteSpace(SubscriptionId))
70+
if (!string.IsNullOrWhiteSpace(SubscriptionId) || !string.IsNullOrWhiteSpace(SubscriptionName))
7171
{
72-
profileClient.SetCurrentContext(SubscriptionId, null, TenantId);
73-
}
74-
else if (!string.IsNullOrWhiteSpace(SubscriptionName))
75-
{
76-
profileClient.SetCurrentContext(null, SubscriptionName, TenantId);
72+
profileClient.SetCurrentContext(SubscriptionId, SubscriptionName, TenantId);
7773
}
7874
else
7975
{

‎src/ResourceManager/Profile/Commands.Profile/Models/RMProfileClient.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,18 +137,24 @@ public AzureContext SetCurrentContext(string tenantId)
137137
public AzureContext SetCurrentContext(string subscriptionId, string subscriptionName, string tenantId)
138138
{
139139
AzureSubscription subscription;
140-
string subscriptionFilter = string.IsNullOrWhiteSpace(subscriptionId) ? subscriptionName : subscriptionId;
140+
141141
if (!string.IsNullOrWhiteSpace(subscriptionId))
142142
{
143143
TryGetSubscriptionById(tenantId, subscriptionId, out subscription);
144144
}
145-
else
145+
else if (!string.IsNullOrWhiteSpace(subscriptionName))
146146
{
147147
TryGetSubscriptionByName(tenantId, subscriptionName, out subscription);
148148
}
149+
else
150+
{
151+
throw new ArgumentException(string.Format(
152+
"Please provide either subscriptionId or subscriptionName"));
153+
}
149154

150155
if (subscription == null)
151156
{
157+
string subscriptionFilter = string.IsNullOrWhiteSpace(subscriptionId) ? subscriptionName : subscriptionId;
152158
throw new ArgumentException(string.Format(
153159
"Provided subscription {0} does not exist", subscriptionFilter));
154160
}

0 commit comments

Comments
 (0)