From 9b04d6663ca5e0227063322dd6fd922e5a07e2ab Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 11 Apr 2022 10:30:25 -0500 Subject: [PATCH] Run IWYU (with the new tools) on test simulator code This mixes some manual work (IWYU pragmas, a better solution to the Arduino preprocessor macros problem) with automated running of the tools. At this point, it would be too much work to separate these into distinct commits, and there isn't that much value to doing so. There are still some things we could do to make things more robust, as some of the headers need to be in a certain order, which happens to be in the same sort order used by IWYU (`testing/*` files need to come after certain headers than include `Arduino.h`), but it's probably not worth the clutter of adding an `#if 1` just to stop IWYU from re-ordering them. I tried to get `#pragma push_macro("max")/pop_macro("max")` to work, but ended up getting completely nonsensical compilation errors, so I gave up on it. Signed-off-by: Michael Richters --- testing/AbsoluteMouseReport.cpp | 9 ++++--- testing/AbsoluteMouseReport.h | 8 +++---- testing/ConsumerControlReport.cpp | 6 ++--- testing/ConsumerControlReport.h | 8 +++---- testing/ExpectedKeyboardReport.cpp | 3 +++ testing/ExpectedKeyboardReport.h | 8 +++---- testing/ExpectedMouseReport.cpp | 3 +++ testing/ExpectedMouseReport.h | 8 +++---- testing/HIDState.cpp | 10 ++++---- testing/HIDState.h | 21 +++++++++------- testing/KeyboardReport.cpp | 7 +++--- testing/KeyboardReport.h | 8 +++---- testing/MouseReport.cpp | 7 ++---- testing/MouseReport.h | 8 +++---- testing/SimHarness.cpp | 6 +++-- testing/SimHarness.h | 8 +++---- testing/State.h | 10 +++----- testing/SystemControlReport.cpp | 5 ++-- testing/SystemControlReport.h | 8 +++---- testing/VirtualDeviceTest.cpp | 17 +++++++++---- testing/VirtualDeviceTest.h | 30 +++++++++++++---------- testing/gtest.h | 36 ++++++++++++++++++++++++++++ testing/{fix-macros.h => iostream.h} | 16 ++++++------- testing/matchers.h | 11 ++++----- testing/setup-googletest.h | 17 +++++++------ tests/issues/951/test/testcase.cpp | 1 - 26 files changed, 165 insertions(+), 114 deletions(-) create mode 100644 testing/gtest.h rename testing/{fix-macros.h => iostream.h} (65%) diff --git a/testing/AbsoluteMouseReport.cpp b/testing/AbsoluteMouseReport.cpp index 7b9bc3da..33f9e485 100644 --- a/testing/AbsoluteMouseReport.cpp +++ b/testing/AbsoluteMouseReport.cpp @@ -16,12 +16,11 @@ #include "testing/AbsoluteMouseReport.h" -#include "Kaleidoscope.h" -#include "testing/fix-macros.h" +#include // for memcpy +#include // for vector -#include - -#include "MouseButtons.h" +#include "MouseButtons.h" // for MOUSE_LEFT, MOUSE_MIDDLE, MOUSE_NEXT, MOUSE_PREV, MOUS... +#include "kaleidoscope/Runtime.h" // for Runtime, Runtime_ namespace kaleidoscope { namespace testing { diff --git a/testing/AbsoluteMouseReport.h b/testing/AbsoluteMouseReport.h index 887c2c1b..cc204aa3 100644 --- a/testing/AbsoluteMouseReport.h +++ b/testing/AbsoluteMouseReport.h @@ -16,11 +16,11 @@ #pragma once -#include -#include +#include // for uint16_t, uint32_t, uint8_t, int8_t +#include // for vector -#include "DeviceAPIs/AbsoluteMouseAPI.h" -#include "HID-Settings.h" +#include "DeviceAPIs/AbsoluteMouseAPI.h" // for HID_MouseAbsoluteReport_Data_t +#include "HID-Settings.h" // for HID_REPORTID_MOUSE_ABSOLUTE namespace kaleidoscope { namespace testing { diff --git a/testing/ConsumerControlReport.cpp b/testing/ConsumerControlReport.cpp index 8239ae11..8f64f511 100644 --- a/testing/ConsumerControlReport.cpp +++ b/testing/ConsumerControlReport.cpp @@ -16,10 +16,10 @@ #include "testing/ConsumerControlReport.h" -#include "Kaleidoscope.h" -#include "testing/fix-macros.h" +#include // for memcpy +#include // for vector -#include +#include "kaleidoscope/Runtime.h" // for Runtime, Runtime_ namespace kaleidoscope { namespace testing { diff --git a/testing/ConsumerControlReport.h b/testing/ConsumerControlReport.h index 61857f72..93ca97d9 100644 --- a/testing/ConsumerControlReport.h +++ b/testing/ConsumerControlReport.h @@ -16,11 +16,11 @@ #pragma once -#include -#include +#include // for uint32_t, uint16_t, uint8_t +#include // for vector -#include "HID-Settings.h" -#include "MultiReport/ConsumerControl.h" +#include "HID-Settings.h" // for HID_REPORTID_CONSUMERCONTROL +#include "MultiReport/ConsumerControl.h" // for HID_ConsumerControlReport_Data_t namespace kaleidoscope { namespace testing { diff --git a/testing/ExpectedKeyboardReport.cpp b/testing/ExpectedKeyboardReport.cpp index bf1f014a..ba4e71b5 100644 --- a/testing/ExpectedKeyboardReport.cpp +++ b/testing/ExpectedKeyboardReport.cpp @@ -16,6 +16,9 @@ #include "testing/ExpectedKeyboardReport.h" +#include // for uint8_t, uint32_t +#include // for set + namespace kaleidoscope { namespace testing { diff --git a/testing/ExpectedKeyboardReport.h b/testing/ExpectedKeyboardReport.h index ce967a7b..789dd875 100644 --- a/testing/ExpectedKeyboardReport.h +++ b/testing/ExpectedKeyboardReport.h @@ -16,10 +16,10 @@ #pragma once -#include -#include -#include -#include +#include // for uint8_t, uint32_t +#include // for set + +#include "testing/iostream.h" // for string namespace kaleidoscope { namespace testing { diff --git a/testing/ExpectedMouseReport.cpp b/testing/ExpectedMouseReport.cpp index fb85d7e5..e6f938c8 100644 --- a/testing/ExpectedMouseReport.cpp +++ b/testing/ExpectedMouseReport.cpp @@ -16,6 +16,9 @@ #include "testing/ExpectedMouseReport.h" +#include "MultiReport/Mouse.h" // for (anonymous union)::(anonymous) +#include "testing/MouseReport.h" // for MouseReport::ReportData + namespace kaleidoscope { namespace testing { diff --git a/testing/ExpectedMouseReport.h b/testing/ExpectedMouseReport.h index 1d6f2e41..932de2c0 100644 --- a/testing/ExpectedMouseReport.h +++ b/testing/ExpectedMouseReport.h @@ -16,12 +16,10 @@ #pragma once -#include -#include -#include -#include +#include // for int8_t, uint32_t, uint8_t -#include "MouseReport.h" +#include "MouseReport.h" // for MouseReport, MouseReport::ReportData +#include "testing/iostream.h" // for string namespace kaleidoscope { namespace testing { diff --git a/testing/HIDState.cpp b/testing/HIDState.cpp index 68c3fb4c..4aac3448 100644 --- a/testing/HIDState.cpp +++ b/testing/HIDState.cpp @@ -16,12 +16,14 @@ #include "testing/HIDState.h" -#include "HID-Settings.h" +// IWYU pragma: no_include <__utility/move.h> -#include "testing/fix-macros.h" +#include // IWYU pragma: keep +#include // for vector + +#include "HID-Settings.h" // for HID_REPORTID_CONSUMERCONTROL, HID_REPORTID_GAMEPAD, HID_RE... +#include "testing/iostream.h" // for operator<<, char_traits, cout, ostream, basic_ostream -// TODO(epan): Add proper logging. -#include #define LOG(x) std::cout namespace kaleidoscope { diff --git a/testing/HIDState.h b/testing/HIDState.h index 9b93e770..af8449d9 100644 --- a/testing/HIDState.h +++ b/testing/HIDState.h @@ -16,15 +16,18 @@ #pragma once -#include "testing/AbsoluteMouseReport.h" -#include "testing/ConsumerControlReport.h" -#include "testing/KeyboardReport.h" -#include "testing/MouseReport.h" -#include "testing/SystemControlReport.h" - -// Out of order due to macro conflicts. -#include "testing/fix-macros.h" -#include +// IWYU pragma: no_include <__memory/unique_ptr.h> + +#include // for size_t +#include // for uint8_t +#include // IWYU pragma: keep +#include // for vector + +#include "testing/AbsoluteMouseReport.h" // for AbsoluteMouseReport +#include "testing/ConsumerControlReport.h" // for ConsumerControlReport +#include "testing/KeyboardReport.h" // for KeyboardReport +#include "testing/MouseReport.h" // for MouseReport +#include "testing/SystemControlReport.h" // for SystemControlReport namespace kaleidoscope { namespace testing { diff --git a/testing/KeyboardReport.cpp b/testing/KeyboardReport.cpp index 89bf822f..429e0f26 100644 --- a/testing/KeyboardReport.cpp +++ b/testing/KeyboardReport.cpp @@ -16,10 +16,11 @@ #include "testing/KeyboardReport.h" -#include "Kaleidoscope.h" -#include "testing/fix-macros.h" +#include // for memcpy +#include // for vector<>::iterator, vector -#include +#include "kaleidoscope/Runtime.h" // for Runtime, Runtime_ +#include "kaleidoscope/key_defs.h" // for HID_KEYBOARD_FIRST_MODIFIER, HID_LAST_KEY namespace kaleidoscope { namespace testing { diff --git a/testing/KeyboardReport.h b/testing/KeyboardReport.h index a798d28c..47d06e77 100644 --- a/testing/KeyboardReport.h +++ b/testing/KeyboardReport.h @@ -16,11 +16,11 @@ #pragma once -#include -#include +#include // for uint8_t, uint32_t +#include // for vector -#include "HID-Settings.h" -#include "MultiReport/Keyboard.h" +#include "HID-Settings.h" // for HID_REPORTID_NKRO_KEYBOARD +#include "MultiReport/Keyboard.h" // for HID_KeyboardReport_Data_t namespace kaleidoscope { namespace testing { diff --git a/testing/MouseReport.cpp b/testing/MouseReport.cpp index 49bb9121..a24b54e0 100644 --- a/testing/MouseReport.cpp +++ b/testing/MouseReport.cpp @@ -16,12 +16,9 @@ #include "testing/MouseReport.h" -#include "Kaleidoscope.h" -#include "testing/fix-macros.h" +#include // for memcpy -#include - -#include "MouseButtons.h" +#include "kaleidoscope/Runtime.h" // for Runtime, Runtime_ namespace kaleidoscope { namespace testing { diff --git a/testing/MouseReport.h b/testing/MouseReport.h index 8b4d0258..aa0f9333 100644 --- a/testing/MouseReport.h +++ b/testing/MouseReport.h @@ -16,11 +16,11 @@ #pragma once -#include -#include +#include // for uint8_t, int8_t, uint32_t -#include "HID-Settings.h" -#include "MultiReport/Mouse.h" +#include "HID-Settings.h" // for HID_REPORTID_MOUSE +#include "MouseButtons.h" // for MOUSE_LEFT, MOUSE_MIDDLE, MOUSE_NEXT, MOUSE_PREV, MOUSE_R... +#include "MultiReport/Mouse.h" // for HID_MouseReport_Data_t namespace kaleidoscope { namespace testing { diff --git a/testing/SimHarness.cpp b/testing/SimHarness.cpp index b6acb7e1..7f716d67 100644 --- a/testing/SimHarness.cpp +++ b/testing/SimHarness.cpp @@ -16,8 +16,10 @@ #include "testing/SimHarness.h" -#include "testing/fix-macros.h" -#include +#include // for millis + +#include "kaleidoscope/Runtime.h" // for Runtime, Runtime_ +#include "kaleidoscope/device/device.h" // for Device, VirtualProps::KeyScanner namespace kaleidoscope { namespace testing { diff --git a/testing/SimHarness.h b/testing/SimHarness.h index bda69547..424a43ca 100644 --- a/testing/SimHarness.h +++ b/testing/SimHarness.h @@ -16,11 +16,11 @@ #pragma once -#include -#include +#include // for size_t +#include // for uint8_t -#include "Kaleidoscope.h" -#include "testing/fix-macros.h" +#include "kaleidoscope/KeyAddr.h" // for KeyAddr +#include "testing/gtest.h" // IWYU pragma: keep namespace kaleidoscope { namespace testing { diff --git a/testing/State.h b/testing/State.h index b8951b5c..694fd5eb 100644 --- a/testing/State.h +++ b/testing/State.h @@ -16,15 +16,11 @@ #pragma once -#include -#include -#include +// IWYU pragma: no_include <__memory/unique_ptr.h> -#include "testing/HIDState.h" +#include // IWYU pragma: keep -// Out of order due to macro conflicts. -#include "testing/fix-macros.h" -#include +#include "testing/HIDState.h" // for HIDState namespace kaleidoscope { namespace testing { diff --git a/testing/SystemControlReport.cpp b/testing/SystemControlReport.cpp index 2fb7cc0a..206ee26c 100644 --- a/testing/SystemControlReport.cpp +++ b/testing/SystemControlReport.cpp @@ -16,10 +16,9 @@ #include "testing/SystemControlReport.h" -#include "Kaleidoscope.h" -#include "testing/fix-macros.h" +#include // for memcpy -#include +#include "kaleidoscope/Runtime.h" // for Runtime, Runtime_ namespace kaleidoscope { namespace testing { diff --git a/testing/SystemControlReport.h b/testing/SystemControlReport.h index c5d4edcf..741c565e 100644 --- a/testing/SystemControlReport.h +++ b/testing/SystemControlReport.h @@ -16,11 +16,11 @@ #pragma once -#include -#include +#include // for uint8_t, uint32_t +#include // for vector -#include "HID-Settings.h" -#include "MultiReport/SystemControl.h" +#include "HID-Settings.h" // for HID_REPORTID_SYSTEMCONTROL +#include "MultiReport/SystemControl.h" // for HID_SystemControlReport_Data_t namespace kaleidoscope { namespace testing { diff --git a/testing/VirtualDeviceTest.cpp b/testing/VirtualDeviceTest.cpp index e7da0619..0bf3282e 100644 --- a/testing/VirtualDeviceTest.cpp +++ b/testing/VirtualDeviceTest.cpp @@ -16,10 +16,19 @@ #include "testing/VirtualDeviceTest.h" -#include "HIDReportObserver.h" -#include "testing/HIDState.h" - -#include +// IWYU pragma: no_include <__algorithm/max.h> +// IWYU pragma: no_include <__tree> + +#include // for bitset +#include // for vector + +#include "HIDReportObserver.h" // for HIDReportObserver +#include "kaleidoscope/Runtime.h" // for Runtime, Runtime_ +#include "testing/HIDState.h" // for HIDState, HIDStateBuilder +#include "testing/KeyboardReport.h" // for KeyboardReport +#include "testing/MouseReport.h" // for MouseReport +#include "testing/gtest.h" // for Message, TestPartResult, EXPECT_EQ, ElementsAreArray +#include "testing/iostream.h" // for operator<<, basic_ostream, char_traits, string, cerr namespace kaleidoscope { namespace testing { diff --git a/testing/VirtualDeviceTest.h b/testing/VirtualDeviceTest.h index 9231c4cd..2c40fdc0 100644 --- a/testing/VirtualDeviceTest.h +++ b/testing/VirtualDeviceTest.h @@ -16,18 +16,24 @@ #pragma once -#include - -#include "testing/ExpectedKeyboardReport.h" -#include "testing/ExpectedMouseReport.h" -#include "testing/SimHarness.h" -#include "testing/State.h" - -// Out of order due to macro conflicts. -#include "testing/fix-macros.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" -#include +// IWYU pragma: no_include <__memory/unique_ptr.h> + +#include // for size_t +#include // for uint32_t, int8_t, uint8_t +#include // for initializer_list +#include // IWYU pragma: keep +#include // for set +#include // for vector + +#include "kaleidoscope/KeyAddr.h" // for KeyAddr +#include "kaleidoscope/key_defs.h" // for Key +#include "testing/ExpectedKeyboardReport.h" // for ExpectedKeyboardReport +#include "testing/ExpectedMouseReport.h" // for ExpectedMouseReport +#include "testing/HIDState.h" // for HIDState +#include "testing/SimHarness.h" // for SimHarness +#include "testing/State.h" // for State +#include "testing/gtest.h" // for Test +#include "testing/iostream.h" // for string namespace kaleidoscope { namespace testing { diff --git a/testing/gtest.h b/testing/gtest.h new file mode 100644 index 00000000..1931e629 --- /dev/null +++ b/testing/gtest.h @@ -0,0 +1,36 @@ +/* -*- mode: c++ -*- + * Copyright (C) 2022 Keyboardio, Inc. + * + * This program is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +#pragma once + +#undef TEST + +// In order to include gtest files, we need to undefine some macros that +// Arduino.h unwisely exports. Rahter than including "gtest/gtest.h" (et al) +// directly, any simulator code should instead include "testing/gtest.h". +#undef min +#undef max + +// The headers listed here other than "gtest/gtest.h" and "gmock/gmock.h" are +// only included to prevent IWYU from inserting them directly into simulator +// source files. This seems mildly preferable to modifying the gtest files +// themselves. +#include "gmock/gmock-matchers.h" // IWYU pragma: export +#include "gmock/gmock.h" // IWYU pragma: export +#include "gtest/gtest-message.h" // IWYU pragma: export +#include "gtest/gtest-test-part.h" // IWYU pragma: export +#include "gtest/gtest.h" // IWYU pragma: export +#include "gtest/gtest_pred_impl.h" // IWYU pragma: export diff --git a/testing/fix-macros.h b/testing/iostream.h similarity index 65% rename from testing/fix-macros.h rename to testing/iostream.h index a6a63c81..40dca8f7 100644 --- a/testing/fix-macros.h +++ b/testing/iostream.h @@ -1,5 +1,5 @@ /* -*- mode: c++ -*- - * Copyright (C) 2020 Eric Paniagua (epaniagua@google.com) + * Copyright (C) 2022 Keyboardio, Inc. * * This program is free software: you can redistribute it and/or modify it under * the terms of the GNU General Public License as published by the Free Software @@ -14,12 +14,12 @@ * this program. If not, see . */ -// No `#pragma once` since these undefs need to have every time a Kaleidoscope/ -// Arduino header is included before non-Kaleidoscope/Arduino header. The undefs -// are needed, due to naming conflicts between Arduino and Googletest. +#pragma once -#undef min +// In order to include certain standard library files, we need to undefine some +// macros that Arduino.h unwisely exports. Rahter than including +// directly, any simulator code should instead include "testing/iostream.h". #undef max -#undef T -#undef U -#undef TEST +#undef min + +#include // IWYU pragma: export diff --git a/testing/matchers.h b/testing/matchers.h index bd717568..6de9fe0e 100644 --- a/testing/matchers.h +++ b/testing/matchers.h @@ -16,13 +16,12 @@ #pragma once -#include "kaleidoscope/key_defs.h" -#include "testing/SystemControlReport.h" +// For some reason, the `MATCHER_P` macro confuses IWYU into trying to include +// this file in itself, so we need to block it with the following: +// IWYU pragma: no_include "testing/matchers.h" -// Out of order because `fix-macros.h` clears the preprocessor environment for -// gtest and gmock. -#include "testing/fix-macros.h" -#include "gmock/gmock.h" +#include "kaleidoscope/key_defs.h" // for Key +#include "testing/gtest.h" // for GMOCK_PP_INTERNAL_FOR_EACH_IMPL_1, GMOCK_PP_INTERNAL_... namespace kaleidoscope { namespace testing { diff --git a/testing/setup-googletest.h b/testing/setup-googletest.h index 85cb034e..b7830d89 100644 --- a/testing/setup-googletest.h +++ b/testing/setup-googletest.h @@ -18,17 +18,16 @@ #pragma once -#include "kaleidoscope/key_defs/keyboard.h" -#include "Kaleidoscope.h" +#include // IWYU pragma: keep -// Out of order because `fix-macros.h` clears the preprocessor environment for -// gtest and gmock. -#include "testing/fix-macros.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" +// Kaleidoscope.h includes Arduino, which unwisely defines `min` and `max` as +// preprocessor macros. We need to undefine these macros before including any +// files from the standard library. +#undef min +#undef max -#include "testing/matchers.h" -#include "testing/VirtualDeviceTest.h" +#include "testing/VirtualDeviceTest.h" // IWYU pragma: keep +#include "testing/matchers.h" // IWYU pragma: keep #define SETUP_GOOGLETEST() \ void executeTestFunction() { \ diff --git a/tests/issues/951/test/testcase.cpp b/tests/issues/951/test/testcase.cpp index ffe19b84..e69cb1b3 100644 --- a/tests/issues/951/test/testcase.cpp +++ b/tests/issues/951/test/testcase.cpp @@ -15,7 +15,6 @@ */ #include "Kaleidoscope.h" -#include "testing/fix-macros.h" #include "testing/setup-googletest.h"