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

Added string compare test utility #46

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

g1n93r
Copy link

@g1n93r g1n93r commented Mar 31, 2021

I added string compare test utility ASSERT_STRSTR and ASSERT_NOT_STRSTR. I found it really usefull for testing json string output.

Enjoy!

@bvdberg
Copy link
Owner

bvdberg commented Apr 1, 2021

There are some issues i see:

  • cointain -> contain
  • why do you check the combinations of hackstack + needle, where one can be NULL? Why not force both of them to be non-NULL?

@g1n93r
Copy link
Author

g1n93r commented Apr 1, 2021

There are some issues i see:

  • cointain -> contain
  • why do you check the combinations of hackstack + needle, where one can be NULL? Why not force both of them to be non-NULL?

You are right! I fixed it so :
ASSERT_NOT_STRSTR(NULL, ""); => PASS
ASSERT_NOT_STRSTR("", NULL); => PASS
ASSERT_NOT_STRSTR(NULL, NULL); => PASS
ASSERT_STRSTR(NULL, ""); => FAIL
ASSERT_STRSTR("", NULL); => FAIL
ASSERT_STRSTR(NULL, NULL); => FAIL
Both ASSERT_NOT_STRSTR(NULL, NULL) and ASSERT_STRSTR(NULL, NULL) are bit ambiguous as one could argue that NULL does contain NULL.
Would you make a special case for those?

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.

2 participants