Tool to generate format.ps1xml #5666

vladimir-shcherbakov · 2018-03-02T23:40:12Z

Description

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

maddieclayton · 2018-03-05T19:05:37Z

@vladimir-shcherbakov the test that failed is one of the flaky tests, so pulling from upstream should stop this from failing.

markcowl

A few comm ents. This looks great. I have kicked off a signed build so I can try it out as well.

markcowl · 2018-03-10T00:55:14Z

src/Common/Commands.Common/Attrubutes/Ps1XmlAttribute.cs

@@ -0,0 +1,33 @@
+// Copyright Microsoft Corporation


Please correct the folder name to 'Attributes' - not sure if this already existed, but we should fix regardless

markcowl · 2018-03-10T00:55:49Z

...eManager/Common/Commands.Common.Authentication.ResourceManager/Models/PSAzureSubscription.cs

@@ -17,6 +17,7 @@
 using System;
 using System.Collections.Generic;
 using System.Management.Automation;
+using Microsoft.WindowsAzure.Commands.Common.Attrubutes;


Attrubutes -> Attributes

markcowl · 2018-03-10T01:01:59Z

tools/RepoTasks/RepoTasks.Cmdlets.Tests/AppDomainWorkerShould.cs

+            if (_domain != null) AppDomain.Unload(_domain);
+        }
+
+        [Fact]


Use

[Trait(Category.AcceptanceType, Category.CheckIn)] for each of these tests, this will make them run in the check-in build. Also, please add this path and assembly to the test mappings file:https://github.com/Azure/azure-powershell/blob/preview/TestMappings.json

@markcowl
The project that owns the Category class with the constants (AcceptanceType, CheckIn) is dependent on many others. Is it a good idea just to copy paste the requires constants instead of adding all the dependencies?

markcowl · 2018-03-10T01:03:09Z

tools/RepoTasks/RepoTasks.Cmdlets.Tests/NewFormatPs1XmlCommandShould.cs

+        private const string CmdletName = "New-FormatPs1Xml";
+        private const string ExpectedAssemblyName = "RepoTasks.CmdletsForTest";
+
+        [Fact]


Add Trait attribute as above

markcowl · 2018-03-10T01:09:19Z

tools/RepoTasks/RepoTasks.Cmdlets.Tests/NewFormatPs1XmlCommandShould.cs

+        {
+            var initialSessionState = InitialSessionState.CreateDefault();
+
+            initialSessionState.Commands.Add(


I am a little worried about polluting the environment and leaving commands arround. We have an IDataStore interface here, with implementations for MockDataStore (for testing) and DiskDataStore (for runtime). If you provide this at runtime (say through a static property, or the like) , then your tests could test this functionality without leaving files behind. Let me know if this is too time-consuming to take up right now, as we could simply take this as test debt and schedule it later.

@markcowl
What if I just delete all the test artifacts, for example, in a final block?

markcowl · 2018-03-10T01:32:00Z

tools/RepoTasks/RepoTasks.Cmdlets/RepoTasks.Cmdlets.csproj

+    <Reference Include="System" />
+    <Reference Include="System.Core" />
+    <Reference Include="System.Management.Automation, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\packages\System.Management.Automation.6.1.7601.17515\lib\net45\System.Management.Automation.dll</HintPath>


We shouldn't need a HintPath for this.

markcowl · 2018-03-10T01:32:42Z

tools/RepoTasks/RepoTasks.CmdletsForTest/RepoTasks.CmdletsForTest.csproj

+    <Reference Include="System" />
+    <Reference Include="System.Core" />
+    <Reference Include="System.Management.Automation, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\packages\System.Management.Automation.6.1.7601.17515\lib\net45\System.Management.Automation.dll</HintPath>


You should not need the hintpath

markcowl · 2018-03-10T01:40:32Z

tools/RepoTasks/RepoTasks.RemoteWorker/AppDomainWorker.cs

+
+            foreach (var cmdletType in cmdletTypList)
+            {
+                var attributes = cmdletType.GetCustomAttributes(


You might find these extensions useful: https://github.com/Azure/azure-powershell/blob/preview/tools/StaticAnalysis/HelpAnalyzer/ReflectionExtensions.cs

Not any required change, just FYI

markcowl · 2018-03-10T01:54:02Z

tools/RepoTasks/RepoTasks.Cmdlets/NewFormatPs1XmlCommand.cs

+            {
+                if (!Path.IsPathRooted(ModulePath))
+                {
+                    ModulePath = Path.GetFullPath(ModulePath);


To resolve the path in powershell, you should use the SessionState.Path property to get a path from the given provider, see: https://github.com/Azure/azure-powershell/blob/preview/src/ResourceManager/Websites/Commands.Websites/Strategies/GitCommand.cs#L45-L46 for an example.

markcowl · 2018-03-10T01:57:34Z

signign run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/245/

…ath.

…ure-powershell into iss#4516

markcowl · 2018-03-25T22:03:27Z

...eManager/Common/Commands.Common.Authentication.ResourceManager/Models/PSAzureSubscription.cs

-        /// Gets or sets subscription State
-        /// </summary>
+        /// <inheritdoc />
+        [Ps1Xml]


I think we only want this one to go in the table view

markcowl · 2018-03-25T22:20:28Z

on demand run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/655/

signing run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/293/

…ign job happy.

…ure-powershell into iss#4516

vladimir-shcherbakov · 2018-03-27T23:26:55Z

sign job: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/301/

vladimir-shcherbakov · 2018-03-28T03:56:44Z

on demand job: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/668/

Tool to generate format.ps1xml

b7ccd51

vladimir-shcherbakov mentioned this pull request Mar 2, 2018

#4516 New-FormatPs1Xml cmdlet. #5559

Closed

8 tasks

azuresdkci added the needs-review label Mar 3, 2018

maddieclayton assigned markcowl Mar 5, 2018

vladimir-shcherbakov requested a review from markcowl March 7, 2018 17:58

markcowl requested changes Mar 10, 2018

View reviewed changes

markcowl added needs-revision and removed needs-review labels Mar 10, 2018

markcowl assigned vladimir-shcherbakov and unassigned markcowl Mar 10, 2018

vladimir-shcherbakov and others added 7 commits March 16, 2018 16:55

PR comments fix.

b0d52bd

RepoTasks solution added to the build.proj

5354e8d

restore packages task added.

f3714b6

Path.Combine(Environment.CurrentDirectory, "Dummy.psd1") to get the p…

e6ad6cc

…ath.

Merge branch 'iss#4516' of https://github.com/vladimir-shcherbakov/az…

3ad32db

…ure-powershell into iss#4516

Merge remote-tracking branch 'upstream/preview' into iss#4516

3041cca

another way to get current directory

5723f76

vladimir-shcherbakov assigned markcowl and unassigned vladimir-shcherbakov Mar 20, 2018

markcowl previously approved these changes Mar 25, 2018

View reviewed changes

markcowl added Waiting for On Demand and removed needs-revision labels Mar 25, 2018

markcowl removed their assignment Mar 25, 2018

vladimir-shcherbakov added 3 commits March 27, 2018 09:53

Commands.Common.Authentication.Abstraction added to the sln to make s…

c040da8

…ign job happy.

Merge branch 'iss#4516' of https://github.com/vladimir-shcherbakov/az…

72dce06

…ure-powershell into iss#4516

Merge remote-tracking branch 'upstream/preview' into iss#4516

4c03d97

vladimir-shcherbakov dismissed markcowl’s stale review via 4c03d97 March 27, 2018 16:59

markcowl approved these changes Mar 30, 2018

View reviewed changes

markcowl merged commit ef88f7b into Azure:preview Mar 30, 2018

vladimir-shcherbakov mentioned this pull request Mar 30, 2018

Create tool to generate format.ps1xml file #4516

Closed

5 tasks

Tool to generate format.ps1xml #5666

Tool to generate format.ps1xml #5666

Uh oh!

Conversation

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

Uh oh!

Description

Checklist

Uh oh!

maddieclayton commented Mar 5, 2018

Uh oh!

markcowl left a comment

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

markcowl commented Mar 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markcowl commented Mar 25, 2018

Uh oh!

vladimir-shcherbakov commented Mar 27, 2018

Uh oh!

vladimir-shcherbakov commented Mar 28, 2018

Uh oh!

Uh oh!

vladimir-shcherbakov commented Mar 2, 2018 •

edited

Loading