-
Notifications
You must be signed in to change notification settings - Fork 723
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
StringAssert.IsNullOrEmpty overloads #4662
Comments
The StringAsserts are part of the legacy/classic assertions. Although we don't expect to do anything more with these, what you suggest can be added, as they seem to be just additions, causing no breaking changes or changes to existing assertions. All the StringAssert methods are implemented in terms of the constraint model, like: public static void StartsWith(string expected, string actual, string message, params object?[]? args)
{
Assert.That(actual, Does.StartWith(expected), () => ConvertMessageWithArgs(message, args));
} And this is the way your suggestions should be implemented too. Suggestion 1 and 3 can easily be added, as they are just simple translations like: Assert.That(somestring,Is.Not.Null.And.Not.Empty); PS: Have you considered using the constraint syntax instead? No 2 and 4 include whitespace, and we currently don't have a constraint for whitespace. (We could probably have that, as it is frequently used in code. ). A solution would be to use the Regex constraint. We do have an open issue on this, see #3918. Perhaps due time to fix this @stevenaw @manfred-brands ? The team will probably not do this (except for creating a whitespace constraint or modifier at some time), but we are very open to receive a Pull Request for StringAsserts. That PR also has to contain unit tests for the asserts. It should be easy to add, and placed along with the other string assert tests. Since this adds functionality it has to go along with the same assertions being documented in the docs repo, under the section for StringAssert. |
@OsirisTerje I wouldn't mind adding a |
@manfred-brands Awesome if you implement that! How should the syntax be ? There are different alternatives, and we should try to be as close as we can to the constraint design. There might be more than a few scenarios. Some random thoughts: Is.EqualTo(sometring).IgnoreWhiteSpace() // Ignores all whitespace in string or Is.EqualTo(sometring).WhiteSpace.IgnoreAll // Looks a bit weird to me, but doesnt need multiple variations of WhiteSpace, only modifiers. so could also be: Is.EqualTo(sometring).WhiteSpace(IgnoreAll) Is.EqualTo(somestring).TrimWhiteSpace() // Ignore leading and ending whitespace or Is.EqualTo(somestring).WhiteSpace().Trim() or as above include as parameter Assert.That(sometring,Is.WhiteSpace() // string consist only of whitespace and it should be possible to write |
@OsirisTerje See #4664 |
Awesome! Looks very good! |
Thanks, I will try and work on this when I have free time, hopefully over the next few months. |
I like the
StringAssert
class because it provides more informative error messages over just doing something likeAssert.True()
. I'd like to suggest adding a few overloads to theStringAssert
class:These are very common things to want to check and right now the best way to do it is with
Assert.True()
which does not provide informative error messages by default.The text was updated successfully, but these errors were encountered: