Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Console.WriteLine for NetCore Projects #145

Merged
merged 14 commits into from
Apr 13, 2017

Conversation

jayaranigarg
Copy link
Member

No description provided.

@msftclas
Copy link

msftclas commented Apr 7, 2017

@jayaranigarg,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@jayaranigarg jayaranigarg requested review from AbhitejJohn and removed request for AbhitejJohn April 10, 2017 13:43
$projectPath = Locate-Item -relativePath $project

Write-Verbose "$msbuild /t:restore -verbosity:quiet $projectPath"
& $msbuild /t:restore -verbosity:quiet $projectPath
Copy link
Contributor

@AbhitejJohn AbhitejJohn Apr 11, 2017

Choose a reason for hiding this comment

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

quiet [](start = 34, length = 5)

can we make this minimal so we know whats going on instead of it looking like its stuck. Might want to change it on top(nuget restore) too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -3,6 +3,7 @@
<PropertyGroup>
<TestFxRoot Condition="$(TestFxRoot) == ''">..\..\</TestFxRoot>
<RepoRoot>..\..\</RepoRoot>
<CommonRoot>$(TestFxRoot)src\Adapter\Common\</CommonRoot>
Copy link
Contributor

Choose a reason for hiding this comment

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

CommonRoot [](start = 2, length = 10)

Do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

No..removing

@@ -3,10 +3,15 @@

namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices
{
#pragma warning disable SA1649 // SA1649FileNameMustMatchTypeName

internal class Constants
Copy link
Contributor

Choose a reason for hiding this comment

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

Constants [](start = 19, length = 9)

why not just call this file constants.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed name to Constants.cs

@@ -3,6 +3,7 @@

namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices
{
using System;
Copy link
Contributor

Choose a reason for hiding this comment

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

using System; [](start = 4, length = 13)

is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

No..removing

@@ -45,7 +51,7 @@ public void LoadAssemblyShouldThrowExceptionIfFileIsNotFound()
[TestMethod]
public void LoadAssemblyShouldLoadAssemblyInCurrentContext()
{
var filePath = Assembly.GetExecutingAssembly().Location;
var filePath = typeof(FileOperationsTests).GetTypeInfo().Assembly.Location; // Assembly.GetExecutingAssembly().Location;
Copy link
Contributor

Choose a reason for hiding this comment

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

// Assembly.GetExecutingAssembly().Location; [](start = 88, length = 44)

nit: remove.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@@ -99,7 +105,7 @@ public void CreateNavigationSessionShouldReturnDiaSession()
typeof(MockDiaSession).AssemblyQualifiedName,
typeof(MockDiaNavigationData).AssemblyQualifiedName);

var diaSession = this.fileOperations.CreateNavigationSession(Assembly.GetExecutingAssembly().Location);
var diaSession = this.fileOperations.CreateNavigationSession(typeof(FileOperationsTests).GetTypeInfo().Assembly.Location); // Assembly.GetExecutingAssembly().Location);
Copy link
Contributor

Choose a reason for hiding this comment

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

// Assembly.GetExecutingAssembly().Location); [](start = 139, length = 45)

nit:remove. Here and everywhere else.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@@ -35,20 +35,47 @@
<WarningLevel>4</WarningLevel>
</PropertyGroup>
<ItemGroup>
<Reference Include="Castle.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc, processorArchitecture=MSIL">
<HintPath>..\..\..\packages\Castle.Core.4.0.0\lib\net45\Castle.Core.dll</HintPath>
<Private>True</Private>
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove.

<Reference Include="Microsoft.VisualStudio.QualityTools.UnitTestFramework, Version=10.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
<Aliases>FrameworkV1</Aliases>
</Reference>
<Reference Include="Moq, Version=4.7.8.0, Culture=neutral, PublicKeyToken=69f491c39445e920, processorArchitecture=MSIL">
<HintPath>..\..\..\packages\Moq.4.7.8\lib\net45\Moq.dll</HintPath>
<Private>True</Private>
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove.

/// The universal test source host tests.
/// </summary>
[TestClass]
public class UniversalTestSourceHostTests
Copy link
Contributor

Choose a reason for hiding this comment

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

UniversalTestSourceHostTests [](start = 17, length = 28)

why was this removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

This moved to shared code.

@AbhitejJohn
Copy link
Contributor

Do check if we need changes in test.cmd as well and check E2E. This is a great value add!! :)

@codito
Copy link
Contributor

codito commented Apr 12, 2017

What's the rationale for naming all files ns10 and ns13? Isn't a folder serving the purpose?

@codito
Copy link
Contributor

codito commented Apr 12, 2017

@mayankbansal018 can you take a look please. These changes are similar to the PlatformAbstractions layer in TP.

@AbhitejJohn
Copy link
Contributor

AbhitejJohn commented Apr 12, 2017

@codito : It would be way easier to identify which file we are editing in VS or any other editor.

@AbhitejJohn
Copy link
Contributor

@jayaranigarg : Do update the nuspec files as well - removing dotnet folder, adding this new platform service and adding the new Tracer library as a dependency.

scripts/test.ps1 Outdated
@@ -49,6 +49,8 @@ $TFT_Configuration = $Configuration
$TFT_Pattern = $Pattern
$TFT_Parallel = $Parallel
$TFT_All = $All
$TFB_NetCoreContainers =@("MSTestAdapter.PlatformServices.NetCore.UnitTests.dll")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: would fit better into the env variables on top where all the other patterns are.

scripts/test.ps1 Outdated
{
Write-Error "Unable to find vstest.console.exe at $vstestPath. Test aborted."
}
$TPV2VSTestPath = Get-TPv2VSTestPath
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: local variables start with non-caps.

scripts/test.ps1 Outdated
function Get-TPv2VSTestPath
{
$packagesPath = Locate-PackagesPath
$vstestConsolePath = Join-Path -path $packagesPath "Microsoft.TestPlatform.15.0.1\tools\net46\vstest.console.exe"
Copy link
Contributor

Choose a reason for hiding this comment

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

variable on top please.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would not work in CDP scenarios where packages do not get copied over.

@@ -60,4 +58,9 @@
<ResolvedCompileFileDefinitions Remove="@(ResolvedCompileFileDefinitions)" Condition="'%(ResolvedCompileFileDefinitions.Filename)' == 'Microsoft.VisualStudio.TestPlatform.TestFramework.Extensions' or '%(ResolvedCompileFileDefinitions.Filename)' == 'Microsoft.VisualStudio.TestPlatform.TestFramework'" />
</ItemGroup>
</Target>
<Target Name="PostBuild" AfterTargets="PostBuildEvent">
<Exec Command="xcopy /Y /I $(TargetDir)..\..\MSTest.CoreAdapter\Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.dll $(TargetDir)" />
Copy link
Contributor

Choose a reason for hiding this comment

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

$(OutDir)?

Write-Error "Unable to find vstest.console.exe at $vstestPath. Test aborted."
}

Write-Verbose "$vstestPath $testContainers $additionalArguments /logger:trx"
Copy link
Contributor

Choose a reason for hiding this comment

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

TPV2 vstest.console does not work? We might just want to use that everywhere for consistency. You can take this in the next PR.

<PackageTargetFallback Condition=" '$(TargetFramework)' == 'netcoreapp1.0' ">$(PackageTargetFallback);portable-net45+win8+wpa81+wp8</PackageTargetFallback>
</PropertyGroup>
<ItemGroup>
<Compile Include="..\MSTest.CoreAdapter.TestUtilities\ActionUtility.cs" Link="ActionUtility.cs" />
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to add cs files in new cs projects.

@mayankbansal018 mayankbansal018 requested review from 035 and mayankbansal018 and removed request for 035 April 13, 2017 10:16
@jayaranigarg jayaranigarg merged commit 5f8e5a4 into microsoft:master Apr 13, 2017
@AbhitejJohn
Copy link
Contributor

Tagging #18 and #19.

@jayaranigarg jayaranigarg deleted the dev/jagarg/console branch December 26, 2017 12:54
singhsarab added a commit to singhsarab/testfx that referenced this pull request Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants