Bump Json.NET and NUnit dependencies

[brianjmiller]

• Remove NUnit test adapter from nuget dependencies as the 2.x version is
  not compatible and 3.x is intended to be run with an adapter installed
  through other means (see new readme)
• Correct a failing unit test not compatible with 3.x

Replaces #21 since the deps have increased again since then and this has a slightly different unit test format.

[brianjmiller]

@oconnetf this still look okay to you? I bumped the deps to the new versions and had a little different take on the fixed unit test, also added a readme for adapter installation.

[oconnetf]

So this is preferable to Assert.That(() => UseActivityIdInvalidUri(), Throws.InstanceOf<System.UriFormatException>());?

It was suggested to me by @mlefoster that Assert.That was a more common NUnit 3 syntax.

[brianjmiller]

In cases where there is an assertion function specific to what the test is doing I was thinking the more specific variant ought to be used. Assert.That seems like a generic wrapper which makes sense for other cases. Assert.Throws Avoids InstanceOf, etc. I didn't find anything definitive but given 3.x had other breaking changes, they just added Assert.ThrowsAsync, and it is decently documented I think the more specific one is fine (or better).

[oconnetf]

I actually agree. I feel like @mlefoster might've even had some documentation justifying the approach he recommended. Unless he speaks up, feel free to merge.

[bscSCORM]

I'm 👍 on this but I'm going to wait a bit to see if we can get @mlefoster to weigh in.

[emmacadence]

https://github.com/nunit/docs/wiki/Assertions

In NUnit 3.0, assertions are written primarily using the Assert.That method, which takes constraint objects as an argument. We call this the Constraint Model of assertions.

In earlier versions of NUnit, a separate method of the Assert class was used for each different assertion. This Classic Model is still supported but since no new features have been added to it for some time, the constraint-based model must be used in order to have full access to NUnit's capabilities.

I mean, the answer to this question is nowhere near as important as "do we have good tests." But the docs seem pretty clear that NUnit developers are trying to move the NUnit community to Assert.That. It's as an explicit a statement as we can get, and it's really the only data point we have about what the NUnit community standards actually are. As a veteran of the Python 2/3 wars, community standards are something I'm kind of sensitive about.

I think the claim that "Assert.That is more generic and Assert.Throws is more specific" is incredibly misleading, because it's not about Assert.That versus Assert.Throws, it's about Assert.That versus the whole classic model, and Assert.That(xyz, Throws...) versus Assert.Throws.

If I had to sell you the value of the semantic difference, it's more that Assert.That allows you to write more more complicated asserts when you want to extend tests. With the constraint model, it's easier to say Assert.That(aFunction, Throws.InstanceOf<UriFormatException>().With.Message.Containing("activity ID")). (Such a thing is absolutely essential, I think, if your function could throw the same type of exception for different reasons.) You can have the same sorts of tests with Assert.Throws by having the exception returned and then checking the properties, but I think the constraint model is far more readable here: "Assert that a function throws an instance of a UriFormatException with a message that contains the string 'activity ID'."

[brianjmiller]

Ironically "but since no new features have been added to it for some time" is contradicted by the recent (in 3.2 this calendar year) addition of Assert.ThrowsAsync. See nunit/nunit#464 There are various discussions in there about Assert.That that all pretty much call it a workaround at least in the async case.

[oconnetf]

Small comment re: preferred syntax. Otherwise, 👍.

[brianjmiller]

@bscSCORM thoughts?

[bscSCORM]

So it seems that based on this discussion we probably should lean towards Assert.That , perhaps except in the case of ThrowsAsync, though there is disagreement on that as well... in any case, this seems good enough to merge and this discussion is useful for future reference.