Bump test project dependencies #383
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Castle.Core, Moq, and xUnit versions were all out of date - hold xunit.runner.visualstudio version back in .NET SDK test projects - can be moved to latest version after we stop testing on netcoreapp2.1 - add missing xunit.analyzers reference to System.Web.Razor.Test project - move all test projects to .NET v4.6.2 (a supported framework) - remove `netstandard` reference in System.Net.Http.Formatting.Test; not needed w/ new TFM & updated references - further separate build of Microsoft.TestCommon project when invoked from NetCore.Test project - special case `RestorePackages` for this case - add System.Net.Http references to avoid conflicting versions e.g. src/ and test/ TFMs differ
- adjust Microsoft.TestCommon code - nit: use `Array.Empty<byte>()` in `TranscodingStreamTests` - `TranscodingStream` `internal`s can be `private` instead
- address xUnit2000 warnings - pass expected values to `Assert.Equal(...)` as correct (left) argument - make generic method types explicit to avoid `error CS0121: The call is ambiguous between the following methods or properties: ...` - note: cannot remove unnecessary xUnit1013 suppression - related bug (xunit/xunit#1466) apparently not fixed in 1.0.0 analyzers package - was xunit/xunit.analyzers#82 fix (in 2017) insufficient?
- avoid `ProtectedMock\`1.ThrowIfPublicMethod(...)` `throw`ing - use new overloads introduced in the Moq 4.11.0 release - adjust to Moq hiding the `ObjectProxy` type better - update `ControllerContext` mocking to avoid NREs - setting `HttpContext.User` on the `Object` left `get` value `null` (did nothing) - nit: use `SetupGet(...)` for another property
dougbu
requested review from
javiercn,
MackinnonBuck,
TanayParikh,
Nick-Stanton and
a team
wtgodbe
reviewed
Jan 30, 2023
| @@ -1,7 +1,7 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | |||
| <Import Project="..\..\packages\xunit.runner.visualstudio.2.3.1\build\net20\xunit.runner.visualstudio.props" Condition="Exists('..\..\packages\xunit.runner.visualstudio.2.3.1\build\net20\xunit.runner.visualstudio.props')" /> | |||
| <Import Project="..\..\packages\xunit.core.2.3.0\build\xunit.core.props" Condition="Exists('..\..\packages\xunit.core.2.3.0\build\xunit.core.props')" /> | |||
| <Import Project="..\..\packages\xunit.runner.visualstudio.2.4.5\build\net462\xunit.runner.visualstudio.props" Condition="Exists('..\..\packages\xunit.runner.visualstudio.2.4.5\build\net462\xunit.runner.visualstudio.props')" /> | |||
There was a problem hiding this comment.
Would it be straightforward to parameterize versions in this repo? Would make future changes easier. But if it's pretty involved then it's probably not worth it.
There was a problem hiding this comment.
It would be fairly involved because packages.config files can't use msbuild variables. Therefore, not worthwhile.
In addition, it's worth doing these changes manually (well, w/ a careful regular expression or two 😉) because it gives us an opportunity to check the assembly paths (in case new assemblies are available).
wtgodbe
reviewed
Jan 30, 2023
| @@ -9,15 +9,22 @@ | |||
| <DefineConstants>$(DefineConstants);NETFX_CORE</DefineConstants> | |||
| <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies> | |||
| <AllowUnsafeBlocks>true</AllowUnsafeBlocks> | |||
|
|
|||
| <!-- Version 2.4.5 does not support netstandard2.1. --> | |||
| <_VSRunnerVersion Condition=" '$(TargetFramework)' == 'netcoreapp2.1' ">2.4.3</_VSRunnerVersion> | |||
There was a problem hiding this comment.
Why not just use 2.4.3 everywhere?
There was a problem hiding this comment.
My aim is to use the latest versions everywhere I could. I also plan to expand the testing matrix to include net6.0 and / or net7.0. Bottom line, netcoreapp2.1 is a special case and I'd rather make that clear.
wtgodbe
approved these changes
Jan 30, 2023
TanayParikh
approved these changes
Jan 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bump test project dependencies
netstandardreference in System.Net.Http.Formatting.Test; not needed w/ new TFM & updated referencesRestorePackagesfor this caseReact to changed xUnit APIs
Array.Empty<byte>()inTranscodingStreamTestsTranscodingStreaminternals can beprivateinsteadResolve xUnit issues new analyzers find
Assert.Equal(...)as correct (left) argumenterror CS0121: The call is ambiguous between the following methods or properties: ...React to new Moq changes
ProtectedMock\1.ThrowIfPublicMethod(...)ObjectProxytype betterControllerContextmocking to avoid NREsHttpContext.Useron theObjectleftgetvaluenull(did nothing)SetupGet(...)for another property