Skip to content

Conversation

@Evangelink
Copy link
Contributor

@Evangelink Evangelink commented Feb 19, 2024

Relates to the following Twitter discussion https://x.com/rastreus/status/1757843921198329864?s=20

My team and I have been working on a new alternative to VSTest testing platform, called Microsoft Testing Platform (that powers MSTest runner). Blogpost announcement https://devblogs.microsoft.com/dotnet/introducing-ms-test-runner/

Most of the information available at this page are related to the platform more than MSTest:
https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-runner-intro?tabs=dotnetcli

Calling the exe directly:

image

With support of new platform through dotnet test:

image

Disabling support of runner in dotnet test:

image

@Alxandr
Copy link
Contributor

Alxandr commented Feb 20, 2024

I'm not terribly keen on adding references to closed-source libraries. In general, I've taken a look at the license for the dependencies here added, and I'm not sure how I feel about it. I'm not a lawyer, and reading legalese (especially in a language that's not my native) is not what I do for a living, so I'm not outright rejecting it as of yet, but I will have to check with some other parts of the .NET and open source communities before I make up my mind on this.

I appreciate the effort taken to add support for this new testing platform. I'm just not sure I like the new platform yet though.

@Evangelink
Copy link
Contributor Author

The platform itself is open source. Most of the extensions are currently closed source but free to use and redistribute, we hope to move them towards OSS (but it will take some time - for readers, feel free to open an issue on testfx asking for some extensions to be made OSS). Retry and Hot reload are on some kind of dual license (free for OSS and requiring some MS license - VS, DevKit, Azure or anything).

It's also possible to get rid of the bridge and to implement directly the new APIs which would bring more benefits (although cutting back compat).

All of that being said, I totally understand your view.

For the pros of the new platform, they are described in the links in the PR. There are also features of the platform (such as test nodes being a tree of tests) that can't be shown directly today as the Test Explorers are too limited.

Take your time to review and process the idea.

Also feel free to reject it.

Have a nice day

@Evangelink
Copy link
Contributor Author

Hi @Alxandr ,

Just wanted to let you know that we are doing more progresses on this new testing platform and that NUnit just merged their PR and released a beta version of the new package.

I'd be happy to either resume work with you here or setup a call to give you more details if you want.

@Alxandr
Copy link
Contributor

Alxandr commented May 23, 2024

Hello. As of now I'm not interested in adding dependencies to packages that are not at-least source-available. And even that I imagine probably requires me to get rid of the FOSSA status from this project. Another thing I really dislike about these nuget packages is that they all have "source repository" links set to https://github.com/microsoft/testfx. As far as I can tell, this is just flat out false - and if so should really be remediated.

It's also possible to get rid of the bridge and to implement directly the new APIs which would bring more benefits (although cutting back compat).

Getting rid of backwards compatibility is definitely unfortunate but might be the way forward. Another option is just to make it a separate package I guess.

The third option, and the one I'm currently leaning towards the most (unless the extensions added here have their license updated) is having both implementation in one package. Given that this is doable with some sort of bridge - it should in theory be doable to just have an implementation for both runners in one assembly, no?

@Evangelink
Copy link
Contributor Author

@Alxandr Thanks for your valuable input. I am happy to let you know that we got approval to OSS the parts required to move forward this PR. I'll be working on making it available for v1.3.0 of the platform (and extensions) and once available, I'll ping you here.

@Alxandr
Copy link
Contributor

Alxandr commented Jun 19, 2024

Ah - perfect. Looking forward to see what the new testing platform can provide :)

@Evangelink
Copy link
Contributor Author

PR is merged: microsoft/testfx#3133

and here is some technical documentation for the platform: https://github.com/microsoft/testfx/blob/main/docs/testingplatform/Index.md

@Evangelink Evangelink marked this pull request as ready for review June 24, 2024 19:47
@Alxandr
Copy link
Contributor

Alxandr commented Jun 25, 2024

Just a heads up - this week is pretty stuffed, so I might not be able to get to this before next week

@Evangelink
Copy link
Contributor Author

No worries @Alxandr. Anyways we will need 1.3 preview to have the open-sourced version and full MIT but I wanted to already see CI issues so I can plan ahead.

@Alxandr
Copy link
Contributor

Alxandr commented Jul 9, 2024

FYI; I've not really started looking at this due to the build failing. I assumed that was something you'd fix before I started a proper review - but if not, that's obviously something that needs to be fixed.

@Evangelink
Copy link
Contributor Author

@Alxandr I have updated the branch. dotnet build, dotnet test and dotnet pack are working locally.

@Evangelink
Copy link
Contributor Author

Ok so it is working locally because I had one build done already... I am getting some difficulties fixing the test because of both yolo sdk main and the fact that the dll is not built when props and targets are being referenced.

Internally we are dogfooding on previous built version so we are not facing this issue of using locally built reference.

I am trying to understand at what step to plug into for the copy.

@Alxandr
Copy link
Contributor

Alxandr commented Jul 30, 2024

Yeah - I started to look at it a bit myself. I was a bit confused, because I seemed to recall we used to build a nuget package and then run tests against that, but I might be mixing up repos in my head. I'll look around a bit and see if maybe a refactor of the build/test here is warranted. I'd really like to run tests on a sample project against the actual to-be-published nupkg, instead of manually referencing files cause I've had issues previously where the nupkg was missing things.

@Evangelink
Copy link
Contributor Author

I have explored a few solutions and as far as I can see there are 2 paths we could take, both with pros and cons:

  1. The test project doesn't reference the package directly but rather will have a set of tests that would create projects and add the nuget package reference at "runtime".

  2. We git force add a fake/empty set of files in the artifacts folder so that MSBuild can resolve them, knowing that they will get overriden when we will execute the test project.

I would go with option 1 as it seems to be the cleanest one but would like your opinion before I implement any.

@Alxandr
Copy link
Contributor

Alxandr commented Aug 1, 2024

There are simpler solutions - just remove the test project from the sln and modify CI to create a nuget package, place it in a directory, and reference it from the test project. I'm pretty sure I do that in another project of mine that produces msbuild SDKs - but it's been a while since I looked at it. Just let this simmer til over the weekend and I will hopefull have time to get reacquainted with that code.

@Alxandr
Copy link
Contributor

Alxandr commented Aug 1, 2024

Just spent a few seconds looking, and there's this file: https://github.com/YoloDev/YoloDev.Sdk/blob/main/sample/NuGet.config - so I'm pretty sure I remember correctly. I think I might want to do the same solution here.

@Alxandr
Copy link
Contributor

Alxandr commented Nov 5, 2024

I think we'll leave the PR untill you know if there are/will be any guidelines for naming the properties, else I think we'll go with renaming it to EnableExpectoTestingPlatformIntegration. I'm not to worried about future updates like 1.4.3, but I don't want to release 1 set of property names just to rename in 2 weeks.

@Alxandr
Copy link
Contributor

Alxandr commented Jan 14, 2025

@Evangelink any updates? Microsoft.Testing.Extensions.VSTestBridge has released version 1.5.1 already.

@Evangelink
Copy link
Contributor Author

Hey thanks for the ping @Alxandr , I totally forgot about the remaining work here. Let me update the needed pieces and ping you back for the final review and merge.

@Evangelink
Copy link
Contributor Author

@Alxandr Please review again.

@Evangelink
Copy link
Contributor Author

Evangelink commented Jan 17, 2025

Small ping on the review @Alxandr. Also linking this here in case you haven't seen the notification.

@Alxandr
Copy link
Contributor

Alxandr commented Jan 17, 2025

Github is not showing this in a nice way, but I added the following to one of the conversations that's not resolved yet:

Then please rename this to EnableExpectoTestingPlatformIntegration.

after you asked me for a review last time.

Other than that, everything is fine.

@Evangelink
Copy link
Contributor Author

Github is not showing this in a nice way, but I added the following to one of the conversations that's not resolved yet:

Then please rename this to EnableExpectoTestingPlatformIntegration.

after you asked me for a review last time.

Other than that, everything is fine.

Oh sorry, totally missed it! Fixing now and I'll update the blogpost example.

@Alxandr
Copy link
Contributor

Alxandr commented Jan 18, 2025

No issue. I also noticed GH didn't add it to the bottom of the conversation (as it used to do I think?), so was easlily missable.

@Alxandr Alxandr merged commit e463432 into YoloDev:main Jan 18, 2025
7 checks passed
@Alxandr
Copy link
Contributor

Alxandr commented Jan 18, 2025

Thanks for the great work. I'm not realeasing this version immediately (cause this PR went on for so long 😛), so I'll let it sit for a day or 3 in case you realise there was something that was missed.

@Evangelink Evangelink deleted the microsoft-testing-platform branch January 18, 2025 12:47
@Evangelink
Copy link
Contributor Author

@Alxandr We would like to publish our blogpost soon. Would it be possible to let me know the version to use for the blogpost? Thanks!

@Alxandr
Copy link
Contributor

Alxandr commented Feb 4, 2025

Crap - I forgot to rename this PR to enable realeasing for it before merging... Let me see if I can't git-snafu my way out of this. It may mess with committ author of the PR merge-commit though (sorry about that in advance, it will still link to this PR).

Also, do note that the blogpost is using the wrong msbuild property.

@Evangelink
Copy link
Contributor Author

It may mess with committ author of the PR merge-commit though (sorry about that in advance, it will still link to this PR).

No worries :)

Also, do note that the blogpost is using the wrong msbuild property.

Oooh thanks!

Seeing the linked PRs, I assume versioning will be 0.15.0

@Alxandr
Copy link
Contributor

Alxandr commented Feb 4, 2025

In theory it should be released to nuget as 0.15.0 in about 40 minutes.

The progress can be followed here: https://github.com/YoloDev/YoloDev.Expecto.TestSdk/actions/runs/13132892110

@Evangelink
Copy link
Contributor Author

Invalid api key :S

@Alxandr
Copy link
Contributor

Alxandr commented Feb 4, 2025

Yeah - it took a lot of retries: https://www.nuget.org/packages/YoloDev.Expecto.TestSdk

@Evangelink
Copy link
Contributor Author

I have some weird warnings about FSharp.Core but outside of that, it's all working great:

image
image

@Alxandr
Copy link
Contributor

Alxandr commented Feb 4, 2025

What warnings? Could you make an issue if you think it's not something related to your system?

@Evangelink
Copy link
Contributor Author

I think it's either my machine or F# in general, I get NU1608: Detected package version outside of dependency constraint: Expecto 10.0.0 requires FSharp.Core (= 7.0.200) but version FSharp.Core 9.0.200-beta.25056.5 was resolved.

@baronfel
Copy link

baronfel commented Feb 4, 2025

@Evangelink that's just a problem with the 10.0.0 version of Expecto - the FSharp.Core version constraint on the PackageReference was = 7.0.200 and it should have been >= 7.0.200. It's fixed in later 10.x releases.

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.

3 participants