Add interface to compare signed byte-sized data #968

arstrube · 2016-05-18T11:31:51Z

I noticed that CppUTest currently makes no provision for comparing signed byte-sized values. In my experience, in embedded development, sint8_t is frequently used.

This is an attempt at providing the missing functionality.

arstrube · 2016-05-18T11:35:02Z

src/CppUTestExt/MockSupport_c.cpp

@@ -788,7 +788,7 @@ const void* returnConstPointerValueOrDefault_c(const void * defaultValue)

 void (*functionPointerReturnValue_c())()
 {
-    return actualCall->returnFunctionPointerValue();
+    return (void (*)()) actualCall->returnFunctionPointerValue();


The cast has nothing to do with this PR - I checked the new code against the Cl2000 and it requires this cast. But it is also currently broken because of added features, and not easy to fix, so I can take this one out here, but there won't be a (working) PR for the Cl2000.

coveralls · 2016-05-18T12:05:08Z

Coverage increased (+0.0007%) to 99.874% when pulling 70b1ce4 on arstrube:chars into c920723 on cpputest:master.

coveralls · 2016-05-19T11:25:23Z

Coverage increased (+0.0007%) to 99.874% when pulling b2697c3 on arstrube:chars into 7ea6f14 on cpputest:master.

basvodde · 2016-05-20T07:37:26Z

tests/SimpleStringTest.cpp

+{
+    SimpleString s(DecimalStringFrom(-5));
+    SimpleString s2(DecimalStringFrom(-5));
+    CHECK(s == s2);


This test doesn't really test much :)

It has many siblings :D

In my opinion, we would do well to change all of them to something like:

SimpleString s(DecimalStringFrom(-5); STRCMP_QUAL("-5", s.asCharString());

Any reason why these tests were written like this?

If you test the CHECK, it might be useful. But in this case, you test the DecimalStringFormat, then it isn't useful whatsoever :)

basvodde · 2016-05-20T07:39:17Z

Whats the difference between this and BYTES_EQUAL?

arstrube · 2016-05-20T08:03:47Z

BYTES_EQUAL compares bytes as characters, e.g. 'a' or '¶'.
CHARS_EQUAL compares bytes as numbers.

When your code is dealing with numeric (decimal) values, it is not helpful to be presented some weird characters when you get a failure ;-)

Actually, I still find two problems with it (the tests elegantly skip over that)

Is should be able to do:

expected <-3 0xfd>
but was  <-2 0xfe>

for signed values as well as

expected<255 0xff>
but was  <253 0xfe>

I am unhappy with the naming. But how to change that without braking everying? CHARS_EQUAL should do 'z' and BYTES_EQUAL should do <-1 0xff>, at least in my book.

basvodde · 2016-05-20T08:18:50Z

Why not have BYTES equal print the value, the hex and the character?

arstrube · 2016-05-20T08:19:35Z

Why indeed not :)
Sound like a much better solution to me.

arstrube · 2016-05-20T08:20:45Z

My code doesn't do unsigned char correctly though. Will need to fix that as well.

arstrube · 2016-05-20T08:41:49Z

Actually, I got confused. Sorry about that. BYTES_EQUAL does unsigned. That's the problem.

The solution I'm trying to find is being able to do both signed and unsigned comparisons properly for byte-sized data.

arstrube · 2016-05-20T10:02:47Z

@basvodde pls have another look. How could this still be improved?

coveralls · 2016-05-20T10:15:05Z

Coverage decreased (-0.05%) to 99.82% when pulling 34007b7 on arstrube:chars into 7ea6f14 on cpputest:master.

coveralls · 2016-05-20T10:15:05Z

Coverage decreased (-0.05%) to 99.82% when pulling a01f174 on arstrube:chars into 7ea6f14 on cpputest:master.

coveralls · 2016-05-20T10:24:03Z

Coverage decreased (-0.1%) to 99.767% when pulling 302899c on arstrube:chars into 7ea6f14 on cpputest:master.

coveralls · 2016-05-20T10:25:27Z

Coverage decreased (-0.05%) to 99.82% when pulling 34007b7 on arstrube:chars into 7ea6f14 on cpputest:master.

coveralls · 2016-05-20T12:17:28Z

Coverage increased (+0.0007%) to 99.874% when pulling 5d9f39b on arstrube:chars into 7ea6f14 on cpputest:master.

arstrube · 2016-05-20T12:18:24Z

I got some things mixed up in my comments. The current situation (before this PR) is:

StringFrom(char) will only print literal characters
BYTES_EQUAL will compare unsigned char, so signed values will be displayed wrong
The C interface will compare char as literal characters
Signed 8 bit data cannot be compared with proper failure messages

Normally, we have macros like "LONGS_EQUAL" vs. "UNSIGNED_LONGS_EQUAL.
Here, we have BYTES_EQUAL vs. SIGNED_BYTES_EQUAL, because BYTES_EQUAL was there first, and it does unsigned.

The C interface has two new macros: CHECK_EQUAL_C_UBYTE and CHECK_EQUAL_C_SBYTE. The C interface doesn't mirror all of the new macros we have for C++.

basvodde · 2016-05-21T05:13:57Z

Uhm, but now the output is not "num hex char" like mentioned?

arstrube · 2016-05-21T05:23:36Z

True. But it never included char, only in the C interface. So, if we do that, it will be like a new feature:

do it only for the C interface? How?
do it for BYTES_EQUAL (doesn't real make sense, it's not CHARS_EUAL)
doesn't make sense for SIGNED_BYTES_EQUAL at all (this is numeric by definition

I suggest to give it some more thought and, perhaps, add it later. It would make sense for the C macro though, in which case there would be no new macros.

basvodde · 2016-05-21T05:28:59Z

Uhm, well to me it is just 3 different representations of the same information. As sometimes showing it in a different representation will trigger an AH!, I think it is useful. I would do it for all of them, or perhaps only when it is in the printable ascii range :)

arstrube · 2016-05-21T17:58:16Z

Hmm. So SIGNED_BYTES_EQUAL and BYTES_EQUAL would then display:

expected <-3 0xfd 'X'>
but was  <-2 0xfe 'Y'>

expected<255 0xff 'Z'>
but was  <253 0xfe 'Y'>

where X, Y, and Z would be the actual screen representation of that character. Similar for the C interface (will take a look next week).

basvodde · 2016-05-22T06:25:33Z

Yes, and then we could check whether the character is printable or not, and not print it if it isn't.

arstrube · 2016-05-23T06:34:21Z

I don't have much time right now, and if I do add the character, I will need to change the tests.

Since I want to test negative numbers and 'large' unsigned bytes, I would have to limit printing to printable characters for reasonable tests.

Can we perhaps merge this 'as is', and add printing the character later? (I can add an issue for that).

coveralls · 2016-05-23T06:57:09Z

Coverage increased (+0.0007%) to 99.874% when pulling dd6902f on arstrube:chars into a032b73 on cpputest:master.

coveralls · 2016-05-23T08:07:01Z

Coverage increased (+0.0007%) to 99.874% when pulling dd6902f on arstrube:chars into a032b73 on cpputest:master.

arstrube · 2016-05-23T12:04:25Z

If I display the character along with numeric data, then

   CHECK_EQUAL_C_CHAR()

which displays a literal character, would be redundant.

   CHECK_EQUAL_C_UBYTE()
   CHECK_EQUAL_C_SBYTE()

would handle unsigned char and signed char, respectively. But we need to keep CHECK_EQUAL_C_CHAR(), because it's always been there.

Should we then do:

    CHECK_EQUAL_C_CHAR() // does signed char
    CHECK_EQUAL_C_BYTE() // does unsigned char

or leave all three (with CHECK_EQUAL_C_UBYTE() giving identical output to CHECK_EQUAL_C_CHAR()?

basvodde · 2016-05-23T13:10:00Z

include/CppUTest/UtestMacros.h

@@ -232,6 +232,18 @@
 #define BYTES_EQUAL_TEXT(expected, actual, text)\
    LONGS_EQUAL_TEXT((expected) & 0xff, (actual) & 0xff, text)


This one will need to be changed to one of the others. But you want to do that later, right?

arstrube reviewed May 18, 2016
View reviewed changes

arstrube force-pushed the chars branch from 70b1ce4 to b2697c3 Compare May 19, 2016 09:44

basvodde reviewed May 20, 2016
View reviewed changes

arstrube changed the title ~~Add interface to compare byte-sized data as decimals~~ Add interface to compare signed byte-sized data May 20, 2016

Add interface to compare byte-sized data as decimals

dd6902f

arstrube force-pushed the chars branch from 5d9f39b to dd6902f Compare May 23, 2016 06:33

arstrube closed this May 23, 2016

arstrube reopened this May 23, 2016

basvodde reviewed May 23, 2016
View reviewed changes

basvodde merged commit 53d55b6 into cpputest:master May 23, 2016

arstrube deleted the chars branch May 25, 2016 06:48

		@@ -232,6 +232,18 @@
		#define BYTES_EQUAL_TEXT(expected, actual, text)\
		LONGS_EQUAL_TEXT((expected) & 0xff, (actual) & 0xff, text)

Add interface to compare signed byte-sized data #968

Add interface to compare signed byte-sized data #968

Uh oh!

Conversation

arstrube commented May 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arstrube May 18, 2016

Choose a reason for hiding this comment

Uh oh!

coveralls commented May 18, 2016

Uh oh!

coveralls commented May 19, 2016

Uh oh!

basvodde May 20, 2016

Choose a reason for hiding this comment

Uh oh!

arstrube May 20, 2016

Choose a reason for hiding this comment

Uh oh!

arstrube May 20, 2016

Choose a reason for hiding this comment

Uh oh!

basvodde May 20, 2016

Choose a reason for hiding this comment

Uh oh!

basvodde commented May 20, 2016

Uh oh!

arstrube commented May 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

basvodde commented May 20, 2016

Uh oh!

arstrube commented May 20, 2016

Uh oh!

arstrube commented May 20, 2016

Uh oh!

arstrube commented May 20, 2016

Uh oh!

arstrube commented May 20, 2016

Uh oh!

coveralls commented May 20, 2016

Uh oh!

coveralls commented May 20, 2016

Uh oh!

coveralls commented May 20, 2016

Uh oh!

coveralls commented May 20, 2016

Uh oh!

coveralls commented May 20, 2016

Uh oh!

arstrube commented May 20, 2016

Uh oh!

basvodde commented May 21, 2016

Uh oh!

arstrube commented May 21, 2016

Uh oh!

basvodde commented May 21, 2016

Uh oh!

arstrube commented May 21, 2016

Uh oh!

basvodde commented May 22, 2016

Uh oh!

arstrube commented May 23, 2016

Uh oh!

coveralls commented May 23, 2016

Uh oh!

coveralls commented May 23, 2016

Uh oh!

arstrube commented May 23, 2016

Uh oh!

basvodde May 23, 2016

Choose a reason for hiding this comment

Uh oh!

arstrube May 23, 2016

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arstrube commented May 18, 2016 •

edited

Loading

arstrube commented May 20, 2016 •

edited

Loading