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
Redesign AssertionScope and how it travels over chained calls #2539
base: develop
Are you sure you want to change the base?
Redesign AssertionScope and how it travels over chained calls #2539
Conversation
Qodana for .NET6 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
bec8dea
to
a517fec
Compare
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 |
Qodana for .NET49 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
fe5cb45
to
17fbaa6
Compare
adaec55
to
6d7b28b
Compare
6d7b28b
to
037a377
Compare
be16e9a
to
3118f0b
Compare
3118f0b
to
fedcaa8
Compare
Attempts to fix #2253
AssertionScope
is only used by consumers as they do now and by certain APIs such asBeEquivalentTo
Should
method will create a new object calledAssertion
(but I'm open to a better name) that contains the context of the assertion and a link to the scope. When no ambientAssertionScope
is 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
.Assertion
instance will be explicitly passed to all the assertion classes (likeStringAssertions
).AssertionScope.Current
anymoreAssertionScope
will need to move toAssertion
.GenericCollectionAssertions.ContainSingle
returns a new version ofAndWhich
that takes a postfix such as[0]
. Only when theWhich
property of that intermediate class is used, the currentAssertion
is updated with that postfix and registered on the current thread (usingAsyncLocal
). This ensures that theShould
call chained onWhich
will pick-up thatAssertion
instance and continue the assertion with the right caller identifier. See for exampleChaining_something_should_do_something
ClearExpectations
AssertionScope
as a way to group related assertions and its responsibilities as the internal assertion APISee also this wiki page.
TODO
AssertionScope
ExceptionAssertions
creates new instances of itself?AssertionScope
IMPORTANT
./build.sh --target spellcheck
or.\build.ps1 --target spellcheck
before pushing and check the good outcome