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 comparison mu_assert_string_eq #8

Merged
merged 3 commits into from Mar 4, 2017
Merged

Conversation

spencerbug
Copy link
Contributor

string comparison is a very useful feature for many tests.
This particular one also checks if the strings are null, and replaces those with "", before doing the comparison.

@soudy
Copy link

soudy commented Mar 5, 2016

+1, would save a lot of repetition in my tests

minunit.h Outdated
@@ -190,6 +191,29 @@ static void (*minunit_teardown)(void) = NULL;
}\
)

#define mu_assert_string_eq(expected, result) MU__SAFE_BLOCK(\
char* minunit_tmp_e;\
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be const char*

@spencerbug spencerbug closed this Jun 27, 2016
@spencerbug spencerbug reopened this Oct 20, 2016
@siu
Copy link
Owner

siu commented Oct 31, 2016

I would like to merge this pull request but the code is not 100% safe. I think I mentioned this earlier but the comment disappeared after the rebase: when dealing with macros one has to be careful when the arguments appear in the expanded body.

With the current implementation actual and expected appear more than once in the macro. If actual or expected are expressions with side effects it will have unexpected behavior from the caller side.

A concrete example, if result = "queue.popAndReturn()" which removes and returns the element on top of a queue. The macro will be expanded to (only expanding expected):

MU__SAFE_BLOCK(
    const char* minunit_tmp_e;
    const char* minunit_tmp_r;
    minunit_assert++;
    if (!queue.popAndReturn()) {
        minunit_tmp_e = "<null pointer>";
    } else {
        minunit_tmp_e = (queue.popAndReturn());
    }
    if (!result) {
        minunit_tmp_r = "<null pointer>";
    } else {
        minunit_tmp_r = (result);
    }
    if(strcmp(minunit_tmp_e, minunit_tmp_r)) {
        snprintf(minunit_last_message, MINUNIT_MESSAGE_LEN, "%s failed:\n\t%s:%d: '%s' expected but was '%s'", __func__, __FILE__, __LINE__, minunit_tmp_e, minunit_tmp_r);
        minunit_status = 1;
        return;
    } else {
        printf(".");
    }
)

I.e. it will potentially invoke popAndReturn twice and the logic would be wrong.

I suggest to assign const char* minunit_tmp_e = expected; and const char* minunit_tmp_r = result; at the beginning of the macro and use the temporary variables for all the comparisons.

@siu
Copy link
Owner

siu commented Mar 4, 2017

Looks good now, thanks for the contribution.

@siu siu merged commit a5e241b into siu:master Mar 4, 2017
@R-Gerard
Copy link

Documentation in the README was omitted.

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

4 participants