search

bxparks / AUnit

Unit testing framework for Arduino platforms inspired by ArduinoUnit and Google Test. Used with EpoxyDuino for continuous builds.
MIT License
179 stars 16 forks source link

assert macros: fix bug with macros expansion #95

Closed hsaturn closed 1 year ago

bxparks commented 1 year ago

I'm going to close this PR because every time you add a commit to it, I get an email, it's too much. When you are more stable, and figured out what you want to upstream versus keeping in your fork, send me another PR.

hsaturn commented 1 year ago

Hoo sorry for this. I won't reopen this one as I don't want to annoy you in any way. It's up to you to decide if you want to enclose arguments with parenthesis. Which is a good habbit (https://wiki.sei.cmu.edu/confluence/display/c/PRE01-C.+Use+parentheses+within+macros+around+parameter+names).

Best regards

bxparks commented 1 year ago

The problem with this PR is that it now has 10 commits. I wouldn't want to accept some of them, for example, the ANSI terminal escape sequences, because they interfere with automated testing tools that monitor the output of AUnit on the serial port from the microcontrollers, and because it's another layer of complexity that I don't want to maintain.

With regards to extra parenthesis around macros, I am not seeing what the actual bug is. Why does:

#define FFF(x,y) f(x,y)

need to be written as:

#define FFF(x,y) f((x),(y))

?

Because as far as I can tell, no expression for x or y will produce buggy code. Even the comma-operator, because the comma expression must be wrapped in () to satisfy the C-preprocessor. In other words, we have to write FFF((a,b), y) instead of FFF(a,b,y) anyway.

If there is no actual bug, then this becomes a discussion over coding style, and those complex macros are already difficult to read and maintain, so I would prefer to avoid sprinkling unnecessary brackets around all the parameters if they are not needed.

hsaturn commented 1 year ago

I'll remove the parenthesis and see why I had to add them. (As I don't want to bother you, maybe I'll never do the test....) I would have personally not use macros but templates instead.

Best regards.

bxparks commented 1 year ago

As far as I am know, C++ templates don't have access to __FILE and __LINE__, and they can't capture the actual arguments inside the assertXxx() macros as c-strings, which is how the assertion failure messages are generated. See AssertVerboseMacros.h for the #arg1 and #arg2 parameters.