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

Move FQN related code into a separate NuGet package #2714

Merged
merged 14 commits into from
Jan 27, 2021

Conversation

Haplois
Copy link
Contributor

@Haplois Haplois commented Jan 24, 2021

Description

As part of #2708:

  • Moved ManagedNameUtilities into a new project.
  • Removed hardcoded support from TestCase class to restore compatibility with older versions of TestPlatform.
  • Create a new NuGet package
  • Determine what to call Microsoft.TestPlatform.ManagedNames package. We're calling it Microsoft.TestPlatform.AdapterUtilities.
  • Fix Ubuntu build

Removed ManagedNameUtilities from ObjectModel in order to restore compatibility with older versions.
@Haplois Haplois self-assigned this Jan 24, 2021
@Haplois Haplois marked this pull request as ready for review January 26, 2021 20:00
 - All references to `Microsoft.TestPlatform.ManagedNames` refactored as `Microsoft.TestPlatform.AdapterUtilities`.
@shyamnamboodiripad
Copy link
Contributor

@Haplois @peterwald Can we now delete the Reflection-based implementation of FQN from the VSUnitTesting repo?

I would still like to ensure that the Reflection and Roslyn based implementations continue to align - so it would be great to preserve unit tests for the FQN functionality in VSUnitTesting repo (i.e. replace the Reflection implementation testted by the unit tests with the new MS.TP.AdapterUtilities nuget - but continue to reference Roslyn based implementation from the VSUnitTesting repo).

Copy link
Contributor

@shyamnamboodiripad shyamnamboodiripad left a comment

Choose a reason for hiding this comment

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

👍🏾 modulo comments

@@ -32,6 +32,7 @@ build*.log
build*.wrn
build*.err
*.binlog
*.svclog
Copy link
Contributor

Choose a reason for hiding this comment

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

In case you were wondering, there was a bug in some recent internal builds of VS that would cause svclog files to be dropped in devenv.exe's CWD. I believe that bug should be fixed now :)


<PropertyGroup>
<TargetFrameworks>netstandard1.0;netstandard2.0</TargetFrameworks>
<TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">$(TargetFrameworks);uap10.0;</TargetFrameworks>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just an observation on something that is likely to come up in the future: Since managed test frameworks will probably require this reference to be added in their framework layer (as opposed to the VS / adapter layer) and since the framework layer will likely be consumed in older versions of VS (as well as outside of VS), test frameworks may end up needing to conditionally add package reference for MS.TP.AdapterUtilities to only a subset of their supported targets and / or conditionally opt in to the FQN functionality in their framework layer only in cases where they know that the caller is VS. Alternatively, if they can't consume the package they may need to maintain a parallel implementation of the FQN spec or copy the Reflection code from the current repo...

@Haplois
Copy link
Contributor Author

Haplois commented Jan 27, 2021

I added net20 and net35 support to the code, it builds when using VS, but I am having some problems with .resx files. I also needed to do some MSBuild hacks to get it to compile using dotnet cli. See: dotnet/msbuild#5985

I'll keep the code that compiles under net20 and net35 but remove the target framework for now. This will be fixed in a future PR, hopefully.

/cc: @shyamnamboodiripad

Copy link
Contributor

@AbhitejJohn AbhitejJohn left a comment

Choose a reason for hiding this comment

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

Concerned with the changes in OM. What does our test matrix look like?

<TargetPlatformVersion>10.0.14393.0</TargetPlatformVersion>
<TargetPlatformMinVersion>10.0.10240.0</TargetPlatformMinVersion>
<TargetFrameworkIdentifier>.NETPortable</TargetFrameworkIdentifier>
<TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this required? (v4.5)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually don't know. Without those properties, the UAP build fails. I am not an expert on that platform so just copied over from ObjectModel.

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird, cause we are setting TFM to a different version below. So does this project target 4.5 or 4.5.1?

 - Added `net20` and `net35` support to the code.
 - Other refactorings.
@Haplois
Copy link
Contributor Author

Haplois commented Jan 27, 2021

We're removing new FQN features from ObjectModel since they were breaking compatibility. So the changes on ObjectModel should not affect any older releases. They didn't ship on a release yet. /cc: @AbhitejJohn

@AbhitejJohn
Copy link
Contributor

@Haplois : Sounds great, thanks.

@Haplois Haplois merged commit 9bcf509 into microsoft:master Jan 27, 2021
@Haplois Haplois deleted the fqn-separation branch January 27, 2021 16:15
jakubch1 added a commit that referenced this pull request Feb 5, 2021
* Update dependencies from https://github.com/dotnet/arcade build 20201130.3 (#2659)

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.SignTool , Microsoft.DotNet.SwaggerGenerator.MSBuild
 From Version 1.0.0-beta.20570.10 -> To Version 1.0.0-beta.20580.3

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Updating Microsoft.VisualStudio.TraceDataCollector source (#2663)

* Getting Microsoft.VisualStudio.TraceDataCollector from CodeCoverageExternals  instead of TestPlatformExternals,

* Signing TraceDataCollector from the right path,

* Removing already signed dlls,

* Not signing corelib.net,

* Removing files from the list as they are not present,

Co-authored-by: faisal <[email protected]>

* Fixed "issue" pluralization in write-release-notes.ps1 (#2665)

* Fixed "issue" pluralization in write-release-notes.ps1

Co-authored-by: Medeni Baykal <[email protected]>

* Bumping Fakes TestRunnerHarness version (#2661)

* Implement Workitem support in TRX logger (#2666)

* Implemented Workitem support in TRX logger

* Renamed Workitem to WorkItem

Co-authored-by: Flepp Jann <[email protected]>

* Do not merge logs from code coverage (#2671)

* Early testhost startup performance work (#2584)

* Removed TypesToLoadAttribute from ObjectModel. (#2674)

* Removed TypesToLoadAttribute from ObjectModel, and moved the functionallity into adapters themselves.

* Attribute refactoring (#2676)

* Getting TraceDataCollector from nuget (#2678)

* Removing TraceDataCollector project,

* Getting tracedatacollector from nuget,

* Removing signing of codecoverage libs as they are already signed,
updating codecoverage version,

* fixing path to tracedatacollector,

* Fixing acceptance tests,

Co-authored-by: faisal <[email protected]>

* Adding environment variable used during build process, (#2679)

* Removing TraceDataCollector project,

* Getting tracedatacollector from nuget,

* Removing signing of codecoverage libs as they are already signed,
updating codecoverage version,

* fixing path to tracedatacollector,

* Fixing acceptance tests,

* Setting TraceDataCollectorPackagesDir vairable which is used in pipeline,

* Fixing environment variable,

* Fixing path, once again,

Co-authored-by: faisal <[email protected]>

* Update dependencies from https://github.com/dotnet/arcade build 20201221.2 (#2680)

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.SignTool , Microsoft.DotNet.SwaggerGenerator.MSBuild
 From Version 1.0.0-beta.20580.3 -> To Version 1.0.0-beta.20621.2

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Upgrade CC and CLR IE versions (#2681)

Co-authored-by: Jakub Chocholowicz <[email protected]>

* Fixed assembly names of TestHost executables (#2682)

* Upgrade fakes version (#2683)

Co-authored-by: Jakub Chocholowicz <[email protected]>

* Upgrade CC to 16.9.0-beta.20630.1 (#2684)

Co-authored-by: Jakub Chocholowicz <[email protected]>

* Loc Update (#2685)

Co-authored-by: Cristiano Suzuki <[email protected]>

* Create AzureDevOps.yml

* Update AzureDevOps.yml

* Update AzureDevOps.yml

* Update AzureDevOps.yml

* Delete AzureDevOps.yml

* Fix nuget feed (#2697)

* Removing deprecated mygets and adding new nuget sources

* Upgrade CC components (#2698)

* Upgrade CC components

* Revert nuget changes

* Refactor it

Co-authored-by: Jakub Chocholowicz <[email protected]>

* Update dependencies from https://github.com/dotnet/arcade build 20210113.4 (#2696)

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.SignTool , Microsoft.DotNet.SwaggerGenerator.MSBuild
 From Version 1.0.0-beta.20621.2 -> To Version 1.0.0-beta.21063.4

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Create AzureDevOps.yml

* Update AzureDevOps.yml

* Update AzureDevOps.yml

* Update AzureDevOps.yml

* Update AzureDevOps.yml

* Update AzureDevOps.yml

* Update AzureDevOps.yml

changed to my repo

* Update AzureDevOps.yml

* Add integration tests for dotnet test (#2689)

* Add integration tests for dotnet test

* Use relative paths and correct proj

* Update to latest .NET 6

* Install 5.0.1 runtime

* Add parametrized project

* Updated build status badge. (#2709)

* Updated build status badge.

* Update dependencies from https://github.com/dotnet/arcade build 20210121.4 (#2712)

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.SignTool , Microsoft.DotNet.SwaggerGenerator.MSBuild
 From Version 1.0.0-beta.21063.4 -> To Version 1.0.0-beta.21071.4

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update dependencies from https://github.com/dotnet/arcade build 20210122.7 (#2713)

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.SignTool , Microsoft.DotNet.SwaggerGenerator.MSBuild
 From Version 1.0.0-beta.21063.4 -> To Version 1.0.0-beta.21072.7

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Medeni Baykal <[email protected]>

* Update azure-pipelines.yml for Azure Pipelines (#2715)

* Add metrics for datacollector.exe - provides information about profilers (#2705)

* Initial version of metrics for DC profiling

* Tests

* Push fixes to make linux build

* provide telemetry opted in flag

* Small refactoring

* Change name

* Upgrade CC comp

* Move to guids

* Revert lang change

* Fixing descriptions

* Last changes

Co-authored-by: Jakub Chocholowicz <[email protected]>

* Added git branch and commit NuGet packages (#2716)

* vstest.console: CommandLineOptions: preserve stacktrace on re-throw (CA2200) (#2606)

* vstest.console: CommandLineOptions: preserve stacktrace on re-throw (CA2200)

* Update AzureDevOps.yml

* Update AzureDevOps.yml

* Move FQN related code into a separate NuGet package (#2714)

* Removed ManagedNameUtilities from ObjectModel in order to restore compatibility with older versions.
* Added `Microsoft.TestPlatform.AdapterUtilities`.
* Added `net20` and `net35` support to the code.

* Including `Microsoft.TestPlatform.AdapterUtilities` in signing (#2719)

* Included missing assemblies in signing.

* Updating nuget Pdb2Pdb package (#2720)

* Adding new Pdb2Pdb package and change to licence tag because of warning
* Fixing license
* Formating fixes

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: fhnaseer <[email protected]>
Co-authored-by: faisal <[email protected]>
Co-authored-by: Adam Ralph <[email protected]>
Co-authored-by: Medeni Baykal <[email protected]>
Co-authored-by: Vritant Bhardwaj <[email protected]>
Co-authored-by: jflepp <[email protected]>
Co-authored-by: Flepp Jann <[email protected]>
Co-authored-by: Codrin-Victor Poienaru <[email protected]>
Co-authored-by: Jakub Chocholowicz <[email protected]>
Co-authored-by: Cristiano Suzuki <[email protected]>
Co-authored-by: Cristiano Suzuki <[email protected]>
Co-authored-by: Pavel Horak <[email protected]>
Co-authored-by: Sanan Yuzbashiyev <[email protected]>
Co-authored-by: Jakub Jareš <[email protected]>
Co-authored-by: Tom Deseyn <[email protected]>
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.

6 participants