Add PSxUnit runner by chunqingchen · Pull Request #2574 · PowerShell/PowerShell

Chunqing Chen (chunqingchen) · 2016-11-01T01:02:59Z

We need a runner that runs across platforms and compatible with .net core
Some of the code are from dotnet-test-xunit as it supports .net core and is a xunit runner, I modify the file to compatible running PS tests and add classification

Microsoft Contribution License Agreements (msftclas) · 2016-11-01T01:03:04Z

Hi Chunqing Chen (@chunqingchen), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). 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.microsoft.com.

TTYL, MSBOT;

Dongbo Wang (daxian-dbw)

Hi Chunqing Chen (@chunqingchen), there are a few issues with your PR:

You need to sign the CLA agreement
Your first 2 commits show changes that are already merged: 0908547. And your third commit is a merge commit. Please rebase your working branch and push again.
Please fix the Travis CI failure.

Steve Lee (SteveL-MSFT) · 2016-11-01T20:30:42Z

Chunqing Chen (@chunqingchen) you need to join the Microsoft GitHub org

Chunqing Chen (chunqingchen) · 2016-12-02T22:03:34Z

Dongbo Wang (@daxian-dbw)
cla signed
redundant commits removed
all checks passed.

Aditya Patwardhan (adityapatwardhan) · 2016-12-13T19:23:58Z

assemblyFilename [](start = 50, length = 16)

Check for null or empty string.

Aditya Patwardhan (adityapatwardhan) · 2016-12-13T19:31:49Z

The same Path.Combine() is done twice. It should be something like:

var runnerJson = Path.Combine(pathWithAssemblyName, $"{assemblyName}.xunit.runner.json")

