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

Fix TestPlatform.ObjectModel reference #100

Merged
merged 10 commits into from May 28, 2021

Conversation

mikeblakeuk
Copy link
Contributor

@mikeblakeuk mikeblakeuk commented May 25, 2021

Using net 4.7.2, dotnet test
Given we use
AppDomain.CurrentDomain.GetAssemblies();
The TestAdapter has a Machine TestAdapter 2.10.1 references v11 of VS Object Model.
This throws:

System.IO.FileNotFoundException: [ System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.VisualStudio.TestPlatform.ObjectModel, Version=11.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified. File name: 'Microsoft.VisualStudio.TestPlatform.ObjectModel, Version=11.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' at System.Reflection.RuntimeAssembly.GetExportedTypes(RuntimeAssembly assembly, ObjectHandleOnStack retTypes) at System.Reflection.RuntimeAssembly.GetExportedTypes()

This ONLY happens when using dotnet / vstest console, not within the IDE

We can now remove net452

(Mono.Cecil can always be included to make the packaging logic easier)

@mikeblakeuk mikeblakeuk changed the title Update Frameworks and remove net572 Update Frameworks and remove net472 May 25, 2021
@robertcoltheart
Copy link
Member

  1. We can't just remove net452, as that's a breaking change. Long term we want to go there, but that's a 2.0 thing.
  2. I ran the scenario you described above, but I'm not seeing the same error. Can you link to a repo to help reproduce this bug, or attach a solution please?

@mikeblakeuk
Copy link
Contributor Author

mikeblakeuk commented May 25, 2021

Remove the Microsoft.VisualStudio.TestPlatform.ObjectModel.dll

image

I don't think it's a fair test.

Microsoft.VisualStudio.TestPlatform.ObjectModel.dll may or may not exist.

For our test csproj, we add a reference to

    <PackageReference Include="Machine.Specifications" Version="1.1.0-beta.3" />
    <PackageReference Include="Machine.Specifications.Runner.VisualStudio" Version="2.11.0" />
    <PackageReference Include="Machine.Specifications.Should" Version="1.0.0" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.9.4" />

But the Microsoft.NET.Test.Sdk doesn't copy the Microsoft.VisualStudio.TestPlatform.ObjectModel.dll in

so DevOps / dotnet test fails

@robertcoltheart
Copy link
Member

This is by design, the ObjectModel dll is part of the dotnet test harness and doesn't need to be present for the runner to function. This sounds very similar to the xunit issue over here xunit/xunit#1852 and is a problem when a test inadvertently loads the MSpec runner dll.

@mikeblakeuk
Copy link
Contributor Author

It is related. We could add catch (FileNotFoundException e) but that feels wrong.
Why do we even reference version v11 in our stuff?
We are using v16 of the SDK so it feels like v11 shouldn't even be referenced at all.

Force 16 for non net 452 users
@robertcoltheart
Copy link
Member

No, it isn't wrong to catch FileNotFoundException, in fact its exactly how NFluent handled this issue. See NFluent/NFluent@63fb9aa#diff-f978bb23b907c755624ea04fe54ca021e22c9c0e3f6e4c701c176a72509c7fd8R96

V11 is used for backwards compatibility, since we are still supporting .net4 versions.

@mikeblakeuk
Copy link
Contributor Author

mikeblakeuk commented May 25, 2021

They added it in the test library.
Ours is in production code, not test code.
net472 can use v16 instead of v11. hence the change

@mikeblakeuk
Copy link
Contributor Author

Just for info, this would fix the Assembly load issue.

@mikeblakeuk mikeblakeuk changed the title Update Frameworks and remove net472 Fix TestPlatform.ObjectModel reference May 27, 2021
move to lib folders to avoid package warnings
@mikeblakeuk
Copy link
Contributor Author

Think this is ready now

@robertcoltheart robertcoltheart merged commit d9fee00 into machine:master May 28, 2021
@mikeblakeuk mikeblakeuk deleted the task/net-4-7-2 branch May 28, 2021 08:17
@mikeblakeuk
Copy link
Contributor Author

Amazing, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants