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

Upgrade to .NET Framework 4.6.1, add .NET Core 3.0 target #219

Closed
wants to merge 11 commits into from
Closed

Upgrade to .NET Framework 4.6.1, add .NET Core 3.0 target #219

wants to merge 11 commits into from

Conversation

hez2010
Copy link
Contributor

@hez2010 hez2010 commented Oct 22, 2019

  1. Some tests failed due to float-point precision (Microsoft has upgrade their float-point number IEEE standard), please fixes them after merging this pull request.
  2. .NET Framework 4.6.1 as minimal version is required to target both .NET Framework and .NET Core.
  3. Only support Visual Studio 2019, because Visual Studio 2017 has no build target file for Sdk Microsoft.NET.Sdk.WindowsDesktop.

TODO (by @ForNeVeR)

  • Make sure the tests are green
  • Review the NuGet packaging instructions
  • Verify that Test failure in VS2019 #217 is really fixed in VS
  • Final review
  • Create new issues for the remaining TODOs

@ForNeVeR ForNeVeR self-requested a review October 22, 2019 09:37
@ForNeVeR
Copy link
Owner

Wow, thanks a lot! I'll take a look at the changes tonight.

The issue with the number formatting in tests is known, we'll do something about it.

Deprecating VS2017 is okay, but I'm not so sure about deprecating older .NET Frameworks. Our current policy is to support all the framework versions supported by Microsoft (that means, starting with 4.5.2). I think we could make 4.5.2-compatibility mode non-default and somehow make it available only for CI builds. I'll try to invent something :)

@hez2010
Copy link
Contributor Author

hez2010 commented Oct 22, 2019

.NET Framework 4.5.2 lacks the assembly System.Core, therefore there's no way to support both .NET Framework 4.5.2 and .NET Core at the same time.
However, I think you can create a new package for .NET Framework 4.5.2 only.

src/WpfMath/WpfMath.csproj Outdated Show resolved Hide resolved
src/WpfMath/WpfMath.csproj Outdated Show resolved Hide resolved
src/WpfMath.Tests/WpfMath.Tests.fsproj Show resolved Hide resolved
@hez2010
Copy link
Contributor Author

hez2010 commented Oct 22, 2019

BTW, I have upgraded the language version to C# 8 and F# 4.7, would you like to opt-in Nullable Reference Type? It may throw cause plenty of warnings at this timeframe, but all nullable related issues will go away once all these warning are resolved. @ForNeVeR

@ForNeVeR
Copy link
Owner

Yes I want to enable them (and generally all the new stuff, all of it! 😏), but let's restrict scope of this PR to .NET Core 3.0 only at the moment.

appveyor.yml Outdated Show resolved Hide resolved
appveyor.yml Outdated Show resolved Hide resolved
Copy link
Owner

@ForNeVeR ForNeVeR left a comment

Choose a reason for hiding this comment

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

Alright, here's a checklist for me:

  • I don't like losing of compatibility with .NET 4.5.2; we aren't doing that. Probably we could implement a hacky solution (e.g. another csproj in the same directory, but based on the old project model)
  • Tests should definitely be green (but I'm okay if you omit netcoreapp3.0 tests for now)

@ForNeVeR
Copy link
Owner

I could take over this PR (and finish the remaining tasks) at any moment, please inform me after you've finished.

@hez2010
Copy link
Contributor Author

hez2010 commented Oct 22, 2019

I've finished.

  • Tests should definitely be green (but I'm okay if you omit netcoreapp3.0 tests for now)

dotnet test will test both net461 and netcoreapp3.0 targets

@ForNeVeR
Copy link
Owner

Okay, could you please run script scripts/approve-all.ps1 after running the tests locally? It will copy the new test data so you could update it

@hez2010
Copy link
Contributor Author

hez2010 commented Oct 22, 2019

Okay, could you please run script scripts/approve-all.ps1 after running the tests locally? It will copy the new test data so you could update it

Done via 5ae0b22

@ForNeVeR
Copy link
Owner

Ok, I'll fix the remaining now. Seems like the CI server still has troubles, I'll take a look.

Thanks a lot for your work!

@ForNeVeR ForNeVeR assigned ForNeVeR and unassigned hez2010 Oct 22, 2019
@hez2010
Copy link
Contributor Author

hez2010 commented Oct 22, 2019

Ok, I'll fix the remaining now. Seems like the CI server still has troubles, I'll take a look.

Thanks a lot for your work!

My local test result:

Microsoft (R) Test Execution Command Line Tool Version 16.3.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

A total of 1 test files matched the specified pattern.
cmd /c move /Y "C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\TestResults\BoxTests.wideItemInMatrixBox.received.txt" "C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\TestResults\BoxTests.wideItemInMatrixBox.approved.txt"
cmd /c move /Y "C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\TestResults\BoxTests.nestedMatrixBox.received.txt" "C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\TestResults\BoxTests.nestedMatrixBox.approved.txt"
  X WpfMath.Tests.BoxTests.wideItemInMatrixBox [116ms]
  Error Message:
   Assert.Equal() Failure
                                 ↓ (pos 45647)
Expected: ···": 4.2924428999999993,\r\n                      "Italic": 0.027···
Actual:   ···": 4.292442899999999,\r\n                      "Italic": 0.0277···
                                 ↑ (pos 45647)
  Stack Trace:
     at ApprovalTests.Reporters.TestFrameworks.AssertReporter.AssertEqual(String approvedContent, String receivedContent)
   at ApprovalTests.Reporters.TestFrameworks.AssertReporter.AssertFileContents(String approved, String received)
   at ApprovalTests.Reporters.TestFrameworks.AssertReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Approvers.FileApprover.ReportFailure(IApprovalFailureReporter reporter)
   at ApprovalTests.Core.Approver.Verify(IApprovalApprover approver, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalApprover approver, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalWriter writer, IApprovalNamer namer, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalWriter writer)
   at ApprovalTests.Approvals.Verify(String text, Func`2 scrubber)
   at WpfMath.Tests.ApprovalTestUtils.verifyObject@55-1.Invoke(String arg00) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\ApprovalTestUtils.fs:line 55
   at WpfMath.Tests.ApprovalTestUtils.verifyObject@55-2.Invoke(Object x) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\ApprovalTestUtils.fs:line 55
   at WpfMath.Tests.BoxTests.verifyBox(String source) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\BoxTests.fs:line 97
   at WpfMath.Tests.BoxTests.wideItemInMatrixBox() in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\BoxTests.fs:line 113
  X WpfMath.Tests.BoxTests.nestedMatrixBox [43ms]
  Error Message:
   Assert.Equal() Failure
                                 ↓ (pos 2331)
Expected: ···: 0.99444400000000011,\r\n          "TotalWidth": 0.500002,\r\n  ···
Actual:   ···: 0.9944440000000001,\r\n          "TotalWidth": 0.500002,\r\n   ···
                                 ↑ (pos 2331)
  Stack Trace:
     at ApprovalTests.Reporters.TestFrameworks.AssertReporter.AssertEqual(String approvedContent, String receivedContent)
   at ApprovalTests.Reporters.TestFrameworks.AssertReporter.AssertFileContents(String approved, String received)
   at ApprovalTests.Reporters.TestFrameworks.AssertReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Approvers.FileApprover.ReportFailure(IApprovalFailureReporter reporter)
   at ApprovalTests.Core.Approver.Verify(IApprovalApprover approver, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalApprover approver, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalWriter writer, IApprovalNamer namer, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalWriter writer)
   at ApprovalTests.Approvals.Verify(String text, Func`2 scrubber)
   at WpfMath.Tests.ApprovalTestUtils.verifyObject@55-1.Invoke(String arg00) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\ApprovalTestUtils.fs:line 55
   at WpfMath.Tests.ApprovalTestUtils.verifyObject@55-2.Invoke(Object x) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\ApprovalTestUtils.fs:line 55
   at WpfMath.Tests.BoxTests.verifyBox(String source) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\BoxTests.fs:line 97
   at WpfMath.Tests.BoxTests.nestedMatrixBox() in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\BoxTests.fs:line 109
cmd /c move /Y "C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\TestResults\BoxTests.casesBox.received.txt" "C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\TestResults\BoxTests.casesBox.approved.txt"
cmd /c move /Y "C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\TestResults\BoxTests.simpleMatrixBox.received.txt" "C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\TestResults\BoxTests.simpleMatrixBox.approved.txt"
  X WpfMath.Tests.BoxTests.casesBox [41ms]
  Error Message:
   Assert.Equal() Failure
                                 ↓ (pos 3217)
Expected: ···: 0.78055500000000011,\r\n              "TotalWidth": 0.52859,\r···
Actual:   ···: 0.7805550000000001,\r\n              "TotalWidth": 0.52859,\r\n···
                                 ↑ (pos 3217)
  Stack Trace:
     at ApprovalTests.Reporters.TestFrameworks.AssertReporter.AssertEqual(String approvedContent, String receivedContent)
   at ApprovalTests.Reporters.TestFrameworks.AssertReporter.AssertFileContents(String approved, String received)
   at ApprovalTests.Reporters.TestFrameworks.AssertReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Approvers.FileApprover.ReportFailure(IApprovalFailureReporter reporter)
   at ApprovalTests.Core.Approver.Verify(IApprovalApprover approver, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalApprover approver, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalWriter writer, IApprovalNamer namer, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalWriter writer)
   at ApprovalTests.Approvals.Verify(String text, Func`2 scrubber)
   at WpfMath.Tests.ApprovalTestUtils.verifyObject@55-1.Invoke(String arg00) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\ApprovalTestUtils.fs:line 55
   at WpfMath.Tests.ApprovalTestUtils.verifyObject@55-2.Invoke(Object x) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\ApprovalTestUtils.fs:line 55
   at WpfMath.Tests.BoxTests.verifyBox(String source) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\BoxTests.fs:line 97
   at WpfMath.Tests.BoxTests.casesBox() in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\BoxTests.fs:line 105
  X WpfMath.Tests.BoxTests.simpleMatrixBox [38ms]
  Error Message:
   Assert.Equal() Failure
                                 ↓ (pos 3594)
Expected: ···: 0.99444400000000011,\r\n              "TotalWidth": 0.500002,···
Actual:   ···: 0.9944440000000001,\r\n              "TotalWidth": 0.500002,\r···
                                 ↑ (pos 3594)
  Stack Trace:
     at ApprovalTests.Reporters.TestFrameworks.AssertReporter.AssertEqual(String approvedContent, String receivedContent)
   at ApprovalTests.Reporters.TestFrameworks.AssertReporter.AssertFileContents(String approved, String received)
   at ApprovalTests.Reporters.TestFrameworks.AssertReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Reporters.FirstWorkingReporter.Report(String approved, String received)
   at ApprovalTests.Approvers.FileApprover.ReportFailure(IApprovalFailureReporter reporter)
   at ApprovalTests.Core.Approver.Verify(IApprovalApprover approver, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalApprover approver, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalWriter writer, IApprovalNamer namer, IApprovalFailureReporter reporter)
   at ApprovalTests.Approvals.Verify(IApprovalWriter writer)
   at ApprovalTests.Approvals.Verify(String text, Func`2 scrubber)
   at WpfMath.Tests.ApprovalTestUtils.verifyObject@55-1.Invoke(String arg00) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\ApprovalTestUtils.fs:line 55
   at WpfMath.Tests.ApprovalTestUtils.verifyObject@55-2.Invoke(Object x) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\ApprovalTestUtils.fs:line 55
   at WpfMath.Tests.BoxTests.verifyBox(String source) in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\BoxTests.fs:line 97
   at WpfMath.Tests.BoxTests.simpleMatrixBox() in C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\BoxTests.fs:line 101

Total tests: 164
     Passed: 160
     Failed: 4
 Total time: 5.7043 Seconds
Test run for C:\Users\hez20\source\repos\wpf-math\src\WpfMath.Tests\bin\Debug\net461\WpfMath.Tests.dll(.NETFramework,Version=v4.6.1)
Microsoft (R) Test Execution Command Line Tool Version 16.3.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

A total of 1 test files matched the specified pattern.

Test Run Successful.
Total tests: 164
     Passed: 164
 Total time: 5.5285 Seconds

All tests for net461 have passed, but not for netcoreapp3.0

@ForNeVeR ForNeVeR mentioned this pull request Dec 8, 2019
7 tasks
@ForNeVeR
Copy link
Owner

ForNeVeR commented Dec 8, 2019

This PR was superseded by #228. Thanks for your work, I'll make sure to preserve your contribution and your name on the commits.

@ForNeVeR ForNeVeR closed this Dec 8, 2019
GuillaumeLebeau added a commit to AgileoAutomation/UnitsNet that referenced this pull request Jan 9, 2020
If target framework is less than .NET 4.6.1, System.Tuple is used instead of System.ValueTuple :
 - dotnet/sdk#3770
 - y-iihoshi/ThScoreFileConverter#22
 - ForNeVeR/xaml-math#219
GuillaumeLebeau added a commit to AgileoAutomation/UnitsNet that referenced this pull request Jan 9, 2020
Add conditional code to use Tuple instead of ValueTuple

If target framework is less than .NET 4.6.1, System.Tuple is used instead of System.ValueTuple :
* dotnet/sdk#3770
* y-iihoshi/ThScoreFileConverter#22
* ForNeVeR/xaml-math#219
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.

Test failure in VS2019 WPF support on .NET Core 3.0 New languages migration
2 participants