if(File.Exists(runnerJson)
{
return File.OpenRead(runnerJson);
}

Aditya Patwardhan (adityapatwardhan) · 2016-12-13T19:32:35Z

File.OpenRead [](start = 27, length = 13)

This can throw exceptions. We should catch and fail gracefully.

corrected. Although I don't try to catch most of the exceptions and want to throw them as they are

Aditya Patwardhan (adityapatwardhan) · 2016-12-13T19:34:36Z

Add ///

style comments for all public types and properties.

Aditya Patwardhan (adityapatwardhan) · 2016-12-13T19:35:55Z

//Console.ReadKey(); [](start = 12, length = 20)

Remove unnecessary commented lines.

Aditya Patwardhan (adityapatwardhan) · 2016-12-13T19:39:19Z

use named parameters.

Aditya Patwardhan (adityapatwardhan) · 2016-12-15T22:38:43Z

Maybe this can be moved before the block which checks for "Skip". So if the testcase does not match the filter then we dont need to check if it is skipped.

Aditya Patwardhan (adityapatwardhan) · 2016-12-15T22:43:55Z

(".\result.xml" [](start = 44, length = 16)

Result file name should not be hardcoded but be accepted as a parameter.

Aditya Patwardhan (adityapatwardhan) · 2016-12-15T22:50:59Z

Is this intentional?

Aditya Patwardhan (adityapatwardhan) · 2016-12-15T22:51:12Z

Is this intentional?

Uncommented.

Aditya Patwardhan (adityapatwardhan) · 2016-12-15T22:52:26Z

Mix use of tabs and spaces.

Aditya Patwardhan (adityapatwardhan) · 2016-12-15T23:53:09Z

Please break this into smaller functions. Some code might be reusable and it will become much more readable.

this is actually from the dotnet-test-xunit project so i would just leave it as is

Aditya Patwardhan (adityapatwardhan) · 2016-12-15T23:53:58Z

🕐

Chunqing Chen (chunqingchen) · 2017-01-04T11:01:09Z

Aditya Patwardhan (@adityapatwardhan) the comments are resolved.

Chunqing Chen (chunqingchen) · 2017-02-01T18:13:14Z

Jason Shirk (@lzybkr) Hi Jason, Aditya has approved the comments, can this be merged?

comment addressed

Jason Shirk (lzybkr) · 2017-02-13T23:58:28Z

Reassigned to Dongbo Wang (@daxian-dbw) - he'll want to look at this as part of moving to msbuild based dotnet cli.

Chunqing Chen (chunqingchen) · 2017-02-24T22:41:39Z

Dongbo Wang (@daxian-dbw) Hi Dongbo, Aditya has approved the comments, can this be merged?

Dongbo Wang (daxian-dbw) · 2017-02-24T23:18:46Z

Chunqing Chen (@chunqingchen) Sorry that I didn't get a chance to quickly go through this PR. Will do it.

Dongbo Wang (daxian-dbw) · 2017-02-25T00:40:58Z

Steve Lee (@SteveL-MSFT) This PR contains code from XUnit. I just want to double check, is there a concern about checking in third party source code to our repository?

Steve Lee (SteveL-MSFT) · 2017-02-25T00:42:51Z

Dongbo Wang (@daxian-dbw) yes, we need to adhere to their licensing

Steve Lee (SteveL-MSFT) · 2017-02-25T00:46:39Z

Chunqing Chen (@chunqingchen) it looks like we're just taking some of their code without modification? can we have this be a runtime dependency so that xunit is already available?

Dongbo Wang (daxian-dbw) · 2017-04-11T16:48:13Z

I'm closing this PR for following 2 reasons:

After moving to netcoreapp2.0, we will refactor our code to get rid of powershell assembly load context, and thus we won't need a special runner to host powershell.
We have moved to MSBuild, and this PR needs to be re-worked to be built with MSBuild.

Microsoft Contribution License Agreements (msftclas) added the cla-required label Nov 1, 2016

Dongbo Wang (daxian-dbw) previously requested changes Nov 1, 2016

View reviewed changes

PowerShellTeam added the Review - Needed label Nov 10, 2016

Hemant Mahawar (HemantMahawar) removed the Review-Needed label Dec 1, 2016

Chunqing Chen (chunqingchen) added cla-already-signed and removed cla-required labels Dec 2, 2016

Jason Shirk (lzybkr) requested a review from Aditya Patwardhan (adityapatwardhan) December 8, 2016 21:48

Jason Shirk (lzybkr) self-assigned this Dec 8, 2016

Aditya Patwardhan (adityapatwardhan) reviewed Dec 13, 2016

View reviewed changes

Dongbo Wang (daxian-dbw) mentioned this pull request Dec 13, 2016

Invoke-RestMethod cmdlet error if the response is neither JSON nor XML #2862

Merged

Chunqing Chen (chunqingchen) added 2 commits December 14, 2016 18:50

Add PSxUnit runner

0bca2d8

remove readme file

f2b4a33

Aditya Patwardhan (adityapatwardhan) reviewed Dec 15, 2016

View reviewed changes

Comment thread test/tools/PSxUnit/ConsoleRunnerLogger.cs Outdated

Aditya Patwardhan (adityapatwardhan) Dec 15, 2016

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional?

Aditya Patwardhan (adityapatwardhan) reviewed Dec 15, 2016

View reviewed changes

Chunqing Chen (chunqingchen) added 2 commits January 3, 2017 17:18

remove readme file

6dcb5d0

Merge branch 'master' of https://github.com/chunqingchen/PowerShell

64f3a54

Resolve code review for Add PsxUnit.

bfec1ad

Aditya Patwardhan (adityapatwardhan) approved these changes Jan 31, 2017

View reviewed changes

Jason Shirk (lzybkr) assigned Dongbo Wang (daxian-dbw) and unassigned Jason Shirk (lzybkr) Feb 13, 2017

Dongbo Wang (daxian-dbw) closed this Apr 11, 2017

Ilya (iSazonov) removed the Review - Needed The PR is being reviewed label Apr 20, 2017

Conversation

Chunqing Chen (chunqingchen) commented Nov 1, 2016

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Nov 1, 2016

Uh oh!

Dongbo Wang (daxian-dbw) left a comment

Choose a reason for hiding this comment

Uh oh!

Steve Lee (SteveL-MSFT) commented Nov 1, 2016

Uh oh!

Chunqing Chen (chunqingchen) commented Dec 2, 2016

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!

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!

Aditya Patwardhan (adityapatwardhan) commented Dec 15, 2016

Uh oh!

Chunqing Chen (chunqingchen) commented Jan 4, 2017

Uh oh!

Chunqing Chen (chunqingchen) commented Feb 1, 2017

Uh oh!

Jason Shirk (lzybkr) commented Feb 13, 2017

Uh oh!

Chunqing Chen (chunqingchen) commented Feb 24, 2017

Uh oh!

Dongbo Wang (daxian-dbw) commented Feb 24, 2017

Uh oh!

Dongbo Wang (daxian-dbw) commented Feb 25, 2017

Uh oh!

Steve Lee (SteveL-MSFT) commented Feb 25, 2017

Uh oh!

Steve Lee (SteveL-MSFT) commented Feb 25, 2017

Uh oh!

Dongbo Wang (daxian-dbw) commented Apr 11, 2017

Uh oh!

Reviewers

Uh oh!