Test controller as nuget package #7314

vladimir-shcherbakov · 2018-09-20T16:47:41Z

Description

Change TestController implementation.
Issue #6180

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review
- the cmdlet markdown files were generated using the platyPS module

MiYanni · 2018-09-28T22:08:21Z

src/ResourceManager/Resources/Commands.Resources.Test/packages.config

  <package id="Microsoft.WindowsAzure.Management" version="4.1.1" targetFramework="net45" />
  <package id="Moq" version="4.2.1510.2205" targetFramework="net45" />
-  <package id="Newtonsoft.Json" version="6.0.8" targetFramework="net45" />
+  <package id="Newtonsoft.Json" version="9.0.1" targetFramework="net452" />


I'm not sure these packages.config changes actually affect anything. These packages should be loaded from restoring Profile.

MiYanni · 2018-09-28T22:08:52Z

src/ResourceManager/Resources/Commands.Resources/packages.config

@@ -3,4 +3,5 @@
  <package id="Microsoft.Azure.Management.Authorization" version="2.8.0-preview" targetFramework="net452" />
  <package id="Microsoft.Azure.Management.ManagementGroups" version="1.1.0-preview" targetFramework="net452" />
  <package id="Microsoft.Azure.Management.ResourceManager" version="1.9.0-preview" targetFramework="net452" />
+  <package id="Microsoft.Azure.PowerShell.TestFx" version="1.0.137-preview" targetFramework="net452" />


This should be added to the Profile packages.config.

MiYanni · 2018-09-28T22:11:00Z

src/ResourceManager/Storage/Commands.Management.Storage.Test/packages.config

-  <package id="Newtonsoft.Json" version="6.0.8" targetFramework="net45" />
+  <package id="Microsoft.Rest.ClientRuntime.Azure.Authentication" version="2.3.0" targetFramework="net452" />
+  <package id="Microsoft.Rest.ClientRuntime.Azure.TestFramework" version="1.7.2" targetFramework="net452" />
+  <package id="Newtonsoft.Json" version="9.0.1" targetFramework="net452" />


Same here with packages.config changes should be in Profile.

MiYanni · 2018-09-28T22:11:06Z

src/ResourceManager/Storage/Commands.Management.Storage/packages.config

@@ -2,6 +2,7 @@
 <packages>
  <package id="Microsoft.Azure.KeyVault.Core" version="1.0.0" targetFramework="net452" />
  <package id="Microsoft.Azure.Management.Storage" version="8.1.0-preview" targetFramework="net452" />
+  <package id="Microsoft.Azure.PowerShell.TestFx" version="1.0.137-preview" targetFramework="net452" />


Same here with packages.config changes should be in Profile.

MiYanni · 2018-09-28T22:12:00Z

src/ResourceManager/Resources/Commands.Resources.Test/ScenarioTests/ActiveDirectoryTests.cs

@@ -88,7 +87,8 @@ public void TestGetADGroupWithSearchString()
        [Trait(Category.AcceptanceType, Category.CheckIn)]
        public void TestGetADGroupWithBadSearchString()
        {
-            ResourcesController.NewInstance.RunPsTest(interceptor, "Test-GetADGroupWithBadSearchString");
+            //ResourcesController.NewInstance.RunPsTest("Test-GetADGroupWithBadSearchString");


Commented code.

markcowl

See changes to common code first. Also, what are the changes to the developer guide on adding a new test project that will come with this change?

markcowl · 2018-10-08T16:33:49Z

tools/Common.Dependencies.Test.targets

@@ -64,6 +64,10 @@
      <HintPath>$(LibraryNugetPackageFolder)\Microsoft.Rest.ClientRuntime.Azure.TestFramework.1.7.2\lib\net452\Microsoft.Rest.ClientRuntime.Azure.TestFramework.dll</HintPath>
      <Private>True</Private>
    </Reference>
+    <Reference Include="Microsoft.Azure.PowerShell.TestFx, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>$(LibraryNugetPackageFolder)\Microsoft.Azure.PowerShell.TestFx.1.0.137-preview\lib\net452\Microsoft.Azure.PowerShell.TestFx.dll</HintPath>


Can we make this one of the published packages from the common repo?

vladimir-shcherbakov · 2018-10-15T23:07:24Z

I'll update the guide as soon as the review has signed off to be sure there won't be any changes.

MiYanni

Can you remove the TestController.cs from each of the test projects you've updated? You'd have to delete them and remove the entry in the .Test.csproj for Profile/Resources/Storage.

vladimir-shcherbakov · 2018-10-30T17:29:40Z

I removed ProfileController as redundant, but 2 others are still in use: ResourcesController uses library calls to setup and cleanup an environment like this:

ADGroup newGroup = null;
var controllerAdmin = ResourcesController.NewInstance;

controllerAdmin.RunPsTestWorkflow(
	interceptor,
	// scriptBuilder
	() =>
	{
		newGroup = CreateNewAdGroup(controllerAdmin);
		return new[] { scriptMethod };
	},
	// cleanup
	() =>
	{
		DeleteAdGroup(controllerAdmin, newGroup);
	},
	MethodBase.GetCurrentMethod().ReflectedType?.ToString(),
	MethodBase.GetCurrentMethod().Name);
}

but this functionality is not supported in the new controller because the setup and cleanup should be performed as part of the test script.

and Storage TestController is still in use for blob containers tests because to get the new one work the tests need to be re-recorded but they use some experimental environment I don't have access to:
Get-ProviderLocation_Stage function returns the "eastus2(stage)" string as a location. And the location is used to create a storage account, like this:

$loc = Get-ProviderLocation_Stage;
New-AzureRmStorageAccount -ResourceGroupName $rgname -Name $stoname -Location $loc -Type $stotype -Kind $kind

MiYanni

LGTM

markcowl · 2018-11-30T21:11:50Z

@vladimir-shcherbakov Can you update this PR for the new released common libraries, pull the latest and fix merge conflicts, and clean up the commits?

cormacpayne · 2018-11-30T22:29:49Z

@vladimir-shcherbakov re-targeted the PR at the new default branch, master, so you will need to pull from that branch to resolve the merge conflicts rather than preview

cormacpayne · 2018-12-05T19:15:54Z

@vladimir-shcherbakov ping on the above comments

markcowl · 2018-12-17T17:52:18Z

src/ResourceManager/Storage/Commands.Management.Storage.Test/ScenarioTests/StorageTestRunner.cs

+                })
+                .WithExtraRmModules(helper => new[]
+                {
+#if !NETSTANDARD


We can remove this IFDEF, now

azuresdkci1x added the needs-review label Sep 20, 2018

vladimir-shcherbakov mentioned this pull request Sep 20, 2018

[Iss#6180] Change TestController implementation #6651

Closed

8 tasks

cormacpayne assigned markcowl Sep 20, 2018

maddieclayton added needs-revision and removed needs-review labels Sep 24, 2018

maddieclayton assigned vladimir-shcherbakov and unassigned markcowl Sep 24, 2018

vladimir-shcherbakov assigned markcowl and unassigned vladimir-shcherbakov Sep 24, 2018

vladimir-shcherbakov added needs-review and removed needs-revision labels Sep 24, 2018

MiYanni suggested changes Sep 28, 2018

View reviewed changes

markcowl requested changes Oct 8, 2018

View reviewed changes

markcowl removed their assignment Oct 8, 2018

markcowl added needs-revision and removed needs-review labels Oct 8, 2018

MiYanni assigned vladimir-shcherbakov Oct 11, 2018

vladimir-shcherbakov assigned MiYanni and unassigned vladimir-shcherbakov Oct 16, 2018

vladimir-shcherbakov added needs-review and removed needs-revision labels Oct 16, 2018

MiYanni suggested changes Oct 23, 2018

View reviewed changes

markcowl added needs-revision and removed needs-review labels Oct 24, 2018

markcowl assigned vladimir-shcherbakov and unassigned MiYanni Oct 24, 2018

vladimir-shcherbakov assigned MiYanni and unassigned vladimir-shcherbakov Oct 30, 2018

vladimir-shcherbakov added needs-review and removed needs-revision labels Oct 30, 2018

MiYanni previously approved these changes Oct 30, 2018

View reviewed changes

MiYanni assigned markcowl and unassigned MiYanni Oct 30, 2018

vladimir-shcherbakov dismissed MiYanni’s stale review via 0713901 October 30, 2018 21:24

markcowl added Merge Conflicts and removed needs-review labels Nov 30, 2018

markcowl assigned vladimir-shcherbakov and unassigned markcowl Nov 30, 2018

cormacpayne changed the base branch from preview to master November 30, 2018 22:29

New test controller for Accout, Storage and Resources

ee2192a

vladimir-shcherbakov force-pushed the test-controller branch from 1f6c27d to ee2192a Compare December 13, 2018 23:44

vladimir-shcherbakov added needs-review and removed Merge Conflicts labels Dec 14, 2018

vladimir-shcherbakov assigned markcowl and unassigned vladimir-shcherbakov Dec 14, 2018

markcowl approved these changes Dec 17, 2018

View reviewed changes

markcowl merged commit 893de03 into Azure:master Dec 17, 2018

Test controller as nuget package #7314

Test controller as nuget package #7314

Uh oh!

Conversation

vladimir-shcherbakov commented Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markcowl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vladimir-shcherbakov commented Oct 15, 2018

Uh oh!

MiYanni left a comment

Choose a reason for hiding this comment

Uh oh!

vladimir-shcherbakov commented Oct 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MiYanni left a comment

Choose a reason for hiding this comment

Uh oh!

markcowl commented Nov 30, 2018

Uh oh!

cormacpayne commented Nov 30, 2018

Uh oh!

cormacpayne commented Dec 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vladimir-shcherbakov commented Sep 20, 2018 •

edited

Loading

vladimir-shcherbakov commented Oct 30, 2018 •

edited

Loading