From 520233f73e997e2a3899271fbf670fedbdeb6718 Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Mon, 11 Apr 2022 13:53:40 +0300 Subject: [PATCH] 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 (?) --- cores/esp8266/WString.cpp | 53 ++++++++++-------------------- cores/esp8266/WString.h | 1 + tests/host/Makefile | 41 ++++++++++------------- tests/host/common/ArduinoCatch.cpp | 25 ++++++++++++-- tests/host/common/ArduinoCatch.hpp | 36 ++++++++++++++++++++ tests/host/common/noniso.c | 35 +++++++++----------- tests/host/core/test_string.cpp | 16 +++++---- 7 files changed, 119 insertions(+), 88 deletions(-) create mode 100644 tests/host/common/ArduinoCatch.hpp diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index f77ded194..26825266a 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -25,6 +25,8 @@ #include "WString.h" #include "stdlib_noniso.h" +#include + #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::digits]; out = utoa(value, buf, base); return out; @@ -47,12 +49,8 @@ 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::digits]; + out = itoa(value, buf, base); return out; } @@ -60,7 +58,7 @@ static String toString(int value, unsigned char base) { static String toString(unsigned int value, unsigned char base) { String out; - char buf[1 + 8 * sizeof(unsigned int)]; + char buf[1 + std::numeric_limits::digits]; out = utoa(value, buf, base); return out; @@ -69,12 +67,8 @@ 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::digits]; + out = ltoa(value, buf, base); return out; } @@ -82,7 +76,7 @@ static String toString(long value, unsigned char base) { static String toString(unsigned long value, unsigned char base) { String out; - char buf[1 + 8 * sizeof(unsigned long)]; + char buf[1 + std::numeric_limits::digits]; out = ultoa(value, buf, base); return out; @@ -93,12 +87,8 @@ 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::digits]; + out = lltoa(value, buf, sizeof(buf), base); return out; } @@ -106,21 +96,8 @@ static String toString(long long value, unsigned char base) { 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); - } - - return out; -} - -static String toString(float value, unsigned char decimalPlaces) { - String out; - - char buf[33]; - out = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf); + char buf[1 + std::numeric_limits::digits]; + out = ulltoa(value, buf, sizeof(buf), base); return out; } @@ -134,6 +111,10 @@ static String toString(double value, unsigned char decimalPlaces) { return out; } +static String toString(float value, unsigned char decimalPlaces) { + return toString(static_cast(value), decimalPlaces); +} + /*********************************************/ /* Constructors */ /*********************************************/ diff --git a/cores/esp8266/WString.h b/cores/esp8266/WString.h index 780c7fdf6..88efa0d1f 100644 --- a/cores/esp8266/WString.h +++ b/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; diff --git a/tests/host/Makefile b/tests/host/Makefile index 13c9feed5..7ed821ebb 100644 --- a/tests/host/Makefile +++ b/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 diff --git a/tests/host/common/ArduinoCatch.cpp b/tests/host/common/ArduinoCatch.cpp index 2a3404a93..530827897 100644 --- a/tests/host/common/ArduinoCatch.cpp +++ b/tests/host/common/ArduinoCatch.cpp @@ -14,6 +14,25 @@ */ #define CATCH_CONFIG_MAIN -#include -#include -#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::convert(String const& str) +{ + return toString(str); +} + +} // namespace Catch diff --git a/tests/host/common/ArduinoCatch.hpp b/tests/host/common/ArduinoCatch.hpp new file mode 100644 index 000000000..8cb81afb7 --- /dev/null +++ b/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 +#include + +#include + +#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 { + static std::string convert(const String&); +}; + +} // namespace Catch diff --git a/tests/host/common/noniso.c b/tests/host/common/noniso.c index eacb2b14f..5c4e14b30 100644 --- a/tests/host/common/noniso.c +++ b/tests/host/common/noniso.c @@ -65,29 +65,24 @@ char* itoa(int value, char* result, int base) *result = 0; return result; } - if (base != 10) + + unsigned uvalue; + char* out = result; + + // 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)) { - return utoa((unsigned)value, result, base); + *result++ = '-'; + uvalue = (unsigned)-value; + } + else + { + uvalue = (unsigned)value; } - char* out = result; - int quotient = abs(value); - - do - { - const int tmp = quotient / base; - *out = "0123456789abcdef"[quotient - (tmp * base)]; - ++out; - quotient = tmp; - } while (quotient); - - // Apply negative sign - if (value < 0) - *out++ = '-'; - - reverse(result, out); - *out = 0; - return result; + utoa(uvalue, result, base); + return out; } int atoi(const char* s) diff --git a/tests/host/core/test_string.cpp b/tests/host/core/test_string.cpp index 1a235ea92..92b37e427 100644 --- a/tests/host/core/test_string.cpp +++ b/tests/host/core/test_string.cpp @@ -13,12 +13,14 @@ all copies or substantial portions of the Software. */ -#include -#include -#include -#include +#include #include +#include +#include +#include +#include + 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::max(); + REQUIRE(str == "abcdeabcde9872147483647"); + str += std::numeric_limits::min(); REQUIRE(str == "abcdeabcde9872147483647-2147483648"); str += (unsigned char)69; REQUIRE(str == "abcdeabcde9872147483647-214748364869");