bilal-fazlani / commanddotnet

A modern framework for building modern CLI apps
https://commanddotnet.bilal-fazlani.com
MIT License
570 stars 29 forks source link

Allow tests to run in .net framework 4.8 #333

Closed arendvw closed 2 years ago

arendvw commented 3 years ago

This is a minimal fix to allow the test project to run in .net framework 4.8. In visual studio 2019 this edit seems to give a nice side by side view of both frameworks.

image

At this moment 300 of the total of 1512 tests are failing

arendvw commented 3 years ago

As a follow up - I can imagine two things need to be done for this request to be useful.

drewburlingame commented 3 years ago

thanks @arendvw. The test failures are from #329. An update to VS or R# changed the name of the process used in assertions. I need to think about how I want to fix it so it also continues to work in Travis.

arendvw commented 3 years ago

thanks @arendvw. The test failures are from #329. An update to VS or R# changed the name of the process used in assertions. I need to think about how I want to fix it so it also continues to work in Travis.

There is also another issue, which is that AppInfo.Instance throws an exception from .net framework because Assembly.GetEntryAssembly() is null. Replacing this with an mock instance reduced the amount of errors to 8 (some which were also related to AppInfo errors).

The exception mentions: Set the version info in AppRunner.Configure(c => c.Services.Set(new AppInfo(...)))

Also, the suggested fix does not seems to work, since AppInfo.Instance is called in multiple places, asany AppInfo set through dependency injection is never called.

For travis:

This should install .net 4.8:

choco install dotnetfx

Apparently xunit supports testing multiple frameworks

dotnet test CommandDotNet.Tests/CommandDotNet.Tests.csproj
dotnet test --framework net48 CommandDotNet.Tests/CommandDotNet.Tests.csproj
arendvw commented 3 years ago

In travis the .net frameworks seem to work pretty well ("failed successfully"). May be worthwhile to test separately from dotnet core on windows.

drewburlingame commented 3 years ago

I haven't forgotten about this PR. I'm working on #329 and updating the pattern for overriding AppInfo. I should have it done by the weekend. Once that's done, I'll follow up on these changes.

drewburlingame commented 3 years ago

I've updated the tests. They should all pass for you locally now.

arendvw commented 3 years ago

I've updated the tests. They should all pass for you locally now.

Thanks for the update, I will update and verify this PR next monday

drewburlingame commented 2 years ago

closing as we've moved to .net5.0