Test: fixing itoa implementation and clean-up of tests and test Makefile (#8531)

* Test: fixing itoa implementation, clean-up of tests and test runner

Update itoa to be the same as newlib, fixing edgecase of abs(INT_MIN)
Update WString.cpp:toString() integer conversions to use noniso funcs
Remove legacy gcc versions from Makefile and allow overrides
Don't fallback to c11 and c++11, source cannot support that

* CXX and CC are make predefined values, assuming ?= does not work (?)

Author: mcspr

cores/esp8266/WString.cpp

@@ -25,6 +25,8 @@
 #include "WString.h"
 #include "stdlib_noniso.h"
 
+#include <limits>
+
 #define OOM_STRING_BORDER_DISPLAY           10
 #define OOM_STRING_THRESHOLD_REALLOC_WARN  128
 
@@ -38,7 +40,7 @@
 static String toString(unsigned char value, unsigned char base) {
     String out;
 
-    char buf[1 + 8 * sizeof(unsigned char)];
+    char buf[1 + std::numeric_limits<unsigned char>::digits];
     out = utoa(value, buf, base);
 
     return out;
@@ -47,20 +49,16 @@ static String toString(unsigned char value, unsigned char base) {
 static String toString(int value, unsigned char base) {
     String out;
 
-    char buf[2 + 8 * sizeof(int)];
-    if (base == 10) {
-        out.concat(buf, sprintf(buf, "%d", value));
-    } else {
-        out = itoa(value, buf, base);
-    }
+    char buf[2 + std::numeric_limits<int>::digits];
+    out = itoa(value, buf, base);
 
     return out;
 }
 
 static String toString(unsigned int value, unsigned char base) {
     String out;
 
-    char buf[1 + 8 * sizeof(unsigned int)];
+    char buf[1 + std::numeric_limits<unsigned int>::digits];
     out = utoa(value, buf, base);
 
     return out;
@@ -69,20 +67,16 @@ static String toString(unsigned int value, unsigned char base) {
 static String toString(long value, unsigned char base) {
     String out;
 
-    char buf[2 + 8 * sizeof(long)];
-    if (base == 10) {
-        out.concat(buf, sprintf(buf, "%ld", value));
-    } else {
-        out = ltoa(value, buf, base);
-    }
+    char buf[2 + std::numeric_limits<long>::digits];
+    out = ltoa(value, buf, base);
 
     return out;
 }
 
 static String toString(unsigned long value, unsigned char base) {
     String out;
 
-    char buf[1 + 8 * sizeof(unsigned long)];
+    char buf[1 + std::numeric_limits<unsigned long>::digits];
     out = ultoa(value, buf, base);
 
     return out;
@@ -93,30 +87,22 @@ static String toString(unsigned long value, unsigned char base) {
 static String toString(long long value, unsigned char base) {
     String out;
 
-    char buf[2 + 8 * sizeof(long long)];
-    if (base == 10) {
-        out.concat(buf, sprintf(buf, "%lld", value));
-    } else {
-        out = lltoa(value, buf, sizeof(buf), base);
-    }
+    char buf[2 + std::numeric_limits<long long>::digits];
+    out = lltoa(value, buf, sizeof(buf), base);
 
     return out;
 }
 
 static String toString(unsigned long long value, unsigned char base) {
     String out;
 
-    char buf[1 + 8 * sizeof(unsigned long long)];
-    if (base == 10) {
-        out.concat(buf, sprintf(buf, "%llu", value));
-    } else {
-        out = ulltoa(value, buf, sizeof(buf), base);
-    }
+    char buf[1 + std::numeric_limits<unsigned long long>::digits];
+    out = ulltoa(value, buf, sizeof(buf), base);
 
     return out;
 }
 
-static String toString(float value, unsigned char decimalPlaces) {
+static String toString(double value, unsigned char decimalPlaces) {
     String out;
 
     char buf[33];
@@ -125,13 +111,8 @@ static String toString(float value, unsigned char decimalPlaces) {
     return out;
 }
 
-static String toString(double value, unsigned char decimalPlaces) {
-    String out;
-
-    char buf[33];
-    out = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf);
-
-    return out;
+static String toString(float value, unsigned char decimalPlaces) {
+    return toString(static_cast<double>(value), decimalPlaces);
 }
 
 /*********************************************/

cores/esp8266/WString.h

@@ -58,6 +58,7 @@ class String {
         String(const String &str);
         String(const __FlashStringHelper *str);
         String(String &&rval) noexcept;
+
         explicit String(char c) {
             sso.buff[0] = c;
             sso.buff[1] = 0;

tests/host/Makefile

@@ -12,30 +12,23 @@ RANLIB ?= ranlib
 
 MAKEFILE = $(word 1, $(MAKEFILE_LIST))
 
-CXX =  $(shell for i in g++-10  g++-9  g++-8  g++;  do which $$i > /dev/null && { echo $$i; break; } done)
-CC  =  $(shell for i in gcc-10  gcc-9  gcc-8  gcc;  do which $$i > /dev/null && { echo $$i; break; } done)
-GCOV = $(shell for i in gcov-10 gcov-9 gcov-8 gcov; do which $$i > /dev/null && { echo $$i; break; } done)
-$(warning using $(CXX))
-ifeq ($(CXX),g++)
-CXXFLAGS += -std=gnu++11
-else
-CXXFLAGS += -std=gnu++17
-endif
-ifeq ($(CC),gcc)
-CFLAGS += -std=gnu11
-else
-CFLAGS += -std=gnu17
-endif
+# Prefer named GCC (and, specifically, GCC10), same as platform.txt / platformio_build.py
+find_tool = $(shell for name in $(1) $(2); do which $$name && break; done 2>/dev/null)
+CXX  = $(call find_tool,g++-10,g++)
+CC   = $(call find_tool,gcc-10,gcc)
+GCOV = $(call find_tool,gcov-10,gcov)
 
-# I wasn't able to build with clang when -coverage flag is enabled, forcing GCC on OS X
-ifeq ($(shell uname -s),Darwin)
-CC  ?= gcc
-CXX ?= g++
-endif
-GCOV ?= gcov
+$(warning using $(CXX) and $(CC))
+
+GCOV     ?= gcov
 VALGRIND ?= valgrind
-LCOV ?= lcov --gcov-tool $(GCOV)
-GENHTML ?= genhtml
+LCOV     ?= lcov --gcov-tool $(GCOV)
+GENHTML  ?= genhtml
+
+# Board fild will be built with GCC10, but we have some limited ability to build with older versions
+# *Always* push the standard used in the platform.txt
+CXXFLAGS += -std=gnu++17
+CFLAGS += -std=gnu17
 
 ifeq ($(FORCE32),1)
 SIZEOFLONG = $(shell echo 'int main(){return sizeof(long);}'|$(CXX) -m32 -x c++ - -o sizeoflong 2>/dev/null && ./sizeoflong; echo $$?; rm -f sizeoflong;)
@@ -174,6 +167,8 @@ INC_PATHS += \
 		../../tools/sdk/lwip2/include \
 	)
 
+TEST_ARGS ?=
+
 TEST_CPP_FILES := \
 	fs/test_fs.cpp \
 	core/test_pgmspace.cpp \
@@ -236,7 +231,7 @@ CI:					# run CI
 doCI: build-info $(OUTPUT_BINARY) valgrind test gcov
 
 test: $(OUTPUT_BINARY)			# run host test for CI
-	$(OUTPUT_BINARY)
+	$(OUTPUT_BINARY) $(TEST_ARGS)
 
 .PHONY: clean
 clean: clean-lcov clean-objects

tests/host/common/ArduinoCatch.cpp

@@ -14,6 +14,25 @@
 */
 
 #define CATCH_CONFIG_MAIN
-#include <catch.hpp>
-#include <sys/time.h>
-#include "Arduino.h"
+#include "ArduinoCatch.hpp"
+
+std::ostream& operator<<(std::ostream& out, const String& str)
+{
+    out.write(str.c_str(), str.length());
+    return out;
+}
+
+namespace Catch
+{
+
+std::string toString(const String& str)
+{
+    return std::string(str.begin(), str.length());
+}
+
+std::string StringMaker<String>::convert(String const& str)
+{
+    return toString(str);
+}
+
+}  // namespace Catch

tests/host/common/ArduinoCatch.hpp

@@ -0,0 +1,36 @@
+/*
+ Arduino.cpp - Mocks for common Arduino APIs
+ Copyright © 2016 Ivan Grokhotkov
+
+ Permission is hereby granted, free of charge, to any person obtaining a copy
+ of this software and associated documentation files (the "Software"), to deal
+ in the Software without restriction, including without limitation the rights
+ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ copies of the Software, and to permit persons to whom the Software is
+ furnished to do so, subject to the following conditions:
+
+ The above copyright notice and this permission notice shall be included in
+ all copies or substantial portions of the Software.
+*/
+
+#include <Arduino.h>
+#include <sys/time.h>
+
+#include <ostream>
+
+#include "catch.hpp"
+
+// Since Catch does not know about Arduino types, help it out so we could have these displayed in the tests output
+
+std::ostream& operator<<(std::ostream&, const String&);
+
+namespace Catch {
+
+std::string toString(const String&);
+
+template<>
+struct StringMaker<String> {
+    static std::string convert(const String&);
+};
+
+} // namespace Catch

tests/host/common/noniso.c

@@ -65,29 +65,24 @@ char* itoa(int value, char* result, int base)
         *result = 0;
         return result;
     }
-    if (base != 10)
-    {
-        return utoa((unsigned)value, result, base);
-    }
 
-    char* out      = result;
-    int   quotient = abs(value);
+    unsigned uvalue;
+    char*    out = result;
 
-    do
+    // after this point we convert the value to unsigned and go to the utoa
+    // only base10 gets minus sign in the front, adhering to the newlib implementation
+    if ((base == 10) && (value < 0))
     {
-        const int tmp = quotient / base;
-        *out          = "0123456789abcdef"[quotient - (tmp * base)];
-        ++out;
-        quotient = tmp;
-    } while (quotient);
-
-    // Apply negative sign
-    if (value < 0)
-        *out++ = '-';
+        *result++ = '-';
+        uvalue    = (unsigned)-value;
+    }
+    else
+    {
+        uvalue = (unsigned)value;
+    }
 
-    reverse(result, out);
-    *out = 0;
-    return result;
+    utoa(uvalue, result, base);
+    return out;
 }
 
 int atoi(const char* s)

tests/host/core/test_string.cpp

@@ -13,12 +13,14 @@
  all copies or substantial portions of the Software.
  */
 
-#include <catch.hpp>
-#include <string.h>
-#include <WString.h>
-#include <limits.h>
+#include <ArduinoCatch.hpp>
 #include <StreamString.h>
 
+#include <string>
+#include <cstring>
+#include <limits>
+#include <climits>
+
 TEST_CASE("String::move", "[core][String]")
 {
     const char buffer[] = "this string goes over the sso limit";
@@ -117,8 +119,10 @@ TEST_CASE("String concantenation", "[core][String]")
     str += "bcde";
     str += str;
     str += 987;
-    str += (int)INT_MAX;
-    str += (int)INT_MIN;
+    REQUIRE(str == "abcdeabcde987");
+    str += std::numeric_limits<int>::max();
+    REQUIRE(str == "abcdeabcde9872147483647");
+    str += std::numeric_limits<int>::min();
     REQUIRE(str == "abcdeabcde9872147483647-2147483648");
     str += (unsigned char)69;
     REQUIRE(str == "abcdeabcde9872147483647-214748364869");