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

Update test projects to latest stable xUnit.net packages and nuget.exe #29

Closed
wants to merge 7 commits into from

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Aug 26, 2016

  • Get tests working in Visual Studio #11 (4 of 4)
  • newer version of xunit.abstractions exists (2.0.1) but it
    "is not compatible with 'xunit.extensibility.core 2.1.0 constraint: xunit.abstractions (= 2.0.0)'"
  • add xunit.runner.msbuild package to central packages.config
  • use latest xunit.exe because Beta xunit.runner.msbuild package needs at least 2.12

See details of the following in the individual commits:

  • Update TestCommon to match latest xUnit.net APIs
  • Cleanup use of static state
  • Don't leak ManualResetEventSlims or Threads waiting on them

- #11 (4 of 4 or so)
- newer version of `xunit.abstractions` exists (2.0.1) but it
  "is not compatible with 'xunit.extensibility.core 2.1.0 constraint: xunit.abstractions (= 2.0.0)'"
- add xunit.runner.msbuild package to central packages.config
 - need this package to execute tests after upgrade to xUnit.net 2.1.0
 - need recent Beta to pick up PR xunit/xunit#807; MSBuild crashes otherwise
 - for this part, do not make *.csproj changes that NuGet attempts; update our `WebStack.xunit.targets` instead
- use latest `xunit.exe` because Beta xunit.runner.msbuild package needs at least 2.12
 - old 2.7 minimum version, 2.8 that is at https://nuget.org/nuget.exe, and 2.12 are also all old
- continue to expose required xUnit.net classes in `Microsoft.TestCommon` namespace
 - adjust to `sealed` classes, namespace moves, et cetera
- do not use removed `Assert.ThrowsDelegate` and `Assert.ThrowsDelegateWithReturn` types
 - can't use these types with underlying `Assert` using `Action` and `Func<object>`
- remove `StringAssertions` since those assertions are now in xUnit.net

nits
- remove `Console.WriteLine()` from a test
- use new `Assert.Matches()` in tests checking exception messages that contain a Moq proxy type name
- do not use `GlobalFacebookConfiguration`
- group tests that read / write `static`s (`ScopeStorage` and `ViewEngines`) together
 - one `[Xunit.Collection]` for all readers and writers
  - won't run in parallel, avoiding reads while writers have messed up the state
 - `IDisposable` for all test classes that don't already reliably clean up their writes
- enable the subset of above tests that were disabled due to previous use of `static`s
- tests sometimes hang with loads of threads stuck in `BackgroundParser.MainThreadState.GetParcel()`
@dnfclas
Copy link

dnfclas commented Aug 26, 2016

Hi @dougbu, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

using Microsoft.TestCommon;
using Moq;

namespace Microsoft.Web.Helpers.Test
{
public class FacebookTest
[Xunit.Collection("Uses ScopeStorage or ViewEngines.Engines")]
Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately CollectionAttribute is found elsewhere in xUnit.net using the exact type and, worse, it's sealed. Can't expose anything like it in the Microsoft.TestCommon namespace. Also can't add a using Xunit without causing many ambiguous references.

@dougbu
Copy link
Member Author

dougbu commented Aug 26, 2016

As usual, it's probably easier to review the individual commits. The middle commits are even easier if you ignore whitespace changes, adding ?w=1 to the URL.

@dougbu
Copy link
Member Author

dougbu commented Aug 26, 2016

Thanks @rynowak! Also many thanks to @pranavkm @NTaylorMullen and @ajaybhargavb for their help with the other parts of this massive 4-part change.

@dougbu
Copy link
Member Author

dougbu commented Aug 26, 2016

Oops, I meant just "the other parts of this one".

}
}

return string.Join(Environment.NewLine, results.ToArray());
Copy link
Member

Choose a reason for hiding this comment

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

nit: ToArray isn't needed here, it can work on IEnumerable<>

@rynowak
Copy link
Member

rynowak commented Aug 26, 2016

:shipit:

using (var mockResult = new MockAsyncResult())
{
// Act
ActionResult result = BeginInvokeActionMethodWithFiltersTester(() => mockResult, action, checkBegin: true, checkEnd: true, filters: actionFilter);
Copy link
Member Author

Choose a reason for hiding this comment

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

Getting rid of the "helper" overload turned out to be a mistake. I'll instead add an IAsyncResult parameter to the old overload. That'll reduce the amount of churn in this class.

@dougbu
Copy link
Member Author

dougbu commented Aug 26, 2016

🆙📅

@rynowak I'd appreciate your eyes on the new commit.

@dougbu
Copy link
Member Author

dougbu commented Aug 27, 2016

🆙📅 replaced last commit to reduce AsyncControllerActionInvokerTest churn.

- reduce work done within inner loop
- add messages for `Exception`s this class `throw`s
- release acquired `Mutex`es
- reduce timeout for acquiring global `Mutex`
- need xUnit.net AppDomain support in console runs but not in VS runs
 - setting in `WebStack.xunit.targets` overrides `xunit.runner.json` default
 - somewhat avoids issues such as xunit/xunit#353 and xunit/xunit#947
- pre-enumerating `[Theory]`s also slows down test discovery and execution
 - remains a good idea to leave the test execution engine running
@dougbu
Copy link
Member Author

dougbu commented Aug 29, 2016

🆙📅 added a couple more workarounds and perf improvements

@rynowak
Copy link
Member

rynowak commented Aug 29, 2016

:shipit:

@dougbu
Copy link
Member Author

dougbu commented Aug 29, 2016

  • 2d8f677; Update test projects to latest stable xUnit.net packages and nuget.exe

Additions:

  • 29a8fee; Cleanup use of static state
  • 2e98bc0; Don't leak ManualResetEventSlims or Threads waiting on them
  • 0c3cbd2; Address frequent PortReserver failures
  • 890f4df; Improve speed of VS test runs

@dougbu dougbu closed this Aug 29, 2016
@dougbu dougbu deleted the dougbu/xunit.latest.11 branch August 29, 2016 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants