Initial support of Microsoft.Testing.Platform #1153

Evangelink · 2024-02-20T13:03:54Z

Provide the base changes to support the new platform while keeping backward compatibility.

There are multiple points to discuss for the design, naming and update of your testing infra.

Fixes #1152

nuget.config

nuget/NUnit3TestAdapter.nuspec

nuget/net462/NUnit3TestAdapter.props

Evangelink · 2024-02-20T13:17:39Z

src/NUnitTestAdapter/TestingPlatformAdapter/NUnitBridgedTestFramework.cs

+                }
+
+                // TODO: Uncomment this line when InternalsVisibleTo is set up.
+                // var nopWriter = new InternalTraceWriter(new StreamWriter(Stream.Null));


I am not sure what's different but on our last test with NUnit we were able to access this type.

Yes, it is public. And has been so since 2013.

@OsirisTerje This is not public, the class is internal

We need you to set the InternalsVisibleTo from nunit.engine.core to testadapter.

My bad, I just checked the old code and we were actually referencing nunit package in addition to nunit.engine.

Is it fine to do so here?

We're talking about two different InternalTraceWriter classes, I referred to the one in the framework:

Referencing the framework from the adapter might be troublesome. We need to ensure people can use different versions of nunit with the same adapter. Possibly some clever use of wildcards on the versions can handle it.

Changing the engine to make this public instead of internal is doable.

I honestly don't remember well all details but we used this trick following a bug we were having. @nohwnd Do you recall?

I don't remember, but all the details of the fix, including link to the issue it is fixing are in the code in form of comments. 🙂 If we need to change our dependencies and use class that is not public, we instantiate it via reflection as well, since we are already patching it via reflection. But looks like you reverted the // new ... so I guess it is okay as is.

src/NUnitTestAdapter/TestingPlatformAdapter/NUnitExtension.cs

src/NUnitTestAdapter/TestingPlatformAdapter/TestApplicationBuilderExtensions.cs

MarcoRossignoli · 2024-02-20T13:32:32Z

SeanKilleen · 2024-02-20T19:50:37Z

I'd personally like to see microsoft/testfx#2298 resolved before we move forward with this, to ensure that Microsoft is actually embracing a Framework agnostic runner vs trying to extinguish other frameworks (the comment referenced in that issue is...not encouraging.)

It may have already been resolved in a satisfactory way (Microsoft.Testing.Platform seems to be in the naming conventions now and the repo is testfx), but I'd like to confirm it.

Evangelink · 2024-02-20T19:58:45Z

@SeanKilleen I can already close the issue you are referencing. As you can see in the nuget packages added everything is already MSTest agnostic. The only thing that remains MSTest oriented is the main doc.

On the contrary to VSTest which is the main tool people use, the new platform is hidden and not something people are much aware of.

SeanKilleen · 2024-02-20T21:23:54Z

@Evangelink great, thanks for confirming!

Evangelink · 2024-03-02T10:00:50Z

@OsirisTerje @SeanKilleen How do you guys want me to move on this PR? Not sure if you prefer to discuss on the PR or ticket.

Are you happy with having the new platform has optional disabled by default on the same nuget package or would you like to have a different package with maybe the runner enabled by default?
If we move on with a flag to enable, how do you want to name it (current draft uses EnableNUnitRunner)?

Not for now but for some future, I'd be happy to start seeing the "VS" part going away (maybe if/when we are able to drop support from VSTest and focus only on the new platform). We are really trying to have the new platform felt more as a dotnet tooling/sdk concept rather than a VS centric feature.

BTW if that's any easier, I'd be happy to setup some call with you (hopefully we have some common timezone time :)) so we can discuss how we can help you.

OsirisTerje · 2024-03-02T14:31:08Z

Best to discuss things on the ticket, as that will kind of persist more than the PR - as we link more to issues than to PRs.

So I'll simply copy your questions here over to the ticket and start answering there.

PS: Are the relevant packages now published so we could drop the local feed and get the CI build green?

Evangelink · 2024-03-14T10:18:25Z

Here is my build and sample testing workflow:

build.cmd
build.cmd -t package
cd samples\1-nunit-runner
dotnet build
.\bin\Debug\net6.0\nunit-runner.exe
cd ..\2-nunit-runner-dotnet-test\
dotnet test

Could you confirm this is working for you?

OsirisTerje · 2024-03-14T16:19:14Z

I see the builds are still failing, due to the local nuget path.

D:\a\1\s\src\NUnit3AdapterExternalTests\NUnit3AdapterExternalTests.csproj : error NU1301: The local source 'C:\src\nunit3-vs-adapter\package' doesn't exist. [D:\a\1\s\NUnit3TestAdapter.sln]

OsirisTerje · 2024-03-14T16:32:10Z

I changed the nuget.config to point to a relative path

But as the packages are not there, the build fails.

I believe the best is to upload the packages to e.g. MyGet or as beta packages to nuget.

If I remove the Local feed, the solution builds, so they don't seem to be needed for the adapter ? You mention "for the samples". The nuget.config needs to go there, see below.

Also, the samples folder should be moved into either the NUnit-*-Samples repo or the docs repo. @SeanKilleen - what do you think?

Your pt.1 and 2 works
Running tests give 1 failed,

Meaning backwards compatibility is gone.

The acceptance tests, we have 72 failed out of 98, but this is our fault, as it is caused by mismatch of our tests with version 4 of NUnit. Need to add an issue to get that fixed :-) We need these tests for this PR.

I added new nuget.config files in the samples pointing up to the package directory.

Pt 3 -7 then works.

The output from 4:

The output from 7:

No information here about tests however, except that "they" succeeded.

Evangelink · 2024-03-14T17:01:19Z

I changed the nuget.config to point to a relative path

Thanks, I called out this was not workin and was only a temp solution to demo you something so we can start some discussion.

Also, the samples folder should be moved into either the NUnit-*-Samples repo or the docs repo

Same as above, this is just a temp "solution" to demo. I expect quite some cleaning to do here.

Output from 4 is using vstest output (old dotnet test) while output from 7 uses the new default.

No information here about tests however, except that "they" succeeded.

This is to reduce output contention by default. We know this is not the most loved solution and we have this issue microsoft/testfx#2162 to track improving this output for "interactive" scenarios.

For the tests, I expect you may want to "improve" them to run maybe in the 2 modes (runner or vstest) for better safety.

OsirisTerje · 2024-04-07T20:33:46Z

I have copied the Samples folder into the adapter issues repo, at https://github.com/nunit/nunit3-vs-adapter.issues/tree/master/Issue1152.

[ ] It should be removed from this PR.
[ ] Referring to the package can be done from a local location, e.g. C:\nuget, or pretty soon now, from Myget.

We depend on the objectmodel version 11. This is for backward compatibility down to VS 2012. (We actually don't know anymore which VS version the adapter works for currently.) And we don't have any metrics either, nor any issues raised wrt this.
I assume you want 4.6.2 to be able to run the new Testing.Platform, and that is fine. That should work enabling the later objectmodel, but when it runs in the "old" mode, it should still run version 11. I believe it will make for some interesting configurations.
Please update this PR with the current master branch. That should make the PR pass the Cake CI build.
Then please redirect this PR to the branch named vnext. It is currently pointing to the same commit in master. We can then use this branch as an integration branch, and also deploy an alpha-version from that branch. It will also make it easier for us to work on this.

# Conflicts: # src/NUnitTestAdapter/NUnit.TestAdapter.csproj # src/NUnitTestAdapterTests/NUnit.TestAdapter.Tests.csproj

Evangelink · 2024-04-22T12:26:58Z

@OsirisTerje I have commented ObjectModel v11 in the test because that's not clear how you want to handle that case.

I have been discussing with @nohwnd and having dependency on newer version of ObjectModel should not break support for older VS.

OsirisTerje · 2024-04-22T12:54:45Z

@Evangelink If it doesn't crash the older ones, we can remove it. We just have to trust you on that one. :-)

I no longer have any of those installed, and not sure if it will break on other things, like what framework we're actually now running under, so, anyway... Then we let that go.

OsirisTerje · 2024-04-22T12:57:31Z

The CI breaks due to the nuget local src, used for the samples. That should be removed. I have already moved the samples themselves, so when this is merged (and pushed to myget using the Publish build), the samples should grab the new version from myget.org

Evangelink · 2024-04-22T14:11:51Z

@Evangelink If it doesn't crash the older ones, we can remove it. We just have to trust you on that one. :-)

It shouldn't said our VSTest expert :)

The CI breaks due to the nuget local src, used for the samples. That should be removed. I have already moved the samples themselves, so when this is merged (and pushed to myget using the Publish build), the samples should grab the new version from myget.org

Yep sorry, will fix that now.

Evangelink · 2024-04-22T14:42:41Z

Working on a fix for the tests.

The copy of the file was moved from the .props to the .targets

Initial support of Microsoft.Testing.Platform

d7dc497

Evangelink commented Feb 20, 2024

View reviewed changes

Evangelink mentioned this pull request Feb 22, 2024

Allow xunit, nunit and other frameworks to run with the new runner microsoft/testfx#2164

Closed

Evangelink added 3 commits March 14, 2024 10:41

Bump bridge version

9da8cd9

Drop telemetry

24133f3

Enable trace writer fix for NUnit

49326bd

Evangelink added 3 commits April 22, 2024 14:06

Merge branch 'master' into microsoft-testing-platform

3789fff

# Conflicts: # src/NUnitTestAdapter/NUnit.TestAdapter.csproj # src/NUnitTestAdapterTests/NUnit.TestAdapter.Tests.csproj

Bump version of the bridge

8754cd1

Comment out object model v11

c3fa697

Evangelink changed the base branch from master to vnext April 22, 2024 12:25

Evangelink added 3 commits April 22, 2024 16:13

Remove samples and "local" entry in nuget.config

8970bc7

Tests: drop Microsoft.TestPlatform.ObjectModel v11

b588819

Fix ObjectModel v11 test

e742849

Fix broken tests

9a340ce

The copy of the file was moved from the .props to the .targets

Evangelink marked this pull request as ready for review May 2, 2024 08:11

Evangelink added 2 commits May 2, 2024 10:18

Bump platform version in nuspec

23c6da7

Nit: tab vs space

80da537

OsirisTerje merged commit 72e9b6b into nunit:vnext May 3, 2024
3 of 4 checks passed

Evangelink deleted the microsoft-testing-platform branch May 3, 2024 19:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial support of Microsoft.Testing.Platform #1153

Initial support of Microsoft.Testing.Platform #1153

Evangelink commented Feb 20, 2024

Evangelink Feb 20, 2024

OsirisTerje Feb 20, 2024

Evangelink Mar 14, 2024

Evangelink Mar 14, 2024 •

edited

OsirisTerje Mar 14, 2024

Evangelink Apr 22, 2024

nohwnd May 2, 2024

MarcoRossignoli commented Feb 20, 2024

SeanKilleen commented Feb 20, 2024 •

edited

Evangelink commented Feb 20, 2024

SeanKilleen commented Feb 20, 2024

Evangelink commented Mar 2, 2024 •

edited

OsirisTerje commented Mar 2, 2024

Evangelink commented Mar 14, 2024

OsirisTerje commented Mar 14, 2024

OsirisTerje commented Mar 14, 2024 •

edited

Evangelink commented Mar 14, 2024

OsirisTerje commented Apr 7, 2024

Evangelink commented Apr 22, 2024

OsirisTerje commented Apr 22, 2024

OsirisTerje commented Apr 22, 2024 •

edited

Evangelink commented Apr 22, 2024

Evangelink commented Apr 22, 2024

Initial support of Microsoft.Testing.Platform #1153

Initial support of Microsoft.Testing.Platform #1153

Conversation

Evangelink commented Feb 20, 2024

Evangelink Feb 20, 2024

Choose a reason for hiding this comment

OsirisTerje Feb 20, 2024

Choose a reason for hiding this comment

Evangelink Mar 14, 2024

Choose a reason for hiding this comment

Evangelink Mar 14, 2024 • edited

Choose a reason for hiding this comment

OsirisTerje Mar 14, 2024

Choose a reason for hiding this comment

Evangelink Apr 22, 2024

Choose a reason for hiding this comment

nohwnd May 2, 2024

Choose a reason for hiding this comment

MarcoRossignoli commented Feb 20, 2024

SeanKilleen commented Feb 20, 2024 • edited

Evangelink commented Feb 20, 2024

SeanKilleen commented Feb 20, 2024

Evangelink commented Mar 2, 2024 • edited

OsirisTerje commented Mar 2, 2024

Evangelink commented Mar 14, 2024

OsirisTerje commented Mar 14, 2024

OsirisTerje commented Mar 14, 2024 • edited

Evangelink commented Mar 14, 2024

OsirisTerje commented Apr 7, 2024

Evangelink commented Apr 22, 2024

OsirisTerje commented Apr 22, 2024

OsirisTerje commented Apr 22, 2024 • edited

Evangelink commented Apr 22, 2024

Evangelink commented Apr 22, 2024

Evangelink Mar 14, 2024 •

edited

SeanKilleen commented Feb 20, 2024 •

edited

Evangelink commented Mar 2, 2024 •

edited

OsirisTerje commented Mar 14, 2024 •

edited

OsirisTerje commented Apr 22, 2024 •

edited