From 228db9aad9fbf345bd2d4cb5db29617b8ecc7bbf Mon Sep 17 00:00:00 2001 From: Daniel Schauenberg Date: Fri, 3 Jan 2025 11:38:25 +0100 Subject: [PATCH 1/7] move lint and valgrind jobs into analysis suite --- .github/workflows/analysis.yaml | 48 +++++++++++++++++++++++++++++++++ .github/workflows/tests.yaml | 2 +- Makefile.am | 2 -- 3 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/analysis.yaml diff --git a/.github/workflows/analysis.yaml b/.github/workflows/analysis.yaml new file mode 100644 index 00000000..a4b59d9f --- /dev/null +++ b/.github/workflows/analysis.yaml @@ -0,0 +1,48 @@ +name: analysis + +on: + workflow_dispatch: + pull_request: + types: [opened, synchronize] + + +jobs: + cpplint: + name: unit + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + + - name: install dependencies + run: | + pip install cpplint + sudo apt-get install -qq valgrind libcurl4-openssl-dev + + - name: build + run: | + ./autogen.sh + ./configure --enable-coverage + + - name: run cpplint + run: make lint + + valgrind: + name: unit + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + + - name: install dependencies + run: | + pip install cpplint + sudo apt-get install -qq valgrind libcurl4-openssl-dev + + - name: build + run: | + ./autogen.sh + ./configure --enable-coverage + + - name: run cpplint + run: make valgrind diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 4c76c465..cf2dcf42 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -28,4 +28,4 @@ jobs: ./configure --enable-coverage - name: run tests - run: make ci + run: make test diff --git a/Makefile.am b/Makefile.am index 0ee77e68..5c1aa8db 100644 --- a/Makefile.am +++ b/Makefile.am @@ -44,8 +44,6 @@ clean-docker-services: docker rm --force restclient-cpp-httpbin 2>/dev/null || true docker rm --force restclient-cpp-squid 2>/dev/null || true -ci: test valgrind - clean-local: find . -name "*.gcda" -print0 | xargs -0 rm From 63e848158426547373a1d9f41d5df571546960be Mon Sep 17 00:00:00 2001 From: Daniel Schauenberg Date: Fri, 3 Jan 2025 11:39:49 +0100 Subject: [PATCH 2/7] fix analysis job names --- .github/workflows/analysis.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/analysis.yaml b/.github/workflows/analysis.yaml index a4b59d9f..8dc65a6a 100644 --- a/.github/workflows/analysis.yaml +++ b/.github/workflows/analysis.yaml @@ -8,7 +8,6 @@ on: jobs: cpplint: - name: unit runs-on: ubuntu-latest steps: @@ -28,7 +27,6 @@ jobs: run: make lint valgrind: - name: unit runs-on: ubuntu-latest steps: From 356e943a0cecc2289160047e2246c0756b746775 Mon Sep 17 00:00:00 2001 From: Daniel Schauenberg Date: Fri, 3 Jan 2025 11:41:43 +0100 Subject: [PATCH 3/7] run apt update before install --- .github/workflows/analysis.yaml | 2 ++ .github/workflows/tests.yaml | 1 + 2 files changed, 3 insertions(+) diff --git a/.github/workflows/analysis.yaml b/.github/workflows/analysis.yaml index 8dc65a6a..0a8ee31b 100644 --- a/.github/workflows/analysis.yaml +++ b/.github/workflows/analysis.yaml @@ -16,6 +16,7 @@ jobs: - name: install dependencies run: | pip install cpplint + sudo apt-get update sudo apt-get install -qq valgrind libcurl4-openssl-dev - name: build @@ -35,6 +36,7 @@ jobs: - name: install dependencies run: | pip install cpplint + sudo apt-get update sudo apt-get install -qq valgrind libcurl4-openssl-dev - name: build diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index cf2dcf42..5aed7305 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -17,6 +17,7 @@ jobs: - name: install dependencies run: | pip install cpplint + sudo apt-get update sudo apt-get install -qq valgrind libcurl4-openssl-dev - name: build googletest From 17645233dedc0879f902b1e3e7c9622f2145873e Mon Sep 17 00:00:00 2001 From: Daniel Schauenberg Date: Fri, 3 Jan 2025 11:45:57 +0100 Subject: [PATCH 4/7] build googletest before running valgrind --- .github/workflows/analysis.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/analysis.yaml b/.github/workflows/analysis.yaml index 0a8ee31b..aa23de7b 100644 --- a/.github/workflows/analysis.yaml +++ b/.github/workflows/analysis.yaml @@ -39,10 +39,13 @@ jobs: sudo apt-get update sudo apt-get install -qq valgrind libcurl4-openssl-dev + - name: build googletest + run: ./utils/build_gtest.sh + - name: build run: | ./autogen.sh ./configure --enable-coverage - - name: run cpplint + - name: run valgrind run: make valgrind From 136bae52ca2416da4aed610e2f8d51a9ba7b722a Mon Sep 17 00:00:00 2001 From: Daniel Schauenberg Date: Fri, 3 Jan 2025 11:53:01 +0100 Subject: [PATCH 5/7] fix linter violations --- Makefile.am | 7 ++++++- source/connection.cc | 7 ++++--- source/helpers.cc | 1 + source/restclient.cc | 2 ++ 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index 5c1aa8db..e66ed0f4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -30,7 +30,12 @@ valgrind: check valgrind --leak-check=full --error-exitcode=1 ./test-program lint: - cpplint --filter=-legal/copyright --root=$(CURDIR) include/restclient-cpp/*.h source/*.cc + # Filter reasons: + # legal/copyright: it's just maintenance overhead to have license headers + # in all files + # whitespace/indent_namespace: it makes code less readable if indentations + # in namespaces are not allowed + cpplint --filter="-legal/copyright,-whitespace/indent_namespace" --root=$(CURDIR) include/restclient-cpp/*.h source/*.cc docker-services: [ -n "$$(docker ps --quiet --filter name=restclient-cpp-httpbin)" ] || \ diff --git a/source/connection.cc b/source/connection.cc index e0dcbb7a..1a8e75e6 100644 --- a/source/connection.cc +++ b/source/connection.cc @@ -8,11 +8,12 @@ #include +#include #include -#include #include #include #include +#include #include #include "restclient-cpp/restclient.h" @@ -378,7 +379,7 @@ RestClient::Connection::performCurlRequest(const std::string& uri) { * * @param uri URI to query * @param ret Reference to the response struct that should be filled - * + * * @return reference to response struct for chaining */ RestClient::Response* @@ -571,7 +572,7 @@ RestClient::Connection::get(const std::string& url) { * * @param url to query * @param response struct - * + * * @return response struct ref for chaining */ RestClient::Response* diff --git a/source/helpers.cc b/source/helpers.cc index aa51bf58..e9b13fd6 100644 --- a/source/helpers.cc +++ b/source/helpers.cc @@ -7,6 +7,7 @@ #include "restclient-cpp/helpers.h" #include +#include #include "restclient-cpp/restclient.h" diff --git a/source/restclient.cc b/source/restclient.cc index ed0fe861..30ee3953 100644 --- a/source/restclient.cc +++ b/source/restclient.cc @@ -17,6 +17,8 @@ #if __cplusplus >= 201402L #include #endif +#include + #include "restclient-cpp/version.h" #include "restclient-cpp/connection.h" From dbe047391b9beb89773cb8ddc5d5a0dfbf2acde8 Mon Sep 17 00:00:00 2001 From: Daniel Schauenberg Date: Fri, 3 Jan 2025 11:56:36 +0100 Subject: [PATCH 6/7] valgrind also needs docker-services to run --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index e66ed0f4..02bb80ed 100644 --- a/Makefile.am +++ b/Makefile.am @@ -26,7 +26,7 @@ include/restclient-cpp/version.h: test: check docker-services ./test-program -valgrind: check +valgrind: check docker-services valgrind --leak-check=full --error-exitcode=1 ./test-program lint: From 235dff927552e5638ac4b7ac5d0d6244057fa906 Mon Sep 17 00:00:00 2001 From: Daniel Schauenberg Date: Fri, 3 Jan 2025 11:57:45 +0100 Subject: [PATCH 7/7] move comment outside of rule for portability --- Makefile.am | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile.am b/Makefile.am index 02bb80ed..d00c7463 100644 --- a/Makefile.am +++ b/Makefile.am @@ -29,12 +29,12 @@ test: check docker-services valgrind: check docker-services valgrind --leak-check=full --error-exitcode=1 ./test-program +# Filter reasons: +# legal/copyright: it's just maintenance overhead to have license headers +# in all files +# whitespace/indent_namespace: it makes code less readable if indentations +# in namespaces are not allowed lint: - # Filter reasons: - # legal/copyright: it's just maintenance overhead to have license headers - # in all files - # whitespace/indent_namespace: it makes code less readable if indentations - # in namespaces are not allowed cpplint --filter="-legal/copyright,-whitespace/indent_namespace" --root=$(CURDIR) include/restclient-cpp/*.h source/*.cc docker-services: