test_dltctrlmsg: For some reasons using chars fails with int on some …

[LocutusOfBorg]

…architectures (such as arm* ppc64el, s390x)

Closes: #705

[vifactor]

I think, the problem is not here. I guess the byte layout generated by dlt-daemon on a big endian platform for the same message will be different, i.e. data-content for the same logLevel and traceStatus will be somewhat altered there.

Question: are you developing dlt-viewer on big-endian platform? If you are just using it there (and not developing), I do not think the tests matter.

[LocutusOfBorg]

I am building dlt-viewer on Debian, so for both LE and BE architectures, and I would like to execute tests for all archs, to avoid missing regressions...
This is the arch lists:
https://buildd.debian.org/status/package.php?p=dlt-viewer&suite=experimental

[vifactor]

tbh, I do not completely understand why it fails on BE. \ff is decimal -1 for int8_t type. Do you understand the fix?

[LocutusOfBorg]

not at all :)

[RubenGarcia]

On some platforms, char is signed and in some it is unsigned. You may have a weird cast -1 => 255 and then another cast to int, where the final comparision -1 == 255 fails..

[RubenGarcia]

On other platforms, chars are 16 bit, so comparing 0xff to -1 will also fail.

[vifactor]

@RubenGarcia thanks for the info. Do you have an idea how the test can be rewritten so that it remains valid both on BE and LE platforms?

[RubenGarcia]

The important thing is that you write down the types of the data that you receive, and understand the conceptual data types.
If the documentation says the thing is supposed to be an unsigned byte, then cast it to uint8_t and only compare it to 255.

I do not like EXPECT_EQ(ctx.logLevel, '\xff'); because that uses either char or wchar_t which you have no control over.

I also do not like EXPECT_EQ(ctx.logLevel, -1); because you don't have control over signedness.

Go to the documentation, verify what exact type ctx.logLevel is, and then use
EXPECT_EQ((type)ctx.logLevel, (type)(<value that the documentation says should be there>));
It would be best if type is platform agnostic, but since you cast both sides, it should work even if the type changes per-platform.

Don't remove EXPECT_EQ(ctx.traceStatus, '\xff');, but change it to what the documentation says it should have.

[bunty95]

Can this be merged into main branch?

[RubenGarcia]

Let's ensure tests succeed on all platforms before merging.

[LocutusOfBorg]

hello, ping?

[RubenGarcia]

Approved by mistake, redoing review.

[RubenGarcia]

Do not remove expect_eq lines and make sure that they match the documentation as requested.