Redesigned AssertionScope and how it travels over chained calls #2539
Conversation
a517fec to
2f85480
Compare
|
Had a rather quick look at this and it looks very promising 🤩 |
Shall I continue with the real implementation? Or do you want to take a deeper look first? |
|
I'm wondering whether the new Assertion class in this redesign is a good name, given that we also have all the classes postfixed with Assertion.GetOrCreate()
.BecauseOf(because, becauseArgs)
.WithExpectation("Expected {context:collection} to be empty{reason}, ")
.Given(() => singleItemArray)
.ForCondition(subject => subject is not null)
.FailWith("but found <null>.")
.Then
.ForCondition(subject => subject.Length == 0)
.FailWith("but found at least one item {0}.", singleItemArray);I do like |
|
|
|
|
|
Assertion makes sense to me. AssertionBuilder does seem to map more closely to what it’s doing, though yes it isn’t strictly speaking a builder. How would this be used by external consumers, if at all? I have only really used Fluent Assertions via .Should(…) edit - reading the PR description more clearly, FluentAssertion is probably just fine since this seems to be a mostly internal API… |
|
If the conditions are evaluated right away I would think |
|
|
It's used both as the internal engine as well as by anybody that builds extensions. See also https://fluentassertions.com/extensibility/#building-your-own-extensions
It doesn't evaluate an assertion. You're building and executing a, potentially, chained assertion.
It is also not the context of an assertion. It's doing the assertion work. PS. For reference, I've updated the original question with a more elaborate example. |
|
Considering you already have |
|
|
What about This way, your chain would read almost like an English sentence assertThat
.BecauseOf(because, becauseArgs)
.WithExpectation("Expected {context:collection} to be empty{reason}, ")
.Given(() => singleItemArray)
.ForCondition(subject => subject is not null)
.FailWith("but found <null>.")
.Then
.ForCondition(subject => subject.Length == 0)
.FailWith("but found at least one item {0}.", singleItemArray); |
|
Something with 'Pipeline' or 'Predicate'? |
|
Hmm, I kind of starting to like |
fe5cb45 to
17fbaa6
Compare
adaec55 to
6d7b28b
Compare
3118f0b to
fedcaa8
Compare
Attempts to fix #2253
Also fixes #1502, #2647, #2664
AssertionScopeis only used by consumers as they do now and by certain APIs such asBeEquivalentToShouldmethod will create a new object calledAssertionChain(but I'm open to a better name) that contains the context of the assertion and a link to the scope. When no ambientAssertionScopeis available, it will be a new one that is created at that point. This is different from v6 where we create a new scope per call toExecute.Assertion.AssertionChaininstance will be explicitly passed to all the assertion classes (likeStringAssertions).AssertionScope.CurrentanymoreAssertionScopehave moved toAssertionChain.GenericCollectionAssertions.ContainSinglereturns a new version ofAndWhichConstraintthat takes a postfix such as[0]. Only when theWhichproperty of that intermediate class is used, the currentAssertionis updated with that postfix and registered on the current thread (usingAsyncLocal). This ensures that theShouldcall chained onWhichwill pick-up thatAssertionChaininstance and continue the assertion with the right caller identifier. See for exampleChaining_something_should_do_somethingClearExpectationsAssertionScopeas a way to group related assertions and its responsibilities as the internal assertion APISee also this wiki page.
TODO
AssertionScopeAssertionScopeAndWhichConstraintwithAndWhich(and then rename it back toAndWhichConstraint)AssertionScopereally is necessary inAssertionChainSpecs.*.specsDisposeor usingusingIt's CA2000, which we disabled in the tests .editorconfig
AssertionChainenAssertionScopeAssertionChain.AddCallerPrefixandReuseOnceWithExpectationExecute.Assertionwith code that usesAssertionChain.GetOrCreatein theShouldextension method and uses that instance throughout the assertionIMPORTANT
./build.sh --target spellcheckor.\build.ps1 --target spellcheckbefore pushing and check the good outcome