From 03ea61a32a7dd9f4e1963bdaf7b7b61b3d8e84d2 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Fri, 27 Jul 2018 10:10:55 -0700 Subject: [PATCH] Add valgrind and string tests to host_tests (#4939) * Add valgrind and string tests to host_tests Valgrind identified an error that was causing intermittent failures in the host tests, and can check for subtle memory access and allocation bugs. Add it to the standard host_test suite, and any errors will cause test failure. Also start adding string tests, since two undefined behaviors have been found so far just by inspection. * Add additional String tests Looks like a possible bug in the concatenation operator, need to verify expected behavior. * Remove verbose from valgrind run No need to be so chatty on the test. Errors were a little hard to spot. Go to normal verbosity. * Add lcov and more string tests LCOV and genhtml can generate nice HTML coverage charts hilighting test coverage issues in a much simpler way than gcov text format. Generate these automatically from gcov output. Add additional string creation and comparison tests. * Move String coverage to >50% Additional string tests and checks * 66% test coverage in String * Add allocation-unit-sized strings test Ensure that strings that are right on the edge of the allocation size are handled properly and account for trailing 0. --- .gitignore | 2 + .travis.yml | 1 + tests/host/Makefile | 22 ++- tests/host/core/test_md5builder.cpp | 2 +- tests/host/core/test_string.cpp | 261 ++++++++++++++++++++++++++++ 5 files changed, 282 insertions(+), 6 deletions(-) create mode 100644 tests/host/core/test_string.cpp diff --git a/.gitignore b/.gitignore index b15779b9b..c327116cf 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,8 @@ tools/sdk/lwip/src/liblwip_src.a tools/sdk/ld/backup tools/sdk/ld/eagle.app.v6.common.ld +tests/hosts/lcov/ + *.pyc *.gch diff --git a/.travis.yml b/.travis.yml index 5591e47a6..0cfba22a2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,6 +38,7 @@ install: } make -C $HOME/astyle/build/gcc prefix=$HOME install; } || true + - sudo apt-get install valgrind lcov script: - $TRAVIS_BUILD_DIR/tests/common.sh diff --git a/tests/host/Makefile b/tests/host/Makefile index dadc13363..036ed18fc 100644 --- a/tests/host/Makefile +++ b/tests/host/Makefile @@ -1,4 +1,5 @@ BINARY_DIRECTORY := bin +LCOV_DIRECTORY := lcov OUTPUT_BINARY := $(BINARY_DIRECTORY)/host_tests CORE_PATH := ../../cores/esp8266 @@ -8,6 +9,9 @@ CC ?= gcc CXX ?= g++ endif GCOV ?= gcov +VALGRIND ?= valgrind +LCOV ?= lcov +GENHTML ?= genhtml CORE_CPP_FILES := $(addprefix $(CORE_PATH)/,\ StreamString.cpp \ @@ -49,11 +53,12 @@ TEST_CPP_FILES := \ fs/test_fs.cpp \ core/test_pgmspace.cpp \ core/test_md5builder.cpp \ + core/test_string.cpp - -CXXFLAGS += -std=c++11 -Wall -coverage -O0 -fno-common -CFLAGS += -std=c99 -Wall -coverage -O0 -fno-common +CXXFLAGS += -std=c++11 -Wall -Werror -coverage -O0 -fno-common -g +CFLAGS += -std=c99 -Wall -Werror -coverage -O0 -fno-common -g LDFLAGS += -coverage -O0 +VALGRINDFLAGS += --leak-check=full --track-origins=yes --error-limit=no --show-leak-kinds=all --error-exitcode=999 remduplicates = $(strip $(if $1,$(firstword $1) $(call remduplicates,$(filter-out $(firstword $1),$1)))) @@ -69,7 +74,7 @@ CPP_OBJECTS = $(CPP_OBJECTS_CORE) $(CPP_OBJECTS_TESTS) OBJECTS = $(C_OBJECTS) $(CPP_OBJECTS) COVERAGE_FILES = $(OBJECTS:.o=.gc*) -all: build-info $(OUTPUT_BINARY) test gcov +all: build-info $(OUTPUT_BINARY) valgrind test gcov test: $(OUTPUT_BINARY) $(OUTPUT_BINARY) @@ -81,11 +86,18 @@ clean-objects: rm -rf $(OBJECTS) clean-coverage: - rm -rf $(COVERAGE_FILES) *.gcov + rm -rf $(COVERAGE_FILES) $(LCOV_DIRECTORY) *.gcov gcov: test find $(CORE_PATH) -name "*.gcno" -exec $(GCOV) -r -pb {} + +valgrind: $(OUTPUT_BINARY) + mkdir -p $(LCOV_DIRECTORY) + $(LCOV) --directory ../../cores/esp8266/ --zerocounters + $(VALGRIND) $(VALGRINDFLAGS) $(OUTPUT_BINARY) + $(LCOV) --directory $(CORE_PATH) --capture --output-file $(LCOV_DIRECTORY)/app.info + $(GENHTML) $(LCOV_DIRECTORY)/app.info -o $(LCOV_DIRECTORY) + build-info: @echo "-------- build tools info --------" @echo "CC: " $(CC) diff --git a/tests/host/core/test_md5builder.cpp b/tests/host/core/test_md5builder.cpp index 72fa56f2c..0041c2eb7 100644 --- a/tests/host/core/test_md5builder.cpp +++ b/tests/host/core/test_md5builder.cpp @@ -38,7 +38,7 @@ TEST_CASE("MD5Builder::addHexString works as expected", "[core][MD5Builder]") WHEN("A char array is parsed"){ MD5Builder builder; builder.begin(); - const char * myPayload = "1234567890abcdeffedcba98765432106469676974616c7369676e61747572656170706c69636174696f6e73"; + const char * myPayload = "1234567890abcdeffedcba98765432106469676974616c7369676e61747572656170706c69636174696F6e73"; builder.addHexString(myPayload); builder.calculate(); REQUIRE(builder.toString() == "47b937a6f9f12a4c389fa5854e023efb"); diff --git a/tests/host/core/test_string.cpp b/tests/host/core/test_string.cpp new file mode 100644 index 000000000..39140b049 --- /dev/null +++ b/tests/host/core/test_string.cpp @@ -0,0 +1,261 @@ +/* + test_string.cpp - String tests + Copyright © 2018 Earle F. Philhower, III + + 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 + +TEST_CASE("String::trim", "[core][String]") +{ + String str; + str = " abcd123 "; + str.trim(); + REQUIRE(str == "abcd123"); +} + +TEST_CASE("String::replace", "[core][String]") +{ + String str; + str = "The quick brown fox jumped over the lazy dog."; + String find = "fox"; + String replace = "vulpes vulpes"; + str.replace(find, replace); + REQUIRE(str == "The quick brown vulpes vulpes jumped over the lazy dog."); +} + +TEST_CASE("String(value, base)", "[core][String]") +{ + String strbase2(9999,2); + String strbase8(9999,8); + String strbase10(9999,10); + String strbase16(9999,16); + REQUIRE(strbase2 == "10011100001111"); + REQUIRE(strbase8 == "23417"); + REQUIRE(strbase10 == "9999"); + REQUIRE(strbase16 == "270f"); + String strnegi(-9999); + String strnegf(-2.123, 3); + REQUIRE(strnegi == "-9999"); + REQUIRE(strnegf == "-2.123"); + String strbase16l((long)999999,16); + REQUIRE(strbase16l == "f423f"); +} + +TEST_CASE("String constructors", "[core][String]") +{ + String s0('c'); + REQUIRE(s0 == "c"); + String bin((unsigned char)5, 4); + REQUIRE(bin == "11"); + String ib((unsigned int)999, 16); + REQUIRE(ib == "3e7"); + String lb((unsigned long)3000000000, 8); + REQUIRE(lb == "26264057000"); + String sl1((long)-2000000000, 10); + REQUIRE(sl1 == "-2000000000"); + String s1("abcd"); + String s2(s1); + REQUIRE(s1 == s2); + String *s3 = new String("manos"); + s2 = *s3; + delete s3; + REQUIRE(s2 == "manos"); + s3 = new String("thisismuchlongerthantheother"); + s2 = s3->c_str(); + delete s3; + REQUIRE(s2 == "thisismuchlongerthantheother"); + String strf((float)3.14159, 5); + REQUIRE(strf == "3.14159"); + String ssh(strf + "_" + s1); + REQUIRE(ssh == "3.14159_abcd"); + String flash = (F("hello from flash")); + REQUIRE(flash == "hello from flash"); +} + +TEST_CASE("String concantenation", "[core][String]") +{ + String str; + REQUIRE(str.length() == 0); + str.reserve(1000); + REQUIRE(str.length() == 0); + str.reserve(0); + REQUIRE(str.length() == 0); + str += 'a'; + str += "bcde"; + str += str; + str += 987; + str += (int)INT_MAX; + str += (int)INT_MIN; + REQUIRE(str == "abcdeabcde9872147483647-2147483648"); + str += (unsigned char)69; + REQUIRE(str == "abcdeabcde9872147483647-214748364869"); + str += (unsigned int)1969; + REQUIRE(str == "abcdeabcde9872147483647-2147483648691969"); + str += (long)-123; + REQUIRE(str == "abcdeabcde9872147483647-2147483648691969-123"); + str += (unsigned long)321; + REQUIRE(str == "abcdeabcde9872147483647-2147483648691969-123321"); + str += (float)-1.01; + REQUIRE(str == "abcdeabcde9872147483647-2147483648691969-123321-1.01"); + str += (double)1.01; + REQUIRE(str == "abcdeabcde9872147483647-2147483648691969-123321-1.011.01"); + str = "clean"; + REQUIRE(str.concat(str) == true); + REQUIRE(str == "cleanclean"); +} + +TEST_CASE("String comparison", "[core][String]") +{ + String alpha("I like fish!"); + REQUIRE(alpha < "I like tacos!"); + REQUIRE(alpha > "I like bacon!"); + REQUIRE(alpha.equalsIgnoreCase("i LiKe FiSh!")); + REQUIRE(alpha.equalsConstantTime("I like fish!")); + REQUIRE(alpha != "I like fish?"); + REQUIRE(alpha.startsWith("I like")); + REQUIRE(!alpha.startsWith("I lick")); + REQUIRE(alpha.startsWith("fish", 7)); + REQUIRE(!alpha.startsWith("fish?", 7)); + REQUIRE(alpha.endsWith("!")); + REQUIRE(alpha.endsWith("fish!")); + REQUIRE(!alpha.endsWith("sh?")); +} + +TEST_CASE("String byte access", "[core][String]") +{ + String s; + s.reserve(1000); + s = "Never Eat Soggy Waffles"; + REQUIRE(s[0] == 'N'); + REQUIRE(s[999] == 0); + s[6] = 'C'; + REQUIRE(!strcmp(s.c_str(), "Never Cat Soggy Waffles")); + unsigned char buff[4]; + s.getBytes(buff, 4, 6); + REQUIRE(!memcmp(buff, "Cat", 4)); + s = "Never E"; + memset(buff, 0, 4); + s.getBytes(buff, 4, 6); + bool ok = (buff[0] == 'E') && (buff[1] == 0) && (buff[2] == 0) && (buff[3] == 0); + REQUIRE(ok == true); +} + +TEST_CASE("String conversion", "[core][String]") +{ + String s = "12345"; + long l = s.toInt(); + REQUIRE(l == 12345); + s = "2147483647"; + l = s.toInt(); + REQUIRE(l == INT_MAX); + s = "-2147483647"; + l = s.toInt(); + REQUIRE(l == -2147483647); + s = "-2147483648"; + l = s.toInt(); + REQUIRE(l == INT_MIN); + s = "3.14159"; + float f = s.toFloat(); + REQUIRE( fabs(f - 3.14159) < 0.0001 ); +} + +TEST_CASE("String case", "[core][String]") +{ + String s = "aBc_123"; + s.toLowerCase(); + REQUIRE(s == "abc_123"); + s = "aBc_123"; + s.toUpperCase(); + REQUIRE(s == "ABC_123"); +} + +TEST_CASE("String nulls", "[core][String]") +{ + String s; + REQUIRE(s == ""); + REQUIRE(s.toFloat() == 0); + REQUIRE(s.toInt() == 0); + s.trim(); + s.toUpperCase(); + s.toLowerCase(); + s.remove(1,1); + s.remove(10); + s.replace("taco", "burrito"); + s.replace('a', 'b'); + REQUIRE(s.substring(10, 20) == ""); + REQUIRE(s.lastIndexOf("tacos", 1) == -1); + REQUIRE(s.lastIndexOf("tacos") == -1); + REQUIRE(s.lastIndexOf('t', 0) == -1); + REQUIRE(s.lastIndexOf('t') == -1); + REQUIRE(s.indexOf("tacos", 1) == -1); + REQUIRE(s.indexOf("tacos") == -1); + REQUIRE(s.indexOf('t', 1) == -1); + REQUIRE(s.indexOf('t') == -1); + s.getBytes(NULL, 100, 0); + s[0] = 't'; + REQUIRE(s == ""); + REQUIRE(s.length() == 0); + s.setCharAt(1, 't'); + REQUIRE(s.startsWith("abc",0) == false); + REQUIRE(s.startsWith("def") == false); + REQUIRE(s.equalsConstantTime("def") == false); + REQUIRE(s.equalsConstantTime("") == true); + REQUIRE(s.equalsConstantTime(s) == true); + REQUIRE(s.equalsIgnoreCase(s) == true); + REQUIRE(s.equals("def") == false); + REQUIRE(s.equals("") == true); + REQUIRE(s.equals(s) == true); + String t = s; + REQUIRE(s.equals(t) == true); + REQUIRE((s <= "")); + REQUIRE(!(s < "")); + REQUIRE((s >= "")); + REQUIRE(!(s > "")); + s += "abc"; + REQUIRE(s == "abc"); +} + +TEST_CASE("String sizes near 8b", "[core][String]") +{ + // Test that proper amount of space allocated (including trailing 0) + // Need valgrind to verify no out-of-bounds errors, the strcmp()s will + // access each byte and cause an exception of the space was not properly + // allocated. + String s7("123456"); + String s8("1234567"); + String s9("12345678"); + String s15("12345678901234"); + String s16("123456789012345"); + String s17("1234567890123456"); + REQUIRE(!strcmp(s7.c_str(),"123456")); + REQUIRE(!strcmp(s8.c_str(),"1234567")); + REQUIRE(!strcmp(s9.c_str(),"12345678")); + REQUIRE(!strcmp(s15.c_str(),"12345678901234")); + REQUIRE(!strcmp(s16.c_str(),"123456789012345")); + REQUIRE(!strcmp(s17.c_str(),"1234567890123456")); + s7 += '_'; + s8 += '_'; + s9 += '_'; + s15 += '_'; + s16 += '_'; + s17 += '_'; + REQUIRE(!strcmp(s7.c_str(),"123456_")); + REQUIRE(!strcmp(s8.c_str(),"1234567_")); + REQUIRE(!strcmp(s9.c_str(),"12345678_")); + REQUIRE(!strcmp(s15.c_str(),"12345678901234_")); + REQUIRE(!strcmp(s16.c_str(),"123456789012345_")); + REQUIRE(!strcmp(s17.c_str(),"1234567890123456_")); +}