Skip to content

Conversation

@lg2de
Copy link
Contributor

@lg2de lg2de commented Mar 8, 2020

This is a new extension to create assertions on TaskCompletionSource<T> (TCS).

This PR creates a breaking change, because for instances of TCS the assertions of the currently catching ObjectAssertions extension will not be available anymore.
But FA is going to version 6.0, so it should be acceptable.

Further, I tried to analyze the assertions which may get lost. Here is my answer to the question "Do we need such assertion?":

  • ObjectAssertions
    • Be/NotBe: No, why should we check equivalency?
    • BeEquivalentTo/NotBeEquivalentTo: No, why should we check equivalency?
  • ReferenceTypeAssertions
    • BeNull/NotBeNull: Unsure, I do not see any relevance.
    • BeSameAs/NotBeSameAs: Unsure, could be relevant if TCS is part of an API.
    • BeOfType/NotBeOfType: No, I do not see any relevance.
    • BeAssignableTo/NotBeAssignableTo: No, I do not see any relevance.
    • Match: No, I do not see any relevance.

Please help to answer these questions and check whether you agree.

While creating unit tests using the clock API I was a bit confused whether the methods CompletesBeforeTimeout and RunsIntoTimeout influences the behavior of DelayAsync. I found, it did not. So I introduced new methods and changed existing usages. Hope you agree.
If not, we should think, whether DelayAsync or even more the IClock API can be adopted to work similar to FakeClock.Wait. Maybe DelayAsync should be replaced by a WaitAsync returning boolean too.

lg2de added 2 commits March 8, 2020 16:30
* added new TaskCompletionSourceAssertions
* clearified the usage of the FakeClock
Copy link
Member

@dennisdoomen dennisdoomen left a comment

Choose a reason for hiding this comment

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

@lg2de
Copy link
Contributor Author

lg2de commented Mar 9, 2020

new specialized.md

In this file all assertions from Specialized namespace should be in, right?

@dennisdoomen
Copy link
Member

In this file all assertions from Specialized namespace should be in, right?

Well, if you add yours, we'll take it from there.

* set c# language version to 8.0
* add documentation for new feature
* code cleanup
@lg2de
Copy link
Contributor Author

lg2de commented Mar 21, 2020

Anything still missing here?

Copy link
Member

@jnyrup jnyrup left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I wanted check out the branch and fiddle a bit around with it as I'm not that familiar with using TaskCompletionSource.

@jnyrup jnyrup merged commit d0c8405 into fluentassertions:develop Mar 21, 2020
@dennisdoomen
Copy link
Member

Another gem by @lg2de 👌

@lg2de lg2de deleted the TaskCompletionSource branch March 22, 2020 14:17
@jnyrup
Copy link
Member

jnyrup commented Mar 27, 2020

@lg2de Forgot to complement you for the the thorough analysis of the consequences of introducing the new Should(this TaskCompletetionSource<T>) overload.
That makes reviewing much easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants