Update branch with latest (many) changes #19

eljonny · 2025-02-18T07:45:16Z

Branch update PR

Also clean up clang-tidy configuration, stop including the deprecated and removed AnalyzeTemporaryDtors option as the option no longer is used or even exists in versions of clang-tidy after version 6. Merge static analysis workflows to use only the debug workflow. It's not necessary to run the analysis on both release and debug build configurations, from what I can see. This is reflected in the README. Added information to the README on running cppcheck from CodeLite. Added information to the README on running cppcheck on the project from the command line/terminal. Added clang-tidy information to the README. Added custom build action to the CodeLite project to run cppcheck.

repo settings in GH.

Use the TestCaseName class (now named TestObjName) also for TestSuite to protect against null char * in string construction, which also simplifies the setSuiteName logic. Change the setSuiteName signature appropriately for the new type. Change private suiteName member type from string to TestObjName. Change constructor parameter for suite name from string to TestObjName, and ensure it's moved properly when storing it. Coincidentally, fixes GH issue #7. Extract strings into structs per-class so strings that are used more than once only need to be changed once, and can be referenced by tests for log verification when appropriate. Also if something will then be used again, just reference it.

In order to protect against null char *, we don't know how a string is constructed before it's passed to TestObjName.

I broke up TestCPP.h and the corresponding .cpp file into many pieces. It just started to get too big, and I wanted more focused TUs. So this is the first commit for that, where I split them out into different header files, put them in an 'internal' subdirectory, and turned TestCPP.h into an aggregate include for including the types and definitions needed by the library. So the common definitions are now split out into their own header-only definition in internal/TestCPPCommon.h. The exception types are now in their own internal/TestCPPExceptions.h header with corresponding .cpp. TestCase and TestSuite now have their own headers, internal/TestCPPTestCase.h and internal/TestCPPTestSuite.h, with corresponding .cpps. And finally TestCPPUtil.h is moved into internal and the TestObjName type has been moved into the Util header and definition .cpp, but is still in the base namespace TestCPP, not in TestCPP::Util. The biggest functional change is splitting the assertions out of TestSuite and into their own Assertions class, in their own TU. So from now on, the TestCPP assertions should be referenced through the Assertions class, not the TestSuite class, which makes a lot more sense. I added a couple new assertions a few weeks back, and I can see adding more, so they really need their own type to encapsulate that functionality. There's some significant cleanup to do, but that will probably be in a separate PR.

Since Assertions are now in their own TU, they now have their own test module. Assertion tests are removed from TestSuite tests. Removed now-unnecessary TestCPPUtil.h #include. Refactored assertion calls. Formatting.

Cover the patched line to get the CodeCov patch coverage to an acceptable level, plus a little. Added 3 new tests for this. Refactoring for API changes. Use TestCPPCommon to check output is as expected. Remove now-unnecessary TestCPPUtil.h #include.

Bump the major version because of the API change. This is still considered a beta-project, regardless. Work on the build takes the refactorings into account. This includes ensuring the new header structure is properly accounted for in packages and installation.

There are now multiple private headers, needed to encapsulate them into a list in order to have them properly applied to packing/installing build steps.

This fixes that.

Now it is, if it doesn't need iostream for debug logging then just include ostream for the friend operator<< overload.

I think this is not required for capture because it's constexpr.

I have to ignore the clang warning to work around the MSVC issue since the library is for C++11 not >=C++14, where there is an available workaround that doesn't involve ignoring a warning.

Initialize everything in TestCase with sane defaults. Name the parameter in the logTestFailure function prototype. Add boolean members that indicate whether for the given TestCase the output should be captured. Add implementation details for capture bools. This ensures all are counted properly. Rename the testPassedMessage TestSuite member to testSuitePassedMessage. This properly differentiates it from the similar member of TestCase. No addTest specializations in header file, only the template. Moved the specialization from the header file that was there to the source file. This fixed an issue when testing where the compiler could not find the proper specialization. Flesh out/add/fix docs. Fixed a huge bug in how the test passage message is enabled/disabled en- mass through a TestSuite. Previously the vector was iterated over with the newer style, but this causes a copy to be made on every iteration of every TestCase, which is completely not what we want to do here. Fall back to index-based vector iteration so we're properly modifying each original TestCase and not making copies thinking we're modifying the actual TestCases.

The following is what was previously tested in TestSuite: - Bare construction, no tests, only suite name The following is now tested, in addition to what was previously tested: - All possible construction parameters > Suite construction with TestCase objects > Suite construction with tuples > Suite construction with various mixes of TestCases and tuples - TestSuite::enableTestPassedMessage with various mixes of numbers of tests. - TestSuite::disableTestPassedMessage with various mixes of numbers of tests.

Excess whitespace in doc comments has been removed. Docs have been expanded/fixed. Removed unnecessary parameter from internal private API and adjust implementation to fix unnecessary reliance on the removed parameter.

Whoopsies

I don't know why I put this in there in the first place. It has been unused ever since I can remember. I think I was just messing with templates back in 2014 and this seemed like a cool function to implement.

Make sure stream capture does not affect logging of test failures. ----------------------------------------------------------------------- Created a subroutine for logging test failures where the output stream is injected instead of just using std::clog. This allows the TestCase::logTestFailure function to log to the real clog stream and the captured (if captured) clog stream to ensure any expected output is present for analysis. This ensures test failures are logged even if the stream being logged to is captured for analysis for the given test. This was an oversight on my part, and there are more than likely other instances where this will need to be corrected. Fix multiple frees on streams (stringstreams and streambufs) ----------------------------------------------------------------------- I've been working on this periodically since last month to figure out what is happening. It turns out, std::unique_ptr seems to add an exit handler to the program (this is a possibility, I haven't looked at the STL implementation code to confirm) that calls the deleter when it is a static object. The default deleter just calls delete on the pointed-to object without checking that and with no context around the object. The solution ended up being constructing each std::unique_ptr with a custom no-op deleter to allow the TestCase destructor to properly handle desctruction, since proper functionality of the TestCase code depends on the semaphore behavior defined in the construction and destruction of TestCases depending on the stream capture arguments, so it knows when to free the buffers: only when nothing is using them. The std::unique_ptr template arguments are adjusted accordingly, and the buffers are now freed at the proper time and place. The streambufs are also left alone with custom no-op deleters, as we have no business freeing streambufs that are not ours to delete (the underlying streams for std::cout, std::clog, and std::cerr).

There is now only one place for incrementing, and the conditions are now logically split and actions clearer. Tests in TestSuiteTests are no longer failing. The output buffer was not being cleared and there was lingering buffer content from previous tests. The tests were checking that nothing was logged related to test passage, so the checks for lack of output were failing. All tests pass! :)

Try to figure out where this is happening since I have no windows box to test this on.

more debug logging

debug logging for windows segfault issue

Was pointing to a previous file name where the link had been already corrected, but not the badge.

With a little extra line in the README under the respective sections.

Does not seem to apply to the PR run of the workflow when the change is applied to the PR fix branch.

* Mark constructors as explicit to conform to the C++ standard These constructors fall into the guideline of constructors that should be marked explicit. They are converting constructors that are not intended to be used for implicit conversions and copy-initialization. See https://en.cppreference.com/w/cpp/language/explicit From that site: Specifies that a constructor [or conversion function (since C++11)] [or deduction guide (since C++17)] is explicit, that is, it cannot be used for implicit conversions and copy-initialization. See https://en.cppreference.com/w/cpp/language/converting_constructor also. Enable inline CPPCheck suppressions in the GitHub static analysis workflow and the local CPPCheck command in the CodeLite project for suppressing one instance of noExplicitConstructor where it is intended to be used for implicit conversions and copy-initialization. Add inline suppression to TestCPPUtil.h for noExplicitConstructor. Add --std=c++11 parameter argument to local CPPCheck check in project. Removed unnecessary ignores from the CPPCheck command arguments where it just needs a clean prior to running the cppcheck custom target because the build system it generated probably includes more than what should be analyzed by CPPCheck. * Moved this change to main

…KE_BUILD_TYPE. This takes that into account.

Break down build arguments into groups into their own CMake files. Same with linking arguments. Start building out CMakePresets. Remove CMakeSettings.

Move "included" files into the CMakePresets.json file since having the include block didn't work like I thought it would.

…uce version to 5

CMakePresets is now modified such that: - there are buildPresets that correspond to all configurationPresets, so the proper base native tool options are applied to all relevant configurations when running a build in VS or on the command line - issues are addressed - added descriptions/names where missing - made presets hidden where appropriate so they don't show up in the VS build configurations menu, and so they can't be referenced when the cmake tool is invoked on the command line Fixed an issue in BuildTypeHandling.cmake where when building with ClangCL for running clang-tidy on Windows with (or without) VS, it was selecting MSVC++ tool options instead of GCC/Clang tool options since it is indeed running an MSBuild build, but the underlying toolchain is ClangCL Added some whitespace where it seemed to make it easier to read. Added the following gcc/clang switches: -Wno-c++98-compat - Because there are C++11 and greater features that conflict with C++98 compatibility, but that's ok because the project is only intended to be built with C++11 or newer -Wno-c++98-compat-pedantic - Same reason as for Wno-c++98-compat -Wno-covered-switch-default - Clang was giving me some confusing results... keep the default: case in my switch, and it would output this from -Wcovered-switch-default: error : default label in switch which covers all enumeration values [-Werror,-Wcovered-switch-default] Then in rewriting it so there's no default: case but basically works the same way, Clang would output this from -Wswitch-default: error : 'switch' missing 'default' label [-Werror,-Wswitch-default] So I go look it up and find this gem on SE-SoftwareEngineering-design: https://lnkd.in/gg8d7U2T I opted for defensive programming, as I usually do, left the default: case in the switch, and added -Wno-covered-switch-default to my build. It seems like it's probably the most correct thing to do... at risk of having an extremely weird (read: improbable) situation where disabling this warning would cause me to miss a bug. I think it's more likely that there would be a bug as a result of not having a default case to handle erroneous values since enums in C/C++ are not strongly-typed (a la Java). I'm not using -Weverything, so I find it odd that there are clashes in warnings, but it was an interesting exercise to work through this issue. I would much rather have -Wswitch-default enabled, because that can catch some really nasty issues; this is just a weird pedantic annoyance, where the default: case is for "just in case" because of the lack of type checking, as I mentioned already. I did not find arguments on StackOverflow very compelling that argued against a default case when all known explicitly-defined cases are covered; since enums are value-based types, they can hold any value of the underlying type, which the compiler might or might not warn about/catch; the collection of known cases is typically a very tiny subset of the set of all possible values (4+ billion with the default underlying int type), so there should be an escape hatch for if the programmer knowningly or unknowingly ventures outside those explicitly-defined cases, where it would make sense to do so and a sensible default can be used or an error/exception can be generated, instead of just crashing the host application with no information about why there was a crash; I didn't want to use an assert because the values can change and are not tied explicitly to the symbols, so an assert could easily be broken.

@return

Fixed documentation - wrong parameter name, @return on a constructor Fixed not explicitly specifying that the fail function never returns a value with [[noreturn]] even though it's void Fix problematic names of constants No need to have unsigned long long when std::chrono::nanoseconds. count() returns long long, as implicit conversions can cause unexpected problems.

Fix std::move being unqualified in a whole bunch of places; remove using std::move where it is no longer needed because std::move is supposed to be qualified. Specify explicitly that auto-destructors are not to be created and used with stream buffer and string stream pointers, because these "exit-destructors" can cause segfaults because the type destructor handles dismantling and destroying these unique_ptr objects. Refactoring problematic names of constants. Fixed missing EOF newlines. Fixed implicitly casting INT_MIN to unsigned.

…ng to use.

@MarkSchofield

Clang-Tidy configuration and .clang-tidy file: - Disable *-use-nullptr clang-tidy checks because they are broken on Windows in the Visual Studio 2022-provided Clang builds, as evidenced by the following issues I discovered in researching why clang-tidy was failing with an ACCESS_VIOLATION (0xc0000005) error: > https://developercommunity.visualstudio.com/t/clang-tidy-terminates-with-Exception-Cod/10822069?pageSize=15&sort=active&openOnly=false&closedOnly=false&topics=repos > llvm/llvm-project#53778 - Set FormatStyle to none so clang-tidy doesn't auto-format files, that is not what I want - Added HeaderFilterRegex that includes all headers, then added ExcludeHeaderFilterRegex to exclude the external headers (boost) - Added WarningsAsErrors * to treat all warnings as errors - Added SystemHeaders false so all system headers are excluded from analysis - Added ExtraArgs: > -Iinclude so library headers are discovered and included in analysis > -I3rdparty/include so boost headers are discovered and properly excluded from analysis - Now formatted as JSON so its format is more obvious without the weird opening/closing 3-character strings that indicate YAML but cause issues. Added hidden windows-specific presets that are intended to be inherited so the descendant presets will have the proper toolchain loaded; these are new to the presets here and have the toolchainFile property defined so as to load a toolchain from the cmake/toolchains location. There are ancestor toolchain presets defined for MSVC and ClangCL. ClangCL is only used for running clang-tidy analysis on Windows with the VS-packaged LLVM distribution. Split MSVC ancestor toolchain presets into stacktrace-enabled and no- stacktrace variants. Set CMAKE_SYSTEM_PROCESSOR so the toolchain scripts select the proper toolchain tools. Changed all VS-specific names, descriptions, and display names to be architecture-specific and also either Windows- or MSVC-specific, depending on context. Add CMAKE_C_FLAGS, CMAKE_CXX_FLAGS, and CMAKE_EXE_LINKER_FLAGS where appropriate and necessary, particularly where static code analysis is to be run. Fix nativeToolOptions since we're now using Ninja instead of MSBuild. Moved nativeToolOptions in code analysis buildPresets to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS in the corresponding configurePresets, because these are cl arguments, not MSBuild/Ninja arguments. Added buildPresets for clang-tidy configurePresets. Added an EOF newline. Added toolchain files since we're no longer using MSBuild, courtesy of https://github.com/MarkSchofield/WindowsToolchain, with some great CMake toolchain files; thank you @MarkSchofield; awesome work, they work wonderfully: - cmake/toolchains/VSWhere.cmake - cmake/toolchains/Windows.Clang.toolchain.cmake - cmake/toolchains/Windows.Kits.cmake - cmake/toolchains/Windows.MSVC.toolchain.cmake These are referenced through CMakePresets.json and are loaded such that the build has all the necessary tools. Fixed find_program for clang-tidy so it is now NO_CACHE. Fixed CLANG_TIDY_COMMAND such that it now uses the --config-file parameter, so all configurations are loaded through that instead of specified on the command line; added cache string description, and force-load the new value. Ignore the global-constructors warnings, as explained in the comment in the CMake files that define the GCC/Clang build flags for different configurations; this only applies to Clang until this issue gets resolved: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71482 Noted in GCCCoverage.cmake for if/when that ref issue gets resolved. Add /external:anglebrackets, /external:W0, and in the case where stacktraces are enabled /external:I3rdparty/include, so warnings in these external headers are ignored. Removed warning suppressions that are now unnecessary because warnings in external headers (boost, system headers) are now ignored thanks to the above CL parameters: - /wd4625 > Implicitly-deleted copy constructors. - /wd4626 > Implicitly-deleted copy assignment operators. - /wd4668 > Undefined preprocessor macros in the Windows SDK internal headers. - /wd5026 > Implicitly-deleted move constructors. - /wd5027 > Implicitly-deleted move assignment operators. - /wd5039 > Windows SDK functions that are passed to extern 'C' APIs that are not marked as noexcept. Added /nologo so if CMake doesn't configure the build to have it, it will be there. Added /Zi to the debug build ensuring all debug information is generated. /FS to avoid file locking issues. Added explicit exception handling enablement in MSVC to fix issues with noexcept generating warnings because there's no exception handling mode specified: /EHscr for Debug (adds noexcept runtime checks with the r), and /EHsc for Release (no runtime noexcept checks). Added runtime checks to Debug (/RTC1) so running the debug build can catch issues during testing or give more information if there's user code that is causing an issue inside the library. Make sure that if we are running an MSVC analysis build, and a ruleset file is explicitly selected, that we add the standard location for built-in ruleset files with /analyze:rulesetdirectory and the VS_INSTALLATION_PATH specified by the toolchain. CMake for some reason automatically adds /RTC1 to Release builds, which causes compilation to fail because /RTC1 and /O2 are incompatible, so the build generator now manually removes the RTC flags from release build compilation commands.

Add noexcept to constructors and operators that should not throw exceptions. Apply const to getLastRuntime function. Add no_destroy template type that replaces the clang::no_destroy attribute since it is not implemented in GCC or MSVC. There are some downsides, but they don't really bother me given the types it's applied to and the operations applied to those statics. For more info, see: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1247r0.html Applied some fixes based on this new level of indirection (calls to no_destroy::get).

Fix up codecov ignores

Added TXZ package to linux packing workflows. Added 7z package to multi-platform packing workflows. In static analysis workflow, ignore files in 3rdparty (Boost headers). Fix up cppcheck args for static analysis workflow with additional suppressions and flags that enhance and hone what it is checking for. Add WiX packing workflows (stacktrace-enabled and no-stacktraces) so MSI packages will now be part of the distribution; they're pretty rough right now, I need to do some design work on the BMPs, but they do work. With this comes a collection of image resources and an RTF-format version of the license to conform to the Windows Installer guidelines. Huge updates to the README.md: - Added badges for the WiX packing workflows - Formatting so the column width is always 72 characters or less - Added a section about building the project in Visual Studio 2022 - Elaborated on the structure of the CMake meta-build after some significant changes in previous commits a while ago where the README was not updated. - Code block formatting. - Made a separate section for static analysis of the project, and expanded on what cppcheck arguments are suggested after some experimentation about what gives the most thorough results. - Added information about what is produced in the base cpack configurations for Linux and Windows. - Added information about the new CPack configuration for WiX on Windows and how to run it to generate the WiX package (MSI file). - Added a subsection to the Development section that informs people what GitHub Actions CI workflows are implemented and what they do. Updated the CPack configurations in cmake/Packing.cmake: - Updated my email address because Google/GMail are not reliable as they are linked to space available in my Google Drive which can fluctuate wildly and sometimes is not available. - Added WIX configurations for MSI packaging: > Added product GUID > Added reference to the new LICENSE.rtf > Added reference to the new TestCPP icon > Added reference to the new Windows Installer progress banner > Added reference to the new Windows Installer Welcome/Goodbye image Added icon image basis.

github-actions · 2025-02-18T07:46:23Z

⚡ Static analysis result ⚡

🔴 cppcheck found 47 issues! Click here to see details.

TestCPP/include/internal/TestCPPAssertions.h

Lines 78 to 83 in 9107222

    
                   string failureMessage = "Arguments are not equivalent!" 
        
               ) 
        
           { 
        
               if (expected != actual) { 
        
                   stringstream err;

!Line: 78 - performance: Function parameter 'failureMessage' should be passed by const reference. [passedByValue]

TestCPP/include/internal/TestCPPAssertions.h

Lines 109 to 114 in 9107222

    
                   string failureMessage = "Arguments are equivalent!" 
        
               ) 
        
           { 
        
               if (expected == actual) { 
        
                   stringstream err;

!Line: 109 - performance: Function parameter 'failureMessage' should be passed by const reference. [passedByValue]

TestCPP/include/internal/TestCPPAssertions.h

Lines 136 to 141 in 9107222

    
                   string failureMessage = "Object is not null!" 
        
               ) 
        
           { 
        
               bool null = ptr == nullptr; 
        
               if (!null) {

!Line: 136 - performance: Function parameter 'failureMessage' should be passed by const reference. [passedByValue]

TestCPP/include/internal/TestCPPAssertions.h

Lines 163 to 168 in 9107222

    
                   string failureMessage = "Object is null!" 
        
               ) 
        
           { 
        
               bool notNull = ptr != nullptr; 
        
               if (!notNull) {

!Line: 163 - performance: Function parameter 'failureMessage' should be passed by const reference. [passedByValue]

TestCPP/src/TestCPPAssertions.cpp

Lines 92 to 97 in 9107222

    
                   string failureMessage 
        
               ) 
        
           { 
        
               if (!condition) { 
        
                   stringstream err;

!Line: 92 - performance: Function parameter 'failureMessage' should be passed by const reference. [passedByValue]

TestCPP/src/TestCPPAssertions.cpp

Lines 107 to 112 in 9107222

    
                   string failureMessage 
        
               ) 
        
           { 
        
               if (condition) { 
        
                   stringstream err;

!Line: 107 - performance: Function parameter 'failureMessage' should be passed by const reference. [passedByValue]

TestCPP/include/internal/TestCPPAssertions.h

Lines 76 to 81 in 9107222

    
           static void assertEquals ( 
        
                   T1 expected, T2 actual, 
        
                   string failureMessage = "Arguments are not equivalent!" 
        
               ) 
        
           { 
        
               if (expected != actual) {

!Line: 76 - style: The function 'assertEquals' is never used. [unusedFunction]

TestCPP/include/internal/TestCPPAssertions.h

Lines 107 to 112 in 9107222

    
           static void assertNotEquals ( 
        
                   T1 expected, T2 actual, 
        
                   string failureMessage = "Arguments are equivalent!" 
        
               ) 
        
           { 
        
               if (expected == actual) {

!Line: 107 - style: The function 'assertNotEquals' is never used. [unusedFunction]

TestCPP/include/internal/TestCPPAssertions.h

Lines 134 to 139 in 9107222

    
           static void assertNull ( 
        
                   T ptr, 
        
                   string failureMessage = "Object is not null!" 
        
               ) 
        
           { 
        
               bool null = ptr == nullptr;

!Line: 134 - style: The function 'assertNull' is never used. [unusedFunction]

TestCPP/include/internal/TestCPPAssertions.h

Lines 161 to 166 in 9107222

    
           static void assertNotNull ( 
        
                   T ptr, 
        
                   string failureMessage = "Object is null!" 
        
               ) 
        
           { 
        
               bool notNull = ptr != nullptr;

!Line: 161 - style: The function 'assertNotNull' is never used. [unusedFunction]

TestCPP/src/TestCPPAssertions.cpp

Lines 42 to 47 in 9107222

    
           void Assertions::assertThrows ( 
        
                   function<void()> shouldThrow, 
        
                   string failureMessage 
        
               ) 
        
           { 
        
               try {

!Line: 42 - style: The function 'assertThrows' is never used. [unusedFunction]

TestCPP/src/TestCPPAssertions.cpp

Lines 77 to 82 in 9107222

    
           void Assertions::assertNoThrows ( 
        
                   function<void()> shouldNotThrow, 
        
                   string failureMessage 
        
               ) 
        
           { 
        
               try {

!Line: 77 - style: The function 'assertNoThrows' is never used. [unusedFunction]

TestCPP/src/TestCPPAssertions.cpp

Lines 90 to 95 in 9107222

    
           void Assertions::assertTrue ( 
        
                   bool condition, 
        
                   string failureMessage 
        
               ) 
        
           { 
        
               if (!condition) {

!Line: 90 - style: The function 'assertTrue' is never used. [unusedFunction]

TestCPP/src/TestCPPAssertions.cpp

Lines 105 to 110 in 9107222

    
           void Assertions::assertFalse ( 
        
                   bool condition, 
        
                   string failureMessage 
        
               ) 
        
           { 
        
               if (condition) {

!Line: 105 - style: The function 'assertFalse' is never used. [unusedFunction]

TestCPP/src/TestCPPAssertions.cpp

Lines 120 to 123 in 9107222

    
               [[noreturn]] void Assertions::fail(string failureMessage) { 
        
                   throw TestFailedException(std::move(failureMessage)); 
        
               } 
        
           }

!Line: 120 - style: The function 'fail' is never used. [unusedFunction]

TestCPP/include/internal/TestCPPUtil.h

Lines 73 to 78 in 9107222

    
                   TestObjName (const char* name); 
        
                   /** 
        
                    * @brief Get the encapsulated name for the TestCPP object that 
        
                    *          holds this object. 
        
                    * @return The name of the TestCPP object that this object

!Line: 73 - style: Class 'TestObjName' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]

Uh oh!

Update branch with latest (many) changes #19

Update branch with latest (many) changes #19

Uh oh!

Conversation

eljonny commented Feb 18, 2025

Uh oh!

github-actions bot commented Feb 18, 2025

⚡ Static analysis result ⚡

Uh oh!

github-actions bot commented Feb 18, 2025

⚡ Static analysis result ⚡

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant