Added Get-AzureRMAutomationHybridRunbookWorkerGroup cmdlet #2325

robplank · 2016-05-25T05:30:20Z

No description provided.

azurecla · 2016-05-25T05:30:30Z

Hi @robplank, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.azure.com.

TTYL, AZPRBOT;

azuresdkci · 2016-05-25T05:35:05Z

Can one of the admins verify this patch?

markcowl · 2016-05-25T06:05:11Z

@robplank There are merge conflicts, please update

@jodoglevy Can someone from the automation team review this PR?

azurecla · 2016-05-25T13:08:02Z

@robplank, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, AZPRBOT;

Updates to Get-HybridRunbookWorkerGroup cmdlet more updates to the Get-hybridRunbookWorkerGroup cmdlet Final updates to Get-AzureRMAutomationHybridRunbookWorkerGroup fixed reference name

robplank · 2016-05-25T16:49:45Z

@markcowl @jodoglevy merge conflicts are resolved

safeermohammed · 2016-05-25T21:56:13Z

src/ResourceManager/Automation/Commands.Automation/Model/HybridRunbookWorkerGroup.cs

+            this.AutomationAccountName = accountName;
+            //this.ID = hybridRunbookWorkerGroup.Id;
+            this.Name = hybridRunbookWorkerGroup.Name;
+            //this.Credential = hybridRunbookWorkerGroup.Credential;


Remove if the commented lines are not needed

robplank · 2016-05-26T05:05:56Z

@felixcho-msft @safeermohammed Requested updates have been made

felixcho-work · 2016-05-26T09:11:44Z

...omation/Commands.Automation/Microsoft.Azure.Commands.ResourceManager.Automation.dll-help.xml

+          </maml:description>
+          <command:parameterValue required="true" variableLength="false">String</command:parameterValue>
+        </command:parameter>
+        <command:parameter required="false" variableLength="false" globbing="false" pipelineInput="false" position="named">


InformationAction and InformationVariable are common parameters in PowerShell 5. No need to document.

felixcho-work · 2016-05-26T09:30:32Z

@robplank I may not be reading the latest commits properly, but should I be seeing diff's where the merge conflicts are resolved?

robplank · 2016-05-26T12:04:11Z

I don't see any merge conflicts, please let me know if you are sure there is a conflict that still exists after my update to the pull request yesterday.

robplank · 2016-05-26T13:39:00Z

@felixcho-msft help file has been updated, let me know if you see anything else that needs to be fixed

felixcho-work · 2016-05-26T15:42:25Z

src/ResourceManager/Automation/Commands.Automation/Common/AutomationClient.cs

@@ -42,6 +42,15 @@
 using Runbook = Microsoft.Azure.Commands.Automation.Model.Runbook;
 using Schedule = Microsoft.Azure.Commands.Automation.Model.Schedule;
 using Variable = Microsoft.Azure.Commands.Automation.Model.Variable;
+<<<<<<< 5a3f20949198c648832d7ae2bed4ba88898a4a3d


@robplank Re: conflicts

I still see these in AutomationClient.cs:
<<<<<<< 5a3f20949198c648832d7ae2bed4ba88898a4a3d
I think I see it in 2 places?

robplank · 2016-05-26T16:30:07Z

@felixcho-msft I removed those two lines.

hovsepm · 2016-05-26T20:31:53Z

@azuresdkci add to whitelist

jodoglevy · 2016-05-26T21:06:58Z

please don't merge yet, I will review today

felixcho-work · 2016-05-26T21:11:41Z

Don’t worry. We missed the cut-off date. They will merge for next release.

Felix

From: Joe Levy [mailto:[email protected]]
Sent: Thursday, May 26, 2016 2:07 PM
To: Azure/azure-powershell [email protected]
Cc: Felix Cho [email protected]; Mention [email protected]
Subject: Re: [Azure/azure-powershell] Added Get-AzureRMAutomationHybridRunbookWorkerGroup cmdlet (#2325)

please don't merge yet, I will review today

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHubhttps://github.com//pull/2325#issuecomment-221994889

jodoglevy · 2016-05-26T23:15:30Z

...ResourceManager/Automation/Commands.Automation/Cmdlet/GetAzureAutomationHybridWorkerGroup.cs

+    [OutputType(typeof(HybridRunbookWorkerGroup))]
+    public class GetAzureAutomationHybridWorkerGroup : AzureAutomationBaseCmdlet
+    {
+        [Parameter(ParameterSetName = AutomationCmdletParameterSets.ByName,Position = 2,  Mandatory = false, ValueFromPipeline = true, HelpMessage = "The Hybrid Runbook Worker Group Name")]


name should be lowercase in the help message

markcowl · 2016-06-13T22:04:52Z

@robplank @felixcho-msft @jodoglevy Not to throw a wrench in the works, but I don't see any tests for the new cmdlet. Are we planning to add tests? It's OK if we add them separately, later in the sprint, I just want to know what the plan is for testing the new cmdlet.

markcowl · 2016-06-15T20:23:14Z

@robplank @felixcho-msft @jodoglevy ping? Not knowing the testing strategy here will prevent us from merging this PR.

robplank · 2016-06-17T03:46:32Z

@markcowl do you just need a unit test added? I don't see how the test are called

robplank · 2016-06-17T04:30:43Z

@markcowl unit test created, let me know if I am missing anything else

markcowl · 2016-06-17T19:36:02Z

src/ResourceManager/Automation/Commands.Automation.Test/Commands.Automation.Test.csproj

@@ -102,7 +102,9 @@
      <SpecificVersion>False</SpecificVersion>
      <HintPath>..\..\..\packages\Microsoft.Bcl.Async.1.0.168\lib\net40\Microsoft.Threading.Tasks.Extensions.Desktop.dll</HintPath>
    </Reference>
-    <Reference Include="Microsoft.VisualStudio.QualityTools.UnitTestFramework, Version=10.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
+    <Reference Include="Microsoft.VisualStudio.QualityTools.UnitTestFramework, Version=10.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">


@robplank We no longer accept MSTest tests - please convert this to xunit, and you will be good. Mainly this is just re-attributing the methods, as the model for tests is largely the same

markcowl · 2016-06-17T19:44:34Z

@robplank Thanks for going above and beyond with this contribution. I think the feature team may want to add an end-to-end test, but I will not gate on that. I hate tpo ask for anything additional, but we have stopped allowing new MSTest tests, as there is significant overhead in running them, and some compatibility concenrs. If you convert this to xunit, and mark these as Check-in tests

[Fact]
[Trait(Category.AcceptanceType, Category.CheckIn)]

we will be able to merge.

robplank · 2016-06-17T20:21:57Z

@markcowl let me know if the changes are not correct.

markcowl · 2016-06-20T18:03:16Z

...ger/Automation/Commands.Automation.Test/UnitTests/GetAzureAutomationHybridWorkerGroupTest.cs

@@ -19,7 +21,8 @@ public class GetAzureAutomationHybridWorkerGroupTest : RMTestBase

        private GetAzureAutomationHybridWorkerGroup cmdlet;

-        [TestInitialize]
+        [Fact]


@robplank Not quite. This depends on test execution order, which may vary from test run to test run. There are two options for test setup in xunit:

Option 1: setup that runs at the start of each test in the class - in this case, just use the constructor, the class constructor for the test class automatically runs at the start of each test. There is no need for adding any attributes in this case, if this is what you want, removing the attributes from the constructor will do the appropriate thing here.

Option 2: setup that runs only once before any tests in the class are executed. For this, use the IClassFixtiure pattern. You can find a good example of this here:

azure-powershell/src/ResourceManager/KeyVault/Commands.KeyVault.Test/ScenarioTests/KeyVaultManagementTests.cs

Line 31 in 83203f0

public KeyVaultManagementTests(KeyVaultTestFixture fixture)

The mechanisms for test seup are described in detail here: https://xunit.github.io/docs/shared-context.html if you are interested.

robplank · 2016-06-21T21:50:39Z

@markcowl please check that latest commit

markcowl · 2016-06-23T16:13:44Z

@robplank. I opened a PR against your branch that actually enables these tests as part of check-in, can you take a look - it would automatically be added to this PR if you merge

robplank · 2016-06-23T17:56:19Z

Thanks @markcowl , the PR has been merged

markcowl · 2016-06-23T20:46:43Z

@robplank It looks like merge conflicts have crept in, can you update the PR? Most likely this was caused by merging another PR that updated the test targets, so just accepting the upstream changes should work.

robplank · 2016-06-27T13:38:57Z

@markcowl @felixcho-msft @safeermohammed if the build error is something I need to fix please let me know what the error is.

markcowl · 2016-06-27T16:01:02Z

@azuresdkci retest this please

markcowl · 2016-06-27T16:01:53Z

@robplank it is not something you need to worry about. It is one of four flaky tests in the check-in tests

jodoglevy · 2016-06-28T18:49:51Z

@robplank thanks for the contribution!

@markcowl thanks for all the help on this.

safeermohammed · 2016-06-28T18:53:12Z

@robplank thanks Rob for these new cmdlets

azurecla added the cla-required label May 25, 2016

azurecla added cla-signed and removed cla-required labels May 25, 2016

New cmdlet Get-AzureRMAutomationHybridRunbookWorkerGroup

8771de5

Updates to Get-HybridRunbookWorkerGroup cmdlet more updates to the Get-hybridRunbookWorkerGroup cmdlet Final updates to Get-AzureRMAutomationHybridRunbookWorkerGroup fixed reference name

safeermohammed reviewed May 25, 2016
View reviewed changes

updates requested by Microsoft

46f82b4

felixcho-work reviewed May 26, 2016
View reviewed changes

updated help for get-AzureRMAutomationHybridRunbookWorkerGroup

8836dfd

felixcho-work reviewed May 26, 2016
View reviewed changes

jodoglevy reviewed May 26, 2016
View reviewed changes

markcowl reviewed Jun 17, 2016
View reviewed changes

markcowl reviewed Jun 20, 2016
View reviewed changes

robplank and others added 8 commits June 26, 2016 19:00

setting VS version back to 2013

610aedf

removed lines added from a git action

43b3e98

fixing merge conflicts

94819e3

fix formating

0739af6

fixed typo

fc349b2

add Unit Test

e540a57

update Test

521df62

update unit test

f73250c

markcowl merged commit b9d401d into Azure:dev Jun 28, 2016

robplank mentioned this pull request Jul 2, 2016

Add support for Hybrid workers robplank/azure-automation-scsm-connector#44

Open

Added Get-AzureRMAutomationHybridRunbookWorkerGroup cmdlet #2325

Added Get-AzureRMAutomationHybridRunbookWorkerGroup cmdlet #2325

Uh oh!

Conversation

robplank commented May 25, 2016

Uh oh!

azurecla commented May 25, 2016

Uh oh!

azuresdkci commented May 25, 2016

Uh oh!

markcowl commented May 25, 2016

Uh oh!

azurecla commented May 25, 2016

Uh oh!

robplank commented May 25, 2016

Uh oh!

safeermohammed May 25, 2016

Choose a reason for hiding this comment

Uh oh!

robplank commented May 26, 2016

Uh oh!

felixcho-work May 26, 2016

Choose a reason for hiding this comment

Uh oh!

felixcho-work commented May 26, 2016

Uh oh!

robplank commented May 26, 2016

Uh oh!

robplank commented May 26, 2016

Uh oh!

felixcho-work May 26, 2016

Choose a reason for hiding this comment

Uh oh!

robplank commented May 26, 2016

Uh oh!

hovsepm commented May 26, 2016

Uh oh!

jodoglevy commented May 26, 2016

Uh oh!

felixcho-work commented May 26, 2016

Uh oh!

jodoglevy May 26, 2016

Choose a reason for hiding this comment

Uh oh!

markcowl commented Jun 13, 2016

Uh oh!

markcowl commented Jun 15, 2016

Uh oh!

robplank commented Jun 17, 2016

Uh oh!

robplank commented Jun 17, 2016

Uh oh!

markcowl Jun 17, 2016

Choose a reason for hiding this comment

Uh oh!

markcowl commented Jun 17, 2016

Uh oh!

robplank commented Jun 17, 2016

Uh oh!

markcowl Jun 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robplank commented Jun 21, 2016

Uh oh!

markcowl commented Jun 23, 2016

Uh oh!

robplank commented Jun 23, 2016

Uh oh!

markcowl commented Jun 23, 2016

Uh oh!

robplank commented Jun 27, 2016

Uh oh!

markcowl commented Jun 27, 2016

Uh oh!

markcowl commented Jun 27, 2016

Uh oh!

jodoglevy commented Jun 28, 2016

Uh oh!

safeermohammed commented Jun 28, 2016

markcowl Jun 20, 2016 •

edited

Loading