[WIP] Migrate to Xunit 2.0 #895

nulltoken · 2015-01-03T19:19:15Z

This depends on #894.

Some things are still missing (eg. [SkippableFact]), but this reduced the time to run the full test suite from 01.41 down to 00:32 on my box. Promising!

Therzok · 2015-01-03T19:21:36Z

LibGit2Sharp.Tests/UnstageFixture.cs

@@ -106,7 +106,7 @@ public void CanUnstageUnknownPathsWithLaxUnmatchedExplicitPathsValidation(string
            {
                Assert.Equal(currentStatus, repo.RetrieveStatus(relativePath));

-                Assert.DoesNotThrow(() => repo.Unstage(relativePath, new ExplicitPathsOptions() { ShouldFailOnUnmatchedPath = false }));
+                repo.Unstage(relativePath, new ExplicitPathsOptions() { ShouldFailOnUnmatchedPath = false });


Assert.DoesNotThrow disappeared from Xunit 2.0. The rationale behind this being "No line line should throw, otherwise the test would fail". More details at xunit/xunit#188

What @nulltoken said - just let the test fail, don't wrap it...

nulltoken · 2015-01-03T19:59:40Z

Hmmm... I haven't updated the AppVeyor build script yet which explains the failure.

However, the Travis CI failure looks more suspicious. @Therzok Any idea?

nulltoken · 2015-01-03T20:02:41Z

FWIW, regarding the (current) lack of [SkippableFact], if any Xunit 2.0 wizard happen to feel in a helping mood, I've asked for some help over at StackOverFlow 🙏

Therzok · 2015-01-03T20:13:48Z

Yeah, it's pretty simple. It's a reflection resolution issue. You need to copy the xunit dlls in the output directory of the tests.

Therzok · 2015-01-03T20:14:59Z

.nuget/packages.config

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>


What's this file doing here?

It's installed by the test runner NuGet package

Therzok · 2015-01-03T20:22:42Z

Also, the travis failure seems related to this.

nulltoken · 2015-01-03T20:29:46Z

Also, the travis failure seems related to this.

Good 👀! From his last comment @bradwilson seems a little bit lost regarding how to fix this issue. Do you think you could lend him a hand?

nulltoken · 2015-01-03T20:33:02Z

LibGit2Sharp.Tests/app.config

+    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
+      <dependentAssembly>
+        <assemblyIdentity name="xunit" publicKeyToken="8d05b1bb7a6fdb6c" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-1.9.2.1705" newVersion="1.9.2.1705" />


Hmmm... This redirect looks fishy.

Therzok · 2015-01-03T20:41:15Z

The typeload exception can occur for a variety of reasons:

Exe having the same name as the dll, and it chooses the exe over the dll.
Architecture mismatch between the application and the library. (x64 -> x86 for example)
Other reasons I have no idea of.

It's most likely something in the deployment step of XUnit that changed between v1 and v2.

Therzok · 2015-01-03T20:42:25Z

LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj

-      <HintPath>..\Lib\xUnit\xunit.extensions.dll</HintPath>
+    <Reference Include="xunit.assert">
+      <HintPath>..\packages\xunit.assert.2.0.0-beta5-build2785\lib\portable-net45+aspnetcore50+win+wpa81+wp80+monoandroid+monotouch10\xunit.assert.dll</HintPath>
+      <Private>True</Private>


Why is this private and the rest are not?

I'm not sure... I'm going to drop this and see if that fails.

nulltoken · 2015-01-04T11:07:56Z

I've opened an issue in the Xunit.Net repository: How can one reimplement [SkippableFact] and [SkippableTheory] in Xunit2.0?

nulltoken · 2015-01-04T19:32:56Z

Yeah! I've succeeded in making Xunit 2.0 run on AppVeyor

nulltoken · 2015-01-05T15:58:58Z

@FeodorFitsner Thanks a lot for making xUnit on AppVeyor such a nice experience with xunit/xunit#117 !

Running #894 (with xUnit 1.9.x) produces this build log and run in 281.340s.

Whereas, running this WIP PR (built-on top on the previous one and only making the necessary changes to switch to xUnit 2.0) produces this build log and run in 394.853s.

However, I'm slightly puzzled regarding the difference in run times. I was (foolishly?) expecting xUnit 2.0 to be actually faster, not slower.

Any idea?

FeodorFitsner · 2015-01-05T17:44:03Z

@nulltoken That's a really good question. It could have been communication lags of pushing test results to real-time build console. However, I ran simple tests for xUnit 1.9.2 and 2.0.0 and they seem to be approx. equal:
xunit 1.9.2: https://ci.appveyor.com/project/FeodorFitsner/appvyr-xunit-tests
xunit 2.0.x: https://ci.appveyor.com/project/FeodorFitsner/xunit-20-tests

Build workers have the same build2785 version installed.

dahlbyk · 2015-01-12T00:56:46Z

I've started hacking around at a replacement for InconclusiveIf(...): xunit/xunit#250 (comment).

nulltoken · 2015-01-12T13:50:04Z

@dahlbyk 😻

Therzok · 2015-03-05T21:59:46Z

mono/mono#1618

Therzok · 2015-03-13T13:00:42Z

@nulltoken any way to trigger a master build of mono to test xunit 2 with it?

nulltoken · 2015-03-13T13:20:38Z

@Therzok Would there a way to do this by patching https://github.com/libgit2/libgit2sharp/blob/vNext/CI/travis.osx.install.deps.sh ? If you find out, feel free to push to this branch.

nulltoken · 2015-07-06T20:59:18Z

Temporarily requires https://www.myget.org/F/xunit/ as an additional NuGet feed. (cf. xunit/samples.xunit@70bc7e0#commitcomment-12022759)

shiftkey · 2015-07-06T21:52:02Z

@nulltoken

Temporarily requires https://www.myget.org/F/xunit/ as an additional NuGet feed

you could add a NuGet.config to the project root to codify this requirement

e.g. https://github.com/xunit/xunit/blob/master/NuGet.Config

nulltoken · 2015-07-06T21:59:37Z

@shiftkey Inetresting! Any example how I could pass this config to the nuget restore command?

shiftkey · 2015-07-06T22:06:12Z

@nulltoken if you're launching it from the repository root, it should work fine as per the docs

NuGet first loads NuGet.config from the default location, then loads any file named NuGet.config starting from the root of the current drive and ending in the current directory.

Current directory is defined as:

Directory from which NuGet.exe is invoked (command line scenario).

Directory in which the current solution was loaded from (Visual Studio scenario).

But I haven't verified this behaviour so perhaps it's incorrect.

Otherwise, you can pass in the config file explicitly:

The default configuration file can be changed through -ConfigFile option. For example, "-ConfigFile c:\my.config" means using file c:\my.config instead of %APPDATA%\NuGet\NuGet.Config as the default configuraion file.

bording · 2015-07-06T22:24:07Z

FYI, when I was first getting the native binaries nuget package stuff going, I had tried to add a temporary myget feed via the nuget.config, and for some reason I ran into problems getting it to restore properly on Travis builds. Instead of investigating why it wasn't working, I just put the temp package directly on nuget,org.

Anyway, it's still worth giving it a try to see if it works correctly this time.

shiftkey · 2015-07-06T22:25:44Z

FYI, when I was first getting the native binaries nuget package stuff going, I had tried to add a temporary myget feed via the nuget.config, and for some reason I ran into problems getting it to restore properly on Travis builds.

@bording interesting. Was this with or without specifying the -ConfigFile option?

bording · 2015-07-06T22:31:41Z

I didn't try the -ConfigFile option at the time, since nuget restore should work without it and didn't need it on my local windows machine. I only remember it failing on the Travis builds.

shiftkey · 2015-07-06T22:33:27Z

@bording @nulltoken I think we've got enough impetus to try both options again - we should be able to see if it succeeds with either option.

bording · 2015-07-06T22:43:25Z

Yes, we definitely should be able to get it working, especially since it makes sense to spend the time now.

Just wanted to make sure people were aware that it might require some tweaking to make everything play nicely together!

shiftkey · 2015-07-06T22:45:01Z

@bording 👍

bradwilson · 2015-07-07T03:12:54Z

@bording said:

for some reason I ran into problems getting it to restore properly on Travis builds.

Note that the file name is case sensitive on Unix systems, but not on Windows systems. It should always be named NuGet.Config if you intend to build from non-Windows machines.

bording · 2015-07-07T03:19:09Z

Note that the file name is case sensitive on Unix systems, but not on Windows systems. It should always be named NuGet.Config if you intend to build from non-Windows machines.

That has to be the issue I ran into then, because I doubt I named it that way when I tried it!

nulltoken · 2015-07-07T05:18:34Z

I've added a NuGet.Config file and all the packages were successfully installed.

However, we hit a failure on Travis

        xUnit.net MSBuild runner (64-bit .NET 4.0.30319.17020)
          Discovering: LibGit2Sharp.Tests
/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild: error : System.TypeLoadException: Could not load type 'Xunit.Sdk.XunitTestFrameworkDiscoverer, xunit.execution.desktop, Version=2.1.0.3042, Culture=neutral, PublicKeyToken=8d05b1bb7a6fdb6c'.
/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild: error :   at (wrapper managed-to-native) System.Type:internal_from_name (string,bool,bool)
/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild: error :   at System.Type.GetType (System.String typeName, Boolean throwOnError) [0x00000] in <filename unknown>:0 
/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild: error :   at System.Runtime.Remoting.ObjRef..ctor (System.String typeName, System.String uri, IChannelInfo cinfo) [0x00000] in <filename unknown>:0 
/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild: error :   at System.Runtime.Remoting.Messaging.CADMessageBase.UnmarshalArgument (System.Object arg, System.Collections.ArrayList args) [0x00000] in <filename unknown>:0 
/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild: error :   at System.Runtime.Remoting.Messaging.CADMethodReturnMessage.GetReturnValue (System.Collections.ArrayList args) [0x00000] in <filename unknown>:0 
/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild: error :   at System.Runtime.Remoting.Messaging.MethodResponse..ctor (IMethodCallMessage msg, System.Runtime.Remoting.Messaging.CADMethodReturnMessage retmsg) [0x00000] in <filename unknown>:0 
/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild: error :   at System.Runtime.Remoting.Channels.CrossAppDomainSink.SyncProcessMessage (IMessage msgRequest) [0x00000] in <filename unknown>:0 
    Task "xunit" execution -- FAILED
    Done building target "Test" in project "/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild".-- FAILED
Done building project "/home/travis/build/libgit2/libgit2sharp/CI/build.msbuild".-- FAILED

AppVeyor is also complaing, but I'm unsure why

  LibGit2Sharp.Tests -> C:\projects\libgit2sharp\LibGit2Sharp.Tests\bin\Release\LibGit2Sharp.Tests.dll
  Copying file from "obj\Release\LibGit2Sharp.Tests.pdb" to "bin\Release\LibGit2Sharp.Tests.pdb".
Done Building Project "C:\projects\libgit2sharp\LibGit2Sharp.Tests\LibGit2Sharp.Tests.csproj" (default targets).
Done Building Project "C:\projects\libgit2sharp\LibGit2Sharp.sln" (default targets).

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:01:53.86
cov-build.exe : 405 C# compilation units (99%) are ready for analysis
At line:1 char:1
+ & cov-build.exe --dir cov-int msbuild "$Env:APPVEYOR_BUILD_FOLDER\LibGit2Sharp.s ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (405 C# compilat...dy for analysis:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError

The cov-build utility completed successfully.

Command executed with exception: 405 C# compilation units (99%) are ready for analysis

When locally ran, one of the tests started to crash. This last issue seems unrelated to this PR and may awfully reminds me of #1091

"F:\libgit2sharp\CI\build.msbuild" (default target) (1) ->
(Test target) ->
  F:\libgit2sharp\CI\build.msbuild(40,5): error : LibGit2Sharp.Tests.StashFixture.StashCallsTheCallback: System.NullReferenceException : Object reference not set to an instance of an object.

    6 Warning(s)
    1 Error(s)

bording · 2015-07-07T05:47:01Z

@nulltoken Looks like that TypeLoadException might still be a mono problem with xUnit 2.0:

xunit/xunit#158

People are reporting still running into it on 4.0.2 😦

nulltoken · 2015-07-07T05:59:07Z

@bording You're right. I've seen this and (sneakily 😉) left this out for @Therzok to see.

However, I'm a bit puzzled by the AppVeyor failure... The Coverity build has never caused any issue before. Disabling it seems to resolve the issue, but I'm not sure to understand what the relation with xUnit 2.0 is.

/cc @FeodorFitsner @dakshesh-coverity

Any idea?

Temporarily disables Coverity analysis to troubleshoot AppVeyor issues - See https://ci.appveyor.com/project/libgit2/libgit2sharp/build/1156/job/sh3k52999pllnoic

nulltoken · 2015-08-23T15:29:32Z

@nulltoken Looks like that TypeLoadException might still be a mono problem with xUnit 2.0:

@Therzok ^^

However, I'm a bit puzzled by the AppVeyor failure... The Coverity build has never caused any issue before. Disabling it seems to resolve the issue, but I'm not sure to understand what the relation with xUnit 2.0 is.

@bradwilson f017882 passes on Windows. It's parent doesn't. Any idea?

Therzok · 2015-08-23T15:31:39Z

@nulltoken I think mono 4.3 with -noappdomain should work.

akoeplinger · 2015-08-23T16:01:12Z

@Therzok Mono >= 4.2 should work even with appdomain as afaik all the bugs where fixed. You can try the weekly builds.

bradwilson · 2015-08-24T00:30:28Z

Right, -noappdomain should only be needed on broken Mono.

carlosmn · 2016-04-20T12:23:53Z

We've gone with 1.9 and a 2.0 runner which works everywhere. Maybe eventually we'll want to move to 2.0 for everything, but that'd mean dropping older mono support.

coveralls · 2017-03-21T06:31:24Z

Changes Unknown when pulling 35d6833 on ntk/xunit20 into ** on vNext**.

Therzok reviewed Jan 3, 2015
View reviewed changes

nulltoken force-pushed the ntk/xunit20 branch from 4e4ef84 to 9d3a3d5 Compare January 3, 2015 19:49

Therzok reviewed Jan 3, 2015
View reviewed changes

nulltoken reviewed Jan 3, 2015
View reviewed changes

Therzok reviewed Jan 3, 2015
View reviewed changes

nulltoken changed the title ~~[WIP] Migrate tpo Xunit 2.0~~ [WIP] Migrate to Xunit 2.0 Jan 3, 2015

nulltoken force-pushed the ntk/xunit20 branch 3 times, most recently from 68560fd to df6f3e4 Compare January 4, 2015 18:51

nulltoken force-pushed the ntk/xunit20 branch 2 times, most recently from 4e439e6 to c2b0385 Compare January 4, 2015 21:54

nulltoken force-pushed the ntk/xunit20 branch from 9d9bf10 to 33f1c1f Compare January 5, 2015 16:55

nulltoken force-pushed the ntk/xunit20 branch 2 times, most recently from 63f6aa7 to 68f035a Compare January 17, 2015 08:08

nulltoken mentioned this pull request Jul 6, 2015

Certificate validation callback #1134

Merged

2 tasks

nulltoken force-pushed the ntk/xunit20 branch from 520ef6f to 18d974c Compare July 6, 2015 20:57

nulltoken force-pushed the ntk/xunit20 branch 2 times, most recently from 59535a7 to 886ca8f Compare August 23, 2015 14:37

nulltoken added 2 commits August 23, 2015 17:25

Upgrade to xUnit 2.0

07426ef

DO NOT MERGE ME

35d6833

Temporarily disables Coverity analysis to troubleshoot AppVeyor issues - See https://ci.appveyor.com/project/libgit2/libgit2sharp/build/1156/job/sh3k52999pllnoic

nulltoken force-pushed the ntk/xunit20 branch from 1d6727b to 35d6833 Compare August 23, 2015 15:25

carlosmn closed this Apr 20, 2016

bording deleted the ntk/xunit20 branch April 21, 2019 02:25

[WIP] Migrate to Xunit 2.0 #895

[WIP] Migrate to Xunit 2.0 #895

Uh oh!

Conversation

nulltoken commented Jan 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nulltoken commented Jan 3, 2015

Uh oh!

nulltoken commented Jan 3, 2015

Uh oh!

Therzok commented Jan 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Therzok commented Jan 3, 2015

Uh oh!

nulltoken commented Jan 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Therzok commented Jan 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nulltoken commented Jan 4, 2015

Uh oh!

nulltoken commented Jan 4, 2015

Uh oh!

nulltoken commented Jan 5, 2015

Uh oh!

FeodorFitsner commented Jan 5, 2015

Uh oh!

dahlbyk commented Jan 12, 2015

Uh oh!

nulltoken commented Jan 12, 2015

Uh oh!

Therzok commented Mar 5, 2015

Uh oh!

Therzok commented Mar 13, 2015

Uh oh!

nulltoken commented Mar 13, 2015

Uh oh!

nulltoken commented Jul 6, 2015

Uh oh!

shiftkey commented Jul 6, 2015

Uh oh!

nulltoken commented Jul 6, 2015

Uh oh!

shiftkey commented Jul 6, 2015

Uh oh!

bording commented Jul 6, 2015

Uh oh!

shiftkey commented Jul 6, 2015

Uh oh!

bording commented Jul 6, 2015

Uh oh!

shiftkey commented Jul 6, 2015

Uh oh!

bording commented Jul 6, 2015

Uh oh!

shiftkey commented Jul 6, 2015

Uh oh!

bradwilson commented Jul 7, 2015

Uh oh!

bording commented Jul 7, 2015

Uh oh!

nulltoken commented Jul 7, 2015

Uh oh!

bording commented Jul 7, 2015

Uh oh!