Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CI] Run unit tests #476

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+40 −12
Open

[CI] Run unit tests #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
64632ad
Fix gtest install
FabioBatSilva Apr 16, 2022
5aab324
Fix gtest install on windows
FabioBatSilva Apr 18, 2022
9c599db
Use -isystem to include gtest
FabioBatSilva Apr 17, 2022
64b649b
Compile utils_math with -O1
FabioBatSilva Apr 17, 2022
1a89222
Uncomment assertions for vector_2d on edge of range
FabioBatSilva Apr 17, 2022
4f11e2b
Add github action to run unit tests
FabioBatSilva Apr 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/unittest.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
name: Unit Tests
on:
push:

jobs:
build:
name: Unit Tests
runs-on: ubuntu-20.04
steps:
-
name: Checkout
uses: actions/checkout@v1.0.0
-
name: Install dependencies
run: make arm_sdk_install gtest_install
-
name: Run all unit tests
run: make all_ut_run
26 changes: 18 additions & 8 deletions make/tools.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,21 +190,31 @@ GTEST_DIR := $(TOOLS_DIR)/gtest-1.11.0
.PHONY: gtest_install
gtest_install: | $(DL_DIR) $(TOOLS_DIR)
gtest_install: GTEST_URL := https://github.com/google/googletest/archive/refs/tags/release-1.11.0.zip
gtest_install: GTEST_FILE := $(notdir $(GTEST_URL))
gtest_install: GTEST_FILE := googletest-$(notdir $(GTEST_URL))
gtest_install: gtest_clean
ifneq ($(OSFAMILY), windows)
$(eval GTEST_TMPDIR := $(shell mktemp -d))

# download the file unconditionally since google code gives back 404
# for HTTP HEAD requests which are used when using the wget -N option
$(V1) [ ! -f "$(DL_DIR)/$(GTEST_FILE)" ] || $(RM) -f "$(DL_DIR)/$(GTEST_FILE)"
$(V1) wget -P "$(DL_DIR)" "$(GTEST_URL)"
$(V1) wget "$(GTEST_URL)" -O "$(DL_DIR)/$(GTEST_FILE)"

# extract the source
$(V1) [ ! -d "$(GTEST_DIR)" ] || $(RM) -rf "$(GTEST_DIR)"
$(V1) $(MKDIR) "$(GTEST_DIR)"
$(V1) unzip -q -d "$(TOOLS_DIR)" "$(DL_DIR)/$(GTEST_FILE)"
# extract gtest source to tmp dir
$(V1) unzip -oq "$(DL_DIR)/$(GTEST_FILE)" -d "$(GTEST_TMPDIR)"

# move gtest to tools dir
$(V1) mv "$(GTEST_TMPDIR)/$(basename $(GTEST_FILE))/googletest" "$(GTEST_DIR)"

# remove gtest tmp dir
$(V1) $(RM) -rf "$(GTEST_TMPDIR)"
else
$(V1) curl --continue - --location --insecure --output "$(DL_DIR)/$(GTEST_FILE)" "$(GTEST_URL)"
$(V1) powershell -noprofile -command Expand-Archive -DestinationPath $(GTEST_DIR) -LiteralPath "$(DL_DIR)/$(GTEST_FILE)"

$(V1) powershell -noprofile -command if (Test-Path $(DL_DIR)/$(basename $(GTEST_FILE))) {Remove-Item -Recurse $(DL_DIR)/$(basename $(GTEST_FILE))}
$(V1) powershell -noprofile -command Expand-Archive -DestinationPath "$(DL_DIR)" -LiteralPath "$(DL_DIR)/$(GTEST_FILE)"

$(V1) powershell -noprofile -command Move-Item -Path "$(DL_DIR)/$(basename $(GTEST_FILE))/googletest" -Destination $(GTEST_DIR)
$(V1) powershell -noprofile -command if (Test-Path $(DL_DIR)/$(basename $(GTEST_FILE))) {Remove-Item -Recurse $(DL_DIR)/$(basename $(GTEST_FILE))}
endif

.PHONY: gtest_clean
Expand Down
2 changes: 1 addition & 1 deletion make/unittest.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ CXXFLAGS += -g -Wall -Wextra -Wno-missing-field-initializers -std=c++11
LDFLAGS += -lpthread

# Google Test requires visibility of gtest includes
GTEST_CXXFLAGS := -I$(GTEST_DIR)
GTEST_CXXFLAGS := -isystem $(GTEST_DIR)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will ignore the double-promotion warnings on gtest.

gtest.cc:1266:43:
  error: implicit conversion from ‘float’ to ‘double’ to match other
  operand of binary expression [-Werror=double-promotion]
   1266 |         costs[l_i + 1][r_i + 1] = replace + 1.00001;
``


# gcov requires specific link options to enable the coverage hooks
LDFLAGS += -fprofile-arcs
Expand Down
2 changes: 1 addition & 1 deletion tests/utils_math/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ EXTRAINCDIRS += $(TOP)/
EXTRAINCDIRS += $(HERE)/


CFLAGS += -O0
CFLAGS += -O1
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gcc/ld doesn't like the inline functions on utils_math.c

  utils_math.c:582: undefined reference to `utils_truncate_number'
    error: ld returned 1 exit status


# Set compiler-specific flags
GCC_CXXFLAGS = -fsingle-precision-constant
Expand Down
4 changes: 2 additions & 2 deletions tests/utils_math/unittest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,15 +296,15 @@ TEST_F(Saturate2dVector, ValsOnEdgeOfRange) {
ret = utils_saturate_vector_2d(&inputVal_x, &inputVal_y, inputVal_max);
EXPECT_FLOAT_EQ(1, inputVal_x);
EXPECT_FLOAT_EQ(1, inputVal_y);
/* EXPECT_EQ(false, ret); <-- Do not test, since this is on the edge and we don't need to know if it's explicitly inside or outside*/
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this was in place..
There are assertions on all the other tests..

Let me know if there is a reason why we can't assert the results

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FabioBatSilva technically you've combined a few separate changes in one PR?

EXPECT_EQ(false, ret);

inputVal_x = .1;
inputVal_y = .1;
inputVal_max = sqrt(.2);
ret = utils_saturate_vector_2d(&inputVal_x, &inputVal_y, inputVal_max);
EXPECT_FLOAT_EQ(.1, inputVal_x);
EXPECT_FLOAT_EQ(.1, inputVal_y);
/* EXPECT_EQ(false, ret); <-- Do not test, since this is on the edge and we don't need to know if it's explicitly inside or outside*/
EXPECT_EQ(false, ret);
}


Expand Down