Skip to content
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

(Not)Be(Abstract|Sealed|Static) class assertions implemented #796

Merged
merged 4 commits into from
Mar 14, 2018

Conversation

melchiork
Copy link
Contributor

For #645
Assertions for types (abstract, sealed, static) added. They are based on C# understanding of those keywords, as in CLR they differ.

@jnyrup
Copy link
Member

jnyrup commented Mar 8, 2018

Looks good to me.

The only minor thing that caught me eyes was that:

  • NotBeSealed uses "to not be".
  • NotBeAbstract uses "not to be".
  • NotBeStatic uses "not to be".

We aren't entirely consistent in what we use.

  • 15 uses of "to not be"
  • 37 uses of "not to be".

 -because clause fixed according to the review
 -docs updated
@melchiork
Copy link
Contributor Author

@jnyrup Thanks, fixed. Also I've added two examples to the docs.

@jnyrup
Copy link
Member

jnyrup commented Mar 11, 2018

It just struck me that from a C# perspective the keywords sealed, abstract and static are only applicable to class like types, and not interfaces or structs.

Several assertions in TypeAssertions checks if the type is an interface.
One example is BeDerivedFrom as you cannot derive from an interface type, but implement it.

@dennisdoomen
Copy link
Member

So? You want to have some changea made to this PR?

@jnyrup
Copy link
Member

jnyrup commented Mar 12, 2018

Yes, I would like to have a check that the Subject type is not a valuetype or an interface or throw an InvalidOperationException with a descriptive message.

This is similar to what we do in ActionAssertions.FailIfSubjectIsAsyncVoid, XDocumentAssertions.HaveRoot and XDocumentAssertions.HaveElement

Calling typeof(StructType).IsSealed returns true, but that is from a CLR perspective.
The purpose of the type assertions seems to be from a C# perspective, i.e. if the type under assertion has the sealed keyword.

@melchiork
Copy link
Contributor Author

@jnyrup @dennisdoomen
Done, I've also check if the type is a Delegate.
I went with ArgumentException as similar was done in TypeAssertions.Implement and TypeAssertions.NotImplement

@jnyrup
Copy link
Member

jnyrup commented Mar 12, 2018

BeDerivedFrom and the alike uses ArgumentException as they're checking the argument.
As the Subject isn't an argument to the assertion method I prefer InvalidOperationException like in HaveRoot.

I really like your use of Theory tests.
Excellent job!

@melchiork
Copy link
Contributor Author

@jnyrup Done. Thanks for the reviews!

@dennisdoomen dennisdoomen merged commit 2fd254e into fluentassertions:master Mar 14, 2018
@jnyrup jnyrup mentioned this pull request Jun 29, 2019
88 tasks
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.

None yet

3 participants