Skip to content

[WIP]Make TestFx repo CPS Based - #411

Closed
Mayank Bansal (mayankbansal018) wants to merge 13 commits into
microsoft:masterfrom
mayankbansal018:makeTestFxCPS
Closed

Mayank Bansal (mayankbansal018) wants to merge 13 commits into
microsoft:masterfrom
mayankbansal018:makeTestFxCPS

Conversation

@mayankbansal018

@mayankbansal018 Mayank Bansal (mayankbansal018) commented Apr 26, 2018 •

Copy link
Copy Markdown
Contributor

Things to verify before pushing this:

  • Localization
  • Assembly, & File Version
  • Nugets Upgradation
  • Vsix Templates / Wizard
  • Run on VSO

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

dotnet bot (@dotnet-bot) test this please

<TestFxRoot Condition="$(TestFxRoot) == ''">..\..\..\</TestFxRoot>
</PropertyGroup>
<Import Project="$(TestFxRoot)scripts\build\TestFx.Settings.targets" />
<Import Project="$(TestFxRoot)scripts\build\TestFx.Settings.Templates.targets" />

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.

Why did we separate them?

<package id="MSTest.Internal.TestFx.Localized.Documentation" version="1.0.0-build-20170420-1" />
<package id="vswhere" version="2.0.2" />
<package id="Pdb2Pdb" version="1.1.0-beta1-62316-01" />
<package id="Microsoft.TestPlatform" version="15.5.0" />

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.

Shouldn't we use 15.7 instead?

<ItemGroup Condition=" '$(TargetFramework)' == 'net45' ">
<PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="11.0.1" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.3' ">

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.

Why a conditional package reference here?


<ItemGroup>
<PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="11.0.0" />
<PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="11.0.1" />

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.

what is the difference between 11.0 and 11.1?

<ErrorReport>prompt</ErrorReport>
<WarningLevel>4</WarningLevel>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<TargetFrameworks>netstandard1.3;net45</TargetFrameworks>

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.

why are we building this for net45?

<file src="Build\NetCore\MSTest.TestAdapter.props" target="build\netcoreapp1.0\" />
<file src="MSTest.CoreAdapter\netstandard1.3\Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.dll" target="build\netcoreapp1.0\" />
<file src="PlatformServices.Interface\netstandard1.3\Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface.dll" target="build\netcoreapp1.0\" />
<file src="MSTest.Core\netstandard1.1\Microsoft.VisualStudio.TestPlatform.TestFramework.dll" target="build\net45\" />

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.

Nit: build\netcoreapp1.0\

<file src="MSTest.CoreAdapter\ru\Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.resources.dll" target="\build\_common\ru" />
<file src="MSTest.CoreAdapter\tr\Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.resources.dll" target="\build\_common\tr" />
<!-- Resources -->
<file src="MSTest.CoreAdapter\netstandard1.3\zh-Hans\Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.resources.dll" target="\build\_common\zh-Hans" />

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.

Will discuss on where to keep resources offline..

var testAssetPath =
Path.Combine(
Directory.GetParent(Directory.GetParent(Directory.GetParent(currentAssemblyPath).FullName).FullName).FullName,
"artifacts",

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.

??

@singhsarab

Copy link
Copy Markdown
Contributor

Mayank Bansal (@mayankbansal018) What is the status of this PR?

@jayaranigarg

Copy link
Copy Markdown
Member

Mayank Bansal (@mayankbansal018) : When are we planning to push this PR in? This issue #481 will also get resolved as part of this PR.

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

Jayarani garg (@jayaranigarg) , lets bring this in next sprint planning, the work is almost done, validations do remain.

@parrainc

Copy link
Copy Markdown
Contributor

Mayank Bansal (@mayankbansal018) are there any news about this PR?
Some opened issues depend on this changes in order to be fixed.

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

Carlos Parra (@parrainc) , Unfortunately I've not been able to prioritize this task, due to other tasks that I've in hand. As per the last status of this PR, I had moved the project to CPS, & everything was running fine.

I understand that there are a few bugs pending on this, because of which I would encourage if you would like to take this further, let me know if that would be possible, I would give you permission on my branch.

@parrainc

Copy link
Copy Markdown
Contributor

Mayank Bansal (@mayankbansal018) i understand.
You can count me in, it would be a pleasure to take this further. I'll make sure to get this done asap, so those few issues that depends on this changes got closed soon 👍

in any case, if questions arise on the road, i'll be commenting here :)

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

Carlos Parra (@parrainc), great to hear this, I've added you as collaborator, on my from, please checkout branch makeTestFxCPS. You will have to merge master back into this, since the branch would be pretty behind. Please do comment back here in case you need any help.

👍 😃

@parrainc

Copy link
Copy Markdown
Contributor

Mayank Bansal (@mayankbansal018) quick question... is there a specific reason of why we are upgrading to netstandard 1.3 instead of a major version (i.e. 1.5, 1.6) or the latest one (2.0)? I'm not sure if latest or a different version than 1.3 is not fully compatible with some of the projects in the solution. If that's not the case then I'm just curious to know what could be done about it.

@dotMorten

Copy link
Copy Markdown
Contributor

Carlos Parra (@parrainc) why upgrade to a version higher than needed? The higher the version, the fewer the platforms can use it

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

Carlos Parra (@parrainc) , as dotMorten mentioned, the lower the version for .net standard, the wider the range for platforms we can cover. For e.g. if we move to > NS1.4, then it won't work with UWP, & we would need to have separate build scripts to fix that.

@parrainc

Copy link
Copy Markdown
Contributor

gotcha

@parrainc

Copy link
Copy Markdown
Contributor

Mayank Bansal (@mayankbansal018) quick update:
did some change to sync up master with testFxCPS branch, and everything is going fine (building ok in VS, and unit and integration tests seems to behave just fine). The only issue I'm having is that while building from CLI, the following error shows up:

C:\Program Files\dotnet\sdk\2.1.403\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDepedencyResolution.targets(198,5): error NETSDK1005: Assets file 'C:...\testfx\src\Adapter\MSTest.CoreAdapter\obj\project.assets.json' doesn't have a target for '.NETFramework,Version=4.6'. Ensure that restore has run and that you have included 'net46' in the TargetFrameworks for your project. [Path to MSTest.CoreAdapter.csproj]

and the same for PlatformServices.Interface.csproj and PlatformServices.Desktop.csproj.

Locally, I included the 'net46' in the target frameworks for those projects (next to the 'net45') and it just works. I'm thinking it might be the version of the SDK I'm using for building, but not sure.

Any thought?

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

Carlos Parra (@parrainc) I'm not sure what is causing this issue. I'll however will do try to take a look at it over the weekend.

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

Carlos Parra (@parrainc) I just tried building this locally, & it worked fine. Were you able to build it locally or are you facing build issues locally as well?

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

dotnet bot (@dotnet-bot) test Windows / Debug Build please

@parrainc

Copy link
Copy Markdown
Contributor

Mayank Bansal (@mayankbansal018) For me it builds locally in VS. In CLI throws me the error I pointed out above.

Something i'm curious is the reason why is failing here. Check the details and looks like a type exists in two assemblies that is calling it.

@mayankbansal018

Copy link
Copy Markdown
Contributor Author

Carlos Parra (@parrainc) I'll try to login to the CI machines, & see what is causing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants