Update testing #152
Update testing #152
Conversation
This command should be in the top-level source directory because ctest expects to find a test file in the top-level build directory. See https://cmake.org/cmake/help/latest/command/enable_testing.html
Remove manual add_test() and instead use gtest_discover_tests() to discover all tests. This way tests can be run using ctest, displaying the outcome of each test.
The custom CMake target 'check' was removed. Use 'test' instead.
WalkthroughThis pull request comprehensively updates the GoogleTest and GoogleMock third-party codebase. It modernizes build configurations to require C++14, refactors Bazel and CMake integration, removes legacy generator scripts, adds new matchers and actions, improves test coverage, updates documentation, and enhances internal utilities for type safety and diagnostics. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
In summary: This PR is a comprehensive update to the GoogleTest and GoogleMock third-party code, but it does not address the objective of adding ffmpeg 7.0 support for chromaprint (#142). All changes are out of scope for the linked issue. Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🔭 Outside diff range comments (3)
src/3rdparty/googletest/ci/macos-presubmit.sh (1)
56-74: Bazel flags: solid, but watch for macro fallout from-Wundef.
-Wundefturns quite a few innocuous#if FOOinto warnings/errors whenFOOisn’t defined.
If your public headers expose such guards, you may need follow-up clean-ups.src/3rdparty/googletest/docs/reference/actions.md (1)
41-49: Minor style nit – stray back-ticks aroundfIn the “callable” section, the first cell shows only a single back-ticked
f, which can be read as column-header noise rather than an example. Consider prefixing with “f(callable)” or usingInvoke(f)consistently.src/3rdparty/googletest/docs/advanced.md (1)
180-183: Tighten wording – “etc.” is sufficientThe phrase “and etc” is redundant.
-... prefix, suffix, regular expression, and etc). For +... prefix, suffix, regular expression, etc.). For
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (79)
.github/workflows/build.yml(2 hunks)CMakeLists.txt(2 hunks)cmake/modules/FindGTest.cmake(0 hunks)src/3rdparty/googletest/.github/ISSUE_TEMPLATE/00-bug_report.md(0 hunks)src/3rdparty/googletest/.github/ISSUE_TEMPLATE/00-bug_report.yml(1 hunks)src/3rdparty/googletest/.github/ISSUE_TEMPLATE/10-feature_request.md(0 hunks)src/3rdparty/googletest/.github/ISSUE_TEMPLATE/10-feature_request.yml(1 hunks)src/3rdparty/googletest/.github/ISSUE_TEMPLATE/config.yml(1 hunks)src/3rdparty/googletest/.gitignore(2 hunks)src/3rdparty/googletest/BUILD.bazel(3 hunks)src/3rdparty/googletest/CMakeLists.txt(1 hunks)src/3rdparty/googletest/CONTRIBUTING.md(5 hunks)src/3rdparty/googletest/CONTRIBUTORS(2 hunks)src/3rdparty/googletest/MODULE.bazel(2 hunks)src/3rdparty/googletest/README.md(5 hunks)src/3rdparty/googletest/WORKSPACE(1 hunks)src/3rdparty/googletest/WORKSPACE.bzlmod(2 hunks)src/3rdparty/googletest/ci/linux-presubmit.sh(5 hunks)src/3rdparty/googletest/ci/macos-presubmit.sh(3 hunks)src/3rdparty/googletest/ci/windows-presubmit.bat(1 hunks)src/3rdparty/googletest/docs/_layouts/default.html(1 hunks)src/3rdparty/googletest/docs/advanced.md(60 hunks)src/3rdparty/googletest/docs/faq.md(24 hunks)src/3rdparty/googletest/docs/gmock_cheat_sheet.md(6 hunks)src/3rdparty/googletest/docs/gmock_cook_book.md(35 hunks)src/3rdparty/googletest/docs/gmock_faq.md(1 hunks)src/3rdparty/googletest/docs/gmock_for_dummies.md(6 hunks)src/3rdparty/googletest/docs/pkgconfig.md(2 hunks)src/3rdparty/googletest/docs/platforms.md(1 hunks)src/3rdparty/googletest/docs/primer.md(15 hunks)src/3rdparty/googletest/docs/quickstart-bazel.md(3 hunks)src/3rdparty/googletest/docs/quickstart-cmake.md(3 hunks)src/3rdparty/googletest/docs/reference/actions.md(6 hunks)src/3rdparty/googletest/docs/reference/assertions.md(5 hunks)src/3rdparty/googletest/docs/reference/matchers.md(7 hunks)src/3rdparty/googletest/docs/reference/mocking.md(2 hunks)src/3rdparty/googletest/docs/reference/testing.md(32 hunks)src/3rdparty/googletest/docs/samples.md(1 hunks)src/3rdparty/googletest/fake_fuchsia_sdk.bzl(1 hunks)src/3rdparty/googletest/googlemock/CMakeLists.txt(8 hunks)src/3rdparty/googletest/googlemock/README.md(2 hunks)src/3rdparty/googletest/googlemock/include/gmock/gmock-actions.h(36 hunks)src/3rdparty/googletest/googlemock/include/gmock/gmock-cardinalities.h(4 hunks)src/3rdparty/googletest/googlemock/include/gmock/gmock-function-mocker.h(9 hunks)src/3rdparty/googletest/googlemock/include/gmock/gmock-more-actions.h(8 hunks)src/3rdparty/googletest/googlemock/include/gmock/gmock-more-matchers.h(2 hunks)src/3rdparty/googletest/googlemock/include/gmock/gmock-nice-strict.h(6 hunks)src/3rdparty/googletest/googlemock/include/gmock/gmock-spec-builders.h(42 hunks)src/3rdparty/googletest/googlemock/include/gmock/gmock.h(1 hunks)src/3rdparty/googletest/googlemock/include/gmock/internal/custom/README.md(1 hunks)src/3rdparty/googletest/googlemock/include/gmock/internal/custom/gmock-generated-actions.h(1 hunks)src/3rdparty/googletest/googlemock/include/gmock/internal/custom/gmock-matchers.h(1 hunks)src/3rdparty/googletest/googlemock/include/gmock/internal/custom/gmock-port.h(1 hunks)src/3rdparty/googletest/googlemock/include/gmock/internal/gmock-internal-utils.h(12 hunks)src/3rdparty/googletest/googlemock/include/gmock/internal/gmock-port.h(2 hunks)src/3rdparty/googletest/googlemock/scripts/README.md(0 hunks)src/3rdparty/googletest/googlemock/scripts/fuse_gmock_files.py(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/LICENSE(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/README(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/README.cppclean(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/cpp/ast.py(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/cpp/gmock_class.py(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/cpp/gmock_class_test.py(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/cpp/keywords.py(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/cpp/tokenize.py(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/cpp/utils.py(0 hunks)src/3rdparty/googletest/googlemock/scripts/generator/gmock_gen.py(0 hunks)src/3rdparty/googletest/googlemock/src/gmock-cardinalities.cc(3 hunks)src/3rdparty/googletest/googlemock/src/gmock-internal-utils.cc(8 hunks)src/3rdparty/googletest/googlemock/src/gmock-matchers.cc(5 hunks)src/3rdparty/googletest/googlemock/src/gmock-spec-builders.cc(18 hunks)src/3rdparty/googletest/googlemock/src/gmock.cc(7 hunks)src/3rdparty/googletest/googlemock/src/gmock_main.cc(3 hunks)src/3rdparty/googletest/googlemock/test/BUILD.bazel(1 hunks)src/3rdparty/googletest/googlemock/test/gmock-actions_test.cc(32 hunks)src/3rdparty/googletest/googlemock/test/gmock-cardinalities_test.cc(17 hunks)src/3rdparty/googletest/googlemock/test/gmock-function-mocker_test.cc(31 hunks)src/3rdparty/googletest/googlemock/test/gmock-internal-utils_test.cc(21 hunks)src/3rdparty/googletest/googlemock/test/gmock-matchers-arithmetic_test.cc(1 hunks)
💤 Files with no reviewable changes (15)
- src/3rdparty/googletest/googlemock/scripts/README.md
- src/3rdparty/googletest/.github/ISSUE_TEMPLATE/10-feature_request.md
- src/3rdparty/googletest/googlemock/scripts/generator/gmock_gen.py
- src/3rdparty/googletest/googlemock/scripts/generator/cpp/utils.py
- src/3rdparty/googletest/googlemock/scripts/generator/cpp/gmock_class.py
- src/3rdparty/googletest/googlemock/scripts/generator/cpp/tokenize.py
- src/3rdparty/googletest/.github/ISSUE_TEMPLATE/00-bug_report.md
- cmake/modules/FindGTest.cmake
- src/3rdparty/googletest/googlemock/scripts/generator/cpp/ast.py
- src/3rdparty/googletest/googlemock/scripts/fuse_gmock_files.py
- src/3rdparty/googletest/googlemock/scripts/generator/README.cppclean
- src/3rdparty/googletest/googlemock/scripts/generator/README
- src/3rdparty/googletest/googlemock/scripts/generator/cpp/gmock_class_test.py
- src/3rdparty/googletest/googlemock/scripts/generator/LICENSE
- src/3rdparty/googletest/googlemock/scripts/generator/cpp/keywords.py
🧰 Additional context used
🪛 LanguageTool
src/3rdparty/googletest/docs/platforms.md
[uncategorized] ~7-~7: Possible missing preposition found.
Context: ... a list of currently supported versions compilers, platforms, and build tools.
(AI_EN_LECTOR_MISSING_PREPOSITION)
src/3rdparty/googletest/CONTRIBUTING.md
[style] ~24-~24: Consider shortening this phrase to avoid wordiness.
Context: ... Googler? If you are a Googler, please make an attempt to submit an internal contribution rather ...
(MAKE_AN_ATTEMPT_AT_STYLE)
[style] ~25-~25: As a shorter alternative for ‘able to’, consider using “can not”.
Context: ...ther than a GitHub Pull Request. If you are not able to submit internally, a PR is acceptable a...
(BE_ABLE_TO)
[uncategorized] ~53-~53: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...couraged to contribute to the discussion and you can also help us to keep the effect...
(COMMA_COMPOUND_SENTENCE)
src/3rdparty/googletest/docs/quickstart-cmake.md
[uncategorized] ~69-~69: Possible missing comma found.
Context: ... configuration declares a dependency on GoogleTest which is downloaded from GitHub. In the...
(AI_HYDRA_LEO_MISSING_COMMA)
src/3rdparty/googletest/docs/gmock_for_dummies.md
[misspelling] ~97-~97: This word is normally spelled as one.
Context: ...shiny new graphics card that has better anti-aliasing? Suddenly you have to update all your g...
(EN_COMPOUNDS_ANTI_ALIASING)
[typographical] ~97-~97: Consider adding a comma after ‘Suddenly’ for more clarity.
Context: ...ics card that has better anti-aliasing? Suddenly you have to update all your golden imag...
(RB_LY_COMMA)
[misspelling] ~195-~195: This word is normally spelled as one.
Context: ...ually the same directory or a testing sub-directory), and put it in a .h and a `cc_librar...
(EN_COMPOUNDS_SUB_DIRECTORY)
src/3rdparty/googletest/docs/reference/assertions.md
[style] ~280-~280: To make your writing concise, consider an alternative.
Context: ...rgest finite float value are considered to be one ULP apart. ### EXPECT_DOUBLE_EQ {#...
(CONSIDERED_TO_BE_WORDINESS)
[style] ~289-~289: To make your writing concise, consider an alternative.
Context: ...gest finite double value are considered to be one ULP apart. ### EXPECT_NEAR {#EXPEC...
(CONSIDERED_TO_BE_WORDINESS)
[grammar] ~299-~299: After the plural determiner ‘both’ use the plural noun “infinities”.
Context: ...or*. If *val* and *val2`* are both infinity of the same sign, the difference is con...
(ARE_DT_NN)
src/3rdparty/googletest/docs/quickstart-bazel.md
[style] ~42-~42: Consider a more expressive alternative.
Context: ...try.bazel.build/modules/googletest). To do this, create a MODULE.bazel file in t...
(DO_ACHIEVE)
src/3rdparty/googletest/docs/primer.md
[style] ~26-~26: Consider using a more concise synonym.
Context: ...ions, so GoogleTest tests can work with a variety of configurations. 4. When tests fail, th...
(A_VARIETY_OF)
[uncategorized] ~30-~30: Possible missing comma found.
Context: ... set up tests that report non-fatal failures after which the current test continues....
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~36-~36: Consider a more concise word here.
Context: ... require the user to enumerate them in order to run them. 6. Tests should be fast. W...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~104-~104: Possible missing comma found.
Context: ...e. You may also supply a custom failure message which will be appended to GoogleTest's ...
(AI_HYDRA_LEO_MISSING_COMMA)
[formatting] ~291-~291: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...ne a destructor or a TearDown() method, because the implicit destructor generated by th...
(COMMA_BEFORE_BECAUSE)
[style] ~292-~292: Consider removing “of” to be more concise
Context: ... generated by the compiler will perform all of the necessary cleanup. Now we'll write tes...
(ALL_OF_THE)
[style] ~341-~341: Consider a more concise word here.
Context: ... have to re-list all your defined tests in order to run them. After defining your tests, y...
(IN_ORDER_TO_PREMIUM)
src/3rdparty/googletest/docs/faq.md
[uncategorized] ~37-~37: Possible missing comma found.
Context: ... But that's getting complicated. So for simplicity we just say that it cannot start with `...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~58-~58: A comma may be missing after the conjunctive/linking adverb ‘Therefore’.
Context: ...g) or with a new version of GoogleTest. Therefore it's best to follow the rule. ## Why d...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~179-~179: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...oo::kBar; // No initializer here. ``` Otherwise your code is invalid C++, and may b...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~310-~310: Possible missing comma found.
Context: ...ch. It will use the definition from the class the constructor of which is current...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~328-~328: The operating system from Apple is written “macOS”
Context: ...e exceptions are enabled (e.g. Windows, Mac OS, and Linux client-side), which will...
(MAC_OS)
[style] ~357-~357: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... it has a GoogleTest assertion failure. Very bad. We have decided to fix this (thanks t...
(EN_WEAK_ADJECTIVE)
[style] ~422-~422: Consider using “bunch”.
Context: ...`` ## GoogleTest output is buried in a whole bunch of LOG messages. What do I do? The Goo...
(WHOLE_LOT)
[style] ~426-~426: To elevate your writing, try using a synonym here.
Context: ...x with the GoogleTest output, making it hard to read. However, there is an easy solu...
(HARD_TO)
src/3rdparty/googletest/README.md
[style] ~60-~60: Consider using a more concise synonym.
Context: ...f assertions: \ Googletest provides a variety of assertions, such as equality, inequalit...
(A_VARIETY_OF)
src/3rdparty/googletest/docs/reference/testing.md
[uncategorized] ~424-~424: Possible missing comma found.
Context: ...eption} testing::AssertionException Exception which can be thrown from [`TestEventLis...
(AI_HYDRA_LEO_MISSING_COMMA)
src/3rdparty/googletest/docs/reference/matchers.md
[uncategorized] ~99-~99: Possible missing comma found.
Context: ... | argument matches the given regular expression with the match starting at the first ch...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
src/3rdparty/googletest/CONTRIBUTING.md
105-105: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
114-114: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
124-124: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
131-131: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
137-137: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
src/3rdparty/googletest/docs/gmock_for_dummies.md
264-264: Heading levels should only increment by one level at a time
Expected: h3; Actual: h6
(MD001, heading-increment)
src/3rdparty/googletest/docs/quickstart-bazel.md
46-46: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
110-110: Inline HTML
Element: pre
(MD033, no-inline-html)
111-111: Inline HTML
Element: strong
(MD033, no-inline-html)
src/3rdparty/googletest/docs/primer.md
70-70: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
75-75: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
src/3rdparty/googletest/docs/faq.md
392-392: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
src/3rdparty/googletest/README.md
17-17: Bare URL used
null
(MD034, no-bare-urls)
src/3rdparty/googletest/docs/gmock_cook_book.md
290-290: Link fragments should be valid
null
(MD051, link-fragments)
1308-1308: Link fragments should be valid
null
(MD051, link-fragments)
2790-2790: Link fragments should be valid
null
(MD051, link-fragments)
src/3rdparty/googletest/docs/reference/matchers.md
201-201: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
src/3rdparty/googletest/docs/advanced.md
421-421: Spaces inside code span elements
null
(MD038, no-space-in-code)
2018-2018: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (152)
src/3rdparty/googletest/googlemock/README.md (1)
38-40: Link update looks goodSwitching the GoogleTest link to HTTPS/GitHub is spot-on. Nothing else to flag here.
src/3rdparty/googletest/CONTRIBUTORS (1)
37-37: Alphabetical insertion verified – LGTM
New names drop in at the correct sorted positions and follow the existing formatting.Also applies to: 58-60
src/3rdparty/googletest/docs/_layouts/default.html (1)
51-51: License link update looks good
Branch rename reflected correctly.src/3rdparty/googletest/docs/gmock_faq.md (1)
372-373: Header path simplification is harmless
Reference is clearer and still accurate.CMakeLists.txt (1)
199-202: Nice: tests are now discoverable byctest
enable_testing()before the tests sub-dir is exactly what the CI needed.
No further notes.src/3rdparty/googletest/googlemock/include/gmock/internal/custom/gmock-generated-actions.h (1)
1-2: IWYU pragmas look fineThe private/friend hints match the pattern used in other new headers. No functional impact.
src/3rdparty/googletest/.github/ISSUE_TEMPLATE/config.yml (1)
2-5: Good addition to point users to DiscussionsKeeps bug-tracker noise down. 👍
src/3rdparty/googletest/googlemock/include/gmock/internal/custom/README.md (1)
17-18: Docs updated to list the new flag macrosMatches the code in
gmock-port.h, all clear.src/3rdparty/googletest/googlemock/include/gmock/internal/custom/gmock-matchers.h (1)
30-33: Consistent IWYU pragmasSame pragma pair as the other custom headers; include hygiene stays consistent.
src/3rdparty/googletest/.gitignore (1)
28-30: 👍 Good call adding VS Code artefacts to.gitignore.
Keeps local editor noise out of the tree.src/3rdparty/googletest/docs/reference/mocking.md (2)
4-4: Header-inclusion wording clarified – looks good.
The switch to “#include <gmock/gmock.h>” makes the instruction unambiguous.
250-252: Useful detail onWillOncebehaviour.
Calling out move-only/rvalue support will save users head-scratching.src/3rdparty/googletest/googlemock/include/gmock/internal/custom/gmock-port.h (1)
34-35: Confirm sentinel comment removal is harmless.
Previous versions of this file contained the marker
// GOOGLETEST_CM0002 DO NOT DELETEwhich some external scripts check for.
Before merging, please verify nothing in your tooling/CI relies on that string.src/3rdparty/googletest/ci/macos-presubmit.sh (1)
43-44: CMake now enforces C++14 – fine, but double-check downstream projects.
Any consumer still compiling with-std=c++11will now break; worth a heads-up in release notes.src/3rdparty/googletest/googlemock/src/gmock_main.cc (1)
35-38: Minor style tweak looks fine.
Adding NRF52 support behindARDUINOgate is harmless.src/3rdparty/googletest/googlemock/test/BUILD.bazel (2)
41-41: Good addition of header file to test sources.Including
gmock-matchers_test.hin the sources ensures the test utilities are available during compilation.
43-43: QNX platform support follows established pattern.The empty linkopts for QNX matches the Windows pattern and provides proper platform-specific build configuration.
src/3rdparty/googletest/docs/pkgconfig.md (2)
25-31: Modern CMake practices implemented correctly.The updates use current CMake best practices:
target_sourceswithPRIVATEscope,enable_testing()instead ofinclude(CTest), and proper target-based linking. These changes make the example more maintainable.
104-104: Verify version consistency across the upgrade.The example shows version
1.11.0, but the PR objectives mention upgrading to1.16.0. Make sure this version aligns with the actual GoogleTest version being used.#!/bin/bash # Check what version of GoogleTest is actually being used in this update rg -A 5 -B 5 "version.*1\.[0-9]+\.[0-9]+" --type mdsrc/3rdparty/googletest/googlemock/include/gmock/gmock.h (2)
56-63: IWYU pragma annotations improve tooling support.Adding
// IWYU pragma: exportcomments helps include-what-you-use tooling understand the public interface boundaries. The newgmock-spec-builders.hinclude is properly annotated as well.
65-72: Namespace placement after port.h include is correct.Moving the namespace declaration after including
gmock-port.hensures proper initialization order and follows the typical pattern for platform-specific setup.src/3rdparty/googletest/docs/quickstart-cmake.md (4)
13-13: C++14 requirement correctly documented.The documentation accurately reflects the new minimum C++ standard requirement for GoogleTest 1.16.0.
55-57: CMake configuration enforces C++14 properly.Setting
CMAKE_CXX_STANDARD_REQUIRED ONensures strict enforcement of the C++14 requirement, preventing builds with older standards that might fail silently.
62-62: Git commit hash updated for GoogleTest upgrade.The commit hash reflects the updated version being used. Keep this hash current as GoogleTest continues to evolve.
Also applies to: 70-70
112-112: Modern CMake target usage is correct.Using
GTest::gtest_maininstead of plaingtest_mainfollows modern CMake practices with imported targets, providing better dependency management and configuration propagation.src/3rdparty/googletest/CONTRIBUTING.md (3)
50-55: HTTPS links and improved navigation.Converting discussion group links to HTTPS and adding proper hyperlinks improves security and usability.
83-84: Branch reference updated correctly.Updating the link to point to the
mainbranch instead ofmasterfollows current repository conventions.
91-91: Python 3.6 minimum requirement is reasonable.Updating from Python 2.3 to 3.6 aligns with modern development practices since Python 2 reached end-of-life.
src/3rdparty/googletest/docs/reference/actions.md (1)
9-20: Table header addition looks goodIntroducing uniform headers (“Action”, “Description”) improves readability and fixes earlier Markdown-lint warnings. No concerns here.
src/3rdparty/googletest/CMakeLists.txt (2)
23-30:find_package(absl …)&find_package(re2 …)rely on consumer-supplied config filesCMake doesn’t ship built-in Find modules for Abseil or RE2. When
GTEST_HAS_ABSLis ON and no imported targets exist, the build will hard-fail unless the caller has pre-installedabsl/re2CMake config packages.Two follow-ups:
- Add a friendly error explaining the expectation.
- Gate the lookup behind
CONFIGto avoid picking up unexpected Find module shims.- find_package(absl REQUIRED) + find_package(absl CONFIG REQUIRED) … - find_package(re2 REQUIRED) + find_package(re2 CONFIG REQUIRED)
4-4: Bumpingcmake_minimum_requiredto 3.13 – confirm downstream coverageThe parent project currently advertises support as far back as 3.5. Nested 3rd-party CMake minimums override the top-level, so consumers with older toolchains will break. Please double-check that 3.13 really is the practical floor (copy-DLL logic needs ≥ 3.21 anyway) or gate the newer features with version checks.
src/3rdparty/googletest/.github/ISSUE_TEMPLATE/10-feature_request.yml (1)
1-34: Well-structured GitHub issue template.The YAML template follows GitHub's best practices with proper field validation and automatic labeling. The required fields encourage thoughtful feature requests with adequate detail and justification.
src/3rdparty/googletest/docs/gmock_for_dummies.md (6)
93-93: HTTPS URL update improves security.Good change from HTTP to HTTPS for the LOGO programming language Wikipedia link.
100-100: HTTPS URL update improves security.Good change from HTTP to HTTPS for the Dependency Injection Wikipedia link.
167-167: Angle bracket includes follow standard convention.The change from quoted includes to angle brackets for system headers like
<gmock/gmock.h>is the correct convention.
227-228: Consistent use of angle bracket includes.Good consistency with the gmock header include style change.
193-198: Helpful Bazel-specific guidance for mock placement.The updated guidance for placing mock classes in Bazel packages with
testonly=Trueis practical and follows Bazel best practices.
485-486: Improved formatting with backticks.The addition of backticks and italics around
*variable*improves readability in theReturnRefdocumentation.src/3rdparty/googletest/WORKSPACE.bzlmod (1)
31-36: Proper bzlmod migration implementation.The intentionally empty file with explanatory comment correctly implements the migration to Bazel's module system. When bzlmod is enabled, this prevents unintentional dependencies on the legacy WORKSPACE file.
src/3rdparty/googletest/googlemock/src/gmock-cardinalities.cc (3)
53-53: Cleaner single-line member initialization.Consolidating the member initializer list to a single line improves readability without affecting functionality.
56-57: Simplified string concatenation maintains clarity.The string concatenation changes make the error messages more compact while preserving readability.
Also applies to: 60-61, 65-66
89-90: Modern C++11 explicit deletion replaces macro.Using explicit
= deletefor copy constructor and assignment operator is clearer and more idiomatic than theGTEST_DISALLOW_COPY_AND_ASSIGN_macro.src/3rdparty/googletest/googlemock/include/gmock/gmock-cardinalities.h (4)
36-37: IWYU pragmas improve include management.The Include What You Use (IWYU) pragmas help tools understand this header's intended usage pattern and dependencies.
68-68: Defaulted virtual destructor is cleaner.Using
= defaultfor the virtual destructor is more explicit and idiomatic than an empty body.
95-95: Defaulted default constructor improves clarity.The
= defaultconstructor makes the intent explicit and follows modern C++ practices.
121-121: Minor formatting improvement.The indentation adjustment for the negation operator improves code alignment.
src/3rdparty/googletest/googlemock/CMakeLists.txt (5)
39-40: Good modernization of CMake requirements.Bumping to CMake 3.13 makes sense for the project modernization and aligns with the C++14 requirement. The simplified project declaration removes unnecessary conditional logic.
68-74: Improved include directory handling for fetch-content compatibility.The switch from
gtest_SOURCE_DIRto querying the target'sINCLUDE_DIRECTORIESproperty directly fixes issues when using fetch-content with custom names. This is a solid improvement.
105-111: Clean implementation of semicolon-escaped include directories.The use of
string(REPLACE)and$<SEMICOLON>generator expression properly handles list separators in CMake target properties. This removes the need for version-specific conditionals.
135-135: Simplified compiler options for MinGW/Cygwin.Removing the conditional CMake version check and always using
add_compile_optionsis cleaner since the minimum CMake version is now 3.13.
146-149: Better test organization with granular matcher tests.Splitting the monolithic
gmock-matchers_testinto arithmetic, comparisons, containers, and misc tests improves test granularity and makes it easier to identify specific test failures.src/3rdparty/googletest/docs/reference/assertions.md (2)
4-4: Good modernization of include directives.Switching from quoted includes to angle-bracket includes (
<gtest/gtest.h>,<gmock/gmock.h>) follows modern best practices and indicates these are system/external headers.Also applies to: 91-91
525-525: Fixed HRESULT documentation pairing.Correcting the pairing from
EXPECT_HRESULT_FAILEDtoASSERT_HRESULT_FAILEDfixes the documentation error.src/3rdparty/googletest/ci/windows-presubmit.bat (3)
49-52: Good workaround for Windows path length limitations.The use of
--output_user_root=C:\tmpis a practical solution to Windows path length restrictions in CI environments.
57-57: Enforces C++14 standard consistently.Using
--copt=/std:c++14aligns with the project's modernization to C++14 as mentioned in the PR objectives.
5-6: Verify Python version consistency.The script sets
PATHto includeC:\Python34butBAZEL_PYTHONtoC:\python34\python.exe. The casing difference might cause issues on case-sensitive systems, and Python 3.4 is quite old.Check if the Python paths are correct and consistent:
#!/bin/bash # Check for Python path consistency issues in CI scripts echo "Checking Python path references in Windows presubmit script..." rg -n "python34|Python34" --case-sensitivesrc/3rdparty/googletest/docs/gmock_cheat_sheet.md (4)
11-11: Good addition of explicit access specifiers.Adding
public:to class declarations makes the examples more complete and ensures mock methods are accessible.Also applies to: 26-26, 61-61, 74-74
23-23: Modernized include directive.Switching to
<gmock/gmock.h>follows the same modernization pattern seen throughout the project.
143-143: Good use of standard library utilities.Replacing a custom
MakeUniquefunction withstd::make_uniqueis the right approach for C++14 code.
233-233: Fixed hyperlink anchor casing.The correction from
using-check-pointstoUsingCheckPointsensures the link works properly.src/3rdparty/googletest/fake_fuchsia_sdk.bzl (4)
1-18: Excellent documentation for the fake SDK approach.The docstring clearly explains why this exists, how to override it, and includes the GitHub issue reference. This addresses a real need while Fuchsia SDK doesn't support bzlmod.
20-28: Clean implementation of stub target generation.The repository rule correctly creates minimal BUILD.bazel files for each stub package. The
filegrouptargets are appropriate stubs that won't break builds.
36-40: Appropriate default stub targets.The default list covers the Fuchsia SDK packages that GoogleTest actually needs (
pkg/fdio,pkg/syslog,pkg/zx) without creating unnecessary overhead.
47-56: Correct module extension implementation.The extension properly handles the
create_faketag and sets up the fake SDK repository when needed. Thereproducible = Trueflag is appropriate for this deterministic operation.src/3rdparty/googletest/README.md (5)
9-12: Documentation accurately reflects modern Git practices.The switch from
mastertomainbranch and the Live at Head philosophy documentation is correct and follows current industry standards.
20-26: Version and C++14 requirement properly documented.The 1.16.0 release documentation and C++14 requirement are clearly stated, which helps users understand compatibility requirements.
53-84: Expanded features section improves documentation clarity.The detailed feature descriptions with explanations make the documentation more informative than the previous bullet-point format.
87-92: Platform support documentation follows best practices.Referencing Google's Foundational C++ Support Policy and the external matrix table is a good approach for maintaining current platform information without duplicating content.
99-104: HTTPS URL updates improve security.Converting HTTP URLs to HTTPS is a good security practice for documentation links.
src/3rdparty/googletest/docs/quickstart-bazel.md (4)
12-14: C++14 and Bazel 7.0+ requirements are properly documented.The updated prerequisites correctly reflect the modern requirements for GoogleTest 1.16.0.
40-52: MODULE.bazel approach correctly replaces WORKSPACE.The documentation properly explains the transition from WORKSPACE to MODULE.bazel files, which is the recommended modern Bazel approach.
102-106: C++14 compilation flag documentation is accurate.The note about using
--cxxopt=-std=c++14and the MSVC equivalent--cxxopt=/std:c++14provides necessary guidance for users.
111-111: Build command includes required C++14 flag.The example build command correctly demonstrates the C++14 compilation flag usage.
src/3rdparty/googletest/BUILD.bazel (5)
39-63: Platform support additions are well-structured.The new config_setting declarations for QNX, FreeBSD, OpenBSD, and Fuchsia follow consistent patterns and use proper platform constraints.
108-108: QNX platform correctly excluded from pthread options.QNX doesn't use pthread the same way as other Unix-like systems, so excluding it from the pthread copts is correct.
127-137: Platform-specific linker options are appropriate.The linker options for QNX (regex), FreeBSD/OpenBSD (math and pthread libraries) are platform-appropriate configurations.
140-154: Abseil dependency expansion aligns with modern GoogleTest features.The additional Abseil modules (flags, debugging, containers, etc.) and RE2 dependency support advanced GoogleTest features when GTEST_HAS_ABSL is enabled.
161-167: Fuchsia SDK dependencies are correctly configured.The Fuchsia-specific dependencies (fdio, syslog, zx) are necessary for death tests and system interaction on Fuchsia platform.
src/3rdparty/googletest/googlemock/include/gmock/gmock-more-matchers.h (4)
43-45: Missing includes properly added.The explicit includes for
<ostream>and<string>fix potential compilation issues and follow good header hygiene practices.
64-86: IsEmpty matcher refactoring improves type safety and diagnostics.The polymorphic matcher implementation provides better error messages (reports actual size) and handles both STL containers and C-strings correctly.
93-95: Polymorphic matcher creation follows GoogleMock patterns.The
MakePolymorphicMatcherusage is the standard approach for creating polymorphic matchers in GoogleMock.
52-57: MSVC warning handling is properly scoped.The warning suppression is correctly limited to MSVC 2015 (version 1900) and uses the proper GoogleTest macros.
src/3rdparty/googletest/googlemock/test/gmock-cardinalities_test.cc (4)
34-34: Explicit include improves header dependencies.Adding the explicit
<ostream>include prevents potential compilation issues when headers change their internal dependencies.
55-61: Modern C++ features improve code clarity.Using
= defaultfor the constructor and= deletefor copy operations is clearer and more explicit than the old macro-based approach.
64-64: Single-line test body improves readability.Consolidating the simple test into a single line is more concise while maintaining clarity.
134-139: EXPECT_NONFATAL_FAILURE formatting is consistent.The consistent formatting of the failure expectation macros improves code readability across the test file.
src/3rdparty/googletest/ci/linux-presubmit.sh (5)
34-35: Container images updated to recent versions.The updates to 2024/2025 dated images look good. These newer images likely include updated toolchains that support the C++14 requirement.
42-42: C++11 removed from test matrix.Good change - this aligns with the PR objective of requiring C++14 as the minimum standard.
54-54: Fixed incorrect environment variable name.Nice catch!
CXXFLAGSis the correct variable name for C++ compiler flags, notCXX_FLAGS.
70-72: TODO indicates Bazel version limitation.The comment mentions using Bazel 5, which doesn't support Bzlmod. Since the rest of the script uses
--enable_bzlmod=truewith newer Bazel versions, this older test provides coverage for legacy WORKSPACE builds. Consider tracking when this can be upgraded.
83-84: Additional compiler warnings enabled.Adding
-Wuninitializedand-Wundefwarnings improves code quality by catching common issues. Good addition.Also applies to: 104-105, 129-130
src/3rdparty/googletest/docs/primer.md (5)
1-1: Consistent product name capitalization throughout.The change from "googletest" to "GoogleTest" improves documentation consistency and uses the proper product name.
Also applies to: 3-3, 5-5, 7-7
76-77: URLs updated to HTTPS.Good security practice to use HTTPS links instead of HTTP.
276-283: Test fixture uses constructor instead of SetUp().This is a cleaner approach for simple initialization. The constructor runs before each test just like SetUp(), but it's more idiomatic C++ when you don't need the virtual function flexibility.
Also applies to: 291-293
397-397: System header uses angle brackets.Correct change - GoogleTest is installed as a system library, so angle brackets are appropriate.
452-452: Removed unnecessary global namespace qualifier.The
::prefix isn't needed here since we're already in the global namespace. Cleaner without it.src/3rdparty/googletest/googlemock/src/gmock-internal-utils.cc (4)
59-75: API improved to separate keys and values.The new
JoinAsKeyValueTuplefunction provides a clearer API by accepting separate vectors for keys and values. The size check ensures they match, and the "key: value" format is more readable than the previous tuple format.
203-228: Compile-time base64 lookup table generation.This constexpr implementation generates the base64 decoding table at compile time. The web-safe encoding support (replacing '-' with '+' and '_' with '/') is a nice touch. The recursive template approach is clever but might be hard to debug if issues arise.
229-255: Base64 decoding implementation handles edge cases.The implementation correctly:
- Skips whitespace and padding ('=')
- Validates input characters
- Handles the bit manipulation for base64 decoding
The state machine approach with
bit_postracking is solid.
134-134: Flag access updated to new macro style.The change from direct flag access to
GMOCK_FLAG_GET(verbose)aligns with the broader refactoring to use consistent flag macros.Also applies to: 137-137
src/3rdparty/googletest/WORKSPACE (4)
31-31: Workspace name simplified.The change from "com_google_googletest" to "googletest" makes the workspace name cleaner and easier to reference in downstream projects.
33-34: Dependencies factored out to separate file.Loading
googletest_deps()from a separate file is good practice - it keeps the WORKSPACE file clean and makes dependency management more modular.
38-46: Python rules updated to v1.1.0.The rules_python update includes the SHA256 checksum for integrity verification. Good practice to pin both version and checksum.
48-61: Added bazel_skylib and platforms dependencies.These are standard Bazel dependencies that many projects need. The skylib provides common Starlark functions, and platforms enables better cross-platform builds.
src/3rdparty/googletest/docs/reference/matchers.md (5)
11-19: Important warning about implicit conversions added.This warning about
EXPECT_THAT(some_bool, "some string")potentially passing is crucial. The recommendation to use explicitEq()prevents subtle bugs from implicit conversions.
105-105: New base64 matcher documented.The
WhenBase64Unescaped(m)matcher corresponds to theBase64Unescapefunction added in gmock-internal-utils.cc. The RFC 4648 reference for web-safe format is helpful.
124-124: Contains matcher extended with Times modifier.The
.Times(n)modifier forContains(e)is a useful addition. The note aboutContains(e).Times(0)for testing absence is a clever use case.
201-201: ResultOf matcher enhanced with better error messages.Adding the
result_descriptionparameter allows for more informative error messages when the matcher fails. This improves debuggability.
292-302: DescribeMatcher usage example added.The example showing how to use
DescribeMatcher<>for nested matcher descriptions is helpful. This enables better error messages for complex custom matchers.src/3rdparty/googletest/googlemock/include/gmock/gmock-nice-strict.h (3)
60-61: IWYU pragma additions look good.These pragmas help tools understand the intended usage of this header.
114-119: Proper type safety improvement with uintptr_t.Converting the mock object pointer to
uintptr_tbefore passing to the registration functions is the right approach. This aligns with the internal implementation that uses these as keys in maps.Also applies to: 126-131, 138-143
185-186: Good modernization to C++11 style.Replacing
GTEST_DISALLOW_COPY_AND_ASSIGN_with explicit= deletedeclarations is cleaner and more explicit.Also applies to: 227-228, 269-270
src/3rdparty/googletest/googlemock/src/gmock-matchers.cc (3)
52-60: Enhanced matcher description formatting.The addition of
param_namesparameter and use ofJoinAsKeyValueTupleprovides more informative matcher descriptions by pairing parameter names with their values.
239-240: Clearer logging with explicit labels.Adding "element #" and "matcher #" prefixes makes the output more readable and unambiguous.
372-389: Cleaner logic separation for size mismatch handling.The refactored code separates the exact match size check from the general verification logic, making it easier to understand and maintain.
src/3rdparty/googletest/MODULE.bazel (1)
1-77: Proper migration to Bazel modules.This MODULE.bazel file correctly defines the googletest module with appropriate dependencies. The Python toolchain configuration and fake Fuchsia SDK setup provide necessary build infrastructure.
src/3rdparty/googletest/docs/reference/testing.md (3)
113-113: New ConvertGenerator documented.Good addition documenting the
ConvertGenerator<T>which allows static_cast of generated values to type T.
173-188: NameGenerator feature well documented.The example clearly shows how to provide custom type names for typed test suites. This improves test output readability.
1339-1341: Critical clarification for InitGoogleTest.Good catch documenting that
argvmust be null-terminated. This prevents potential crashes from improper argv handling.src/3rdparty/googletest/googlemock/src/gmock.cc (2)
95-125: Clean template consolidation of flag parsing.The templated approach for bool and string flag parsing reduces code duplication. The consistent
flag_nameparameter improves readability.
158-170: Macro approach reduces repetition effectively.The
GMOCK_INTERNAL_PARSE_FLAGmacro eliminates the need for separate if-statements for each flag. This makes adding new flags easier and reduces the chance of copy-paste errors.src/3rdparty/googletest/googlemock/include/gmock/internal/gmock-internal-utils.h (5)
66-67: Breaking change: Function renamed fromJoinAsTupletoJoinAsKeyValueTupleThis is an internal API change that better reflects the function's purpose. The new name clearly indicates it joins key-value pairs rather than generic tuples.
82-87: Good addition ofstd::reference_wrappersupportThe overload properly extracts the raw pointer from reference wrappers, maintaining consistency with the smart pointer handling pattern.
315-324: Solid improvement using compiler built-ins for unreachable codeReplacing the recursive call with
__builtin_unreachable()and__assume(0)provides better optimization hints to compilers and avoids potential stack overflow in debug builds.
424-440: Good modernization tostd::index_sequenceDropping the custom
MakeIndexSequenceimplementation in favor of standard library utilities reduces code complexity and improves maintainability.
482-482: Declaration without visible implementationThe
Base64Unescapefunction is declared here but the implementation isn't visible in this header. Ensure it's properly implemented in the corresponding .cc file.#!/bin/bash # Find the implementation of Base64Unescape rg -A 10 "Base64Unescape.*\{" --type cppsrc/3rdparty/googletest/googlemock/test/gmock-function-mocker_test.cc (2)
205-206: Good modernization: Explicit delete instead of macrosReplacing
GTEST_DISALLOW_COPY_AND_ASSIGN_with explicit= deletefor copy constructor and assignment operator is proper C++11 style. More readable and standard-compliant.Also applies to: 278-279, 495-496, 506-507, 562-563, 581-582, 652-653, 667-669, 719-721, 731-734, 766-768
953-967: Smart test for Clang unused member function warningThis test ensures that
MOCK_METHODdoesn't trigger-Wunused-member-functionwarnings when enabled as an error. Good defensive testing against regression.src/3rdparty/googletest/googlemock/include/gmock/internal/gmock-port.h (1)
73-138: Well-structured dual flag implementationThe conditional compilation cleanly supports both Abseil flags and legacy implementation. The abstraction through
GMOCK_FLAG_GET/SETmacros makes the client code agnostic to the underlying implementation. Good use ofstatic_assertto enforce semicolons after macro invocations.src/3rdparty/googletest/googlemock/test/gmock-internal-utils_test.cc (2)
73-90: Test properly updated for renamed functionThe test correctly reflects the API change from
JoinAsTupletoJoinAsKeyValueTuple, testing the key-value pair formatting functionality.
724-763: Comprehensive test coverage for Base64UnescapeGood coverage of edge cases: invalid strings, short strings with/without padding, and long strings with whitespace. The test data includes a nice literary quote for the long string test.
src/3rdparty/googletest/googlemock/include/gmock/gmock-more-actions.h (2)
595-598: Correct use of perfect forwardingChanged from pass-by-value to universal reference (
F&& f) withstd::forward. This properly preserves the value category of the callable, enabling move-only and reference-qualified function objects to work correctly.
602-614: Good improvements to InvokeArgumentActionTwo key improvements here:
- SFINAE constraint
(index < sizeof...(Args))prevents template instantiation errors- Moving the callable with
std::move(args_tuple.template Get<index>())supports move-only callablesThis makes the action more robust and efficient.
src/3rdparty/googletest/docs/gmock_cook_book.md (1)
288-291: Sentence is missing its verb – reader loses the threadThe paragraph drops the phrase “you can rewrite your code…”, leaving a dangling subordinate clause.
-Alternatively, instead of introducing a new interface -to accept a std::function instead of the free function, and then use +Alternatively, instead of introducing a new interface, you can rewrite your code +to accept a std::function instead of the free function, and then use [MockFunction](#MockFunction) to mock the std::function.Likely an incorrect or invalid review comment.
src/3rdparty/googletest/googlemock/test/gmock-matchers-arithmetic_test.cc (1)
1-1517: Good test coverage for arithmetic matchersThis new test file provides comprehensive coverage for arithmetic matchers including edge cases like NaN and infinity. The use of typed test fixtures for floating-point tests is a solid approach for testing different floating-point types systematically.
src/3rdparty/googletest/googlemock/include/gmock/gmock-function-mocker.h (3)
69-101: Smart compile-time validation for method specifiersThe addition of constexpr string utilities enables compile-time validation of mock method specifiers. This catches typos and invalid specifiers during compilation rather than runtime, which is a solid improvement for developer experience.
184-186: Logical qualifier ordering in mock method signaturesThe reordering places qualifiers in a more natural sequence:
const→ ref-qualifier →noexcept→override/final. This matches typical C++ declaration patterns better.
232-239: Cleaner calltype specifier handlingThe simplified calltype detection and unpacking follows the same pattern as other specifiers, making the code more consistent and maintainable.
Also applies to: 295-301
src/3rdparty/googletest/googlemock/src/gmock-spec-builders.cc (5)
551-607: Better type safety with uintptr_t for mock object identificationSwitching from
const void*touintptr_tfor mock object keys is a good move. It's more explicit about using the pointer value as an identifier rather than dereferencing it, and it works better with hash-based containers.
551-555: Hash map for better lookup performanceGood switch from
std::maptostd::unordered_mapfor the reaction map. Since ordering isn't needed here, the O(1) average-case lookup is preferable to O(log n).
55-57: Proper QURT OS platform supportThe addition of QURT-specific fatal exception handling is appropriate for this embedded RTOS environment where standard exit functions may not be available.
Also applies to: 531-536
411-425: More informative expectation failure messagesIncluding the expectation description in error messages makes it much easier to identify which expectation failed, especially when multiple expectations exist for the same method.
534-535: Correct use of _Exit to avoid reentrancyGood catch on using
_Exit(1)instead ofexit(1). Since this runs in a destructor at program termination, avoiding atexit handlers prevents potential reentrancy issues.src/3rdparty/googletest/googlemock/test/gmock-actions_test.cc (1)
1-2218: Third-party library update looks appropriateThis is part of the GoogleTest 1.1.16 update mentioned in the PR objectives. The extensive changes add C++14/17 feature support which aligns with the stated requirement for C++14.
The main additions include:
- Type traits tests for internal metaprogramming utilities
- Enhanced Return action tests for move semantics and reference types
- Comprehensive DoAll action tests with rvalue reference support
- Tests for actions with multiple call operators and move-only types
No issues found. The modernizations (like using
std::make_uniqueinstead ofnew) are good improvements.src/3rdparty/googletest/googlemock/include/gmock/gmock-spec-builders.h (6)
58-59: IWYU pragma additions look good.These pragmas help with Include-What-You-Use tooling to properly handle this internal header file.
412-413: Good type safety improvement with uintptr_t.Changing from
const void*touintptr_tfor mock object identifiers is a solid improvement. It's cleaner for use as map keys and prevents accidental pointer dereferences.Also applies to: 417-418, 422-423, 427-428, 433-433
715-718: Excellent debugging enhancement with expectation descriptions.The addition of human-readable descriptions for expectations will make test failures much easier to understand. The implementation properly uses move semantics and integrates well into error messages.
Also applies to: 785-788, 848-848, 927-932, 1224-1231, 1247-1251
1393-1435: Smart use of RAII for cleanup handlers.The destructor-based cleanup pattern ensures that uninteresting calls and failures are properly reported even if exceptions occur. This is more robust than the previous approach.
1001-1030: Clever overload design for OnceAction support.The dual overload approach with
ActionAdaptorproperly handles move-only actions while maintaining backward compatibility. The SFINAE technique ensures the right overload is selected.Also applies to: 1097-1105
1745-1896: Well-structured InvokeWith implementation despite complexity.The implementation properly handles all cases (uninteresting calls, matched expectations, excessive calls) with appropriate RAII cleanup. The
can_print_resulttype trait is a nice touch for conditionally printing results based on type capabilities.One observation: The comment about avoiding local callback types for compilation speed (lines 1837-1840, 1888-1890) is good documentation of a non-obvious design choice.
src/3rdparty/googletest/googlemock/include/gmock/gmock-actions.h (2)
1-2361: Third-party library update looks goodThis is a substantial update to Google Mock's action framework as part of the googletest 1.1.16 upgrade. The changes modernize the codebase to leverage C++14 features, add move-only action support via
OnceAction, and improve type safety throughout.The update aligns with the PR objectives stating that googletest 1.1.16 requires C++14. Since this is a vendored third-party library, the implementation details are Google's responsibility and appear well-tested.
269-380: Verify C++14 compiler requirement is enforcedThe new metaprogramming utilities (negation, conjunction, disjunction, etc.) backport C++17 features for C++14 compatibility. Since googletest 1.1.16 requires C++14, ensure the project's CMake configuration properly enforces this requirement.
#!/bin/bash # Check if CMakeLists.txt enforces C++14 or higher rg -A 5 -B 5 'CMAKE_CXX_STANDARD|cxx_std_|CXX_STANDARD' CMakeLists.txt
| make VERBOSE=1 | ||
| make check VERBOSE=1 | ||
| make test VERBOSE=1 | ||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer ctest --output-on-failure for richer diagnostics
make test is fine, but calling ctest directly prints failing test names and captured output, which speeds up debugging in CI logs.
- make test VERBOSE=1
+ ctest --output-on-failureOptional, but improves signal-to-noise in large test suites.
Also applies to: 49-51
🤖 Prompt for AI Agents
In .github/workflows/build.yml around lines 30 to 32 and also lines 49 to 51,
replace the `make test VERBOSE=1` commands with `ctest --output-on-failure` to
get richer diagnostics. This change will print failing test names and their
captured output directly in the CI logs, improving debugging efficiency. Keep
the `make VERBOSE=1` commands as they are.
| set(CMAKE_CXX_STANDARD 14) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
C++14 is set but not enforced
You bumped the standard to 14, good call.
Without CMAKE_CXX_STANDARD_REQUIRED ON CMake will silently downgrade on toolchains that default to an older standard and you'll hit compiler errors later. Fail fast instead.
set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)🤖 Prompt for AI Agents
In CMakeLists.txt around lines 30 to 31, the C++14 standard is set but not
enforced, which can cause CMake to silently downgrade the standard on some
toolchains. Add the line `set(CMAKE_CXX_STANDARD_REQUIRED ON)` immediately after
setting `CMAKE_CXX_STANDARD` to ensure the compiler strictly uses C++14 and
fails early if unsupported.
The GoogleTest module was introduced in version 3.9, gtest_discover_tests() in 3.10.
|
To not have the googletest code in your repo, you could use the CMake The setup is then very simple, see https://google.github.io/googletest/quickstart-cmake.html#set-up-a-project |
|
Thank you for this. |
|
@lalinsky Just received a bunch of notification mails from coderabbit. Wasn't sure if just setting resolved without a comment was ok. Sorry for spamming! |
Yeah, it's completely fine to resolve them without a comment. I find CodeRabbit useful for my own changes, but it gets spammy when you update external code like this. Thank you for the changes. |
2 participants
Changes included in this PR:
enable_testing()in top-level CMakeLists.txt for compatibility withctestfind_package(GTest)useinclude(GoogleTest)Threads(not needed)gtest_discover_tests(...)BUILD_SHARED_LIBSisON, copy DLLs to tests bin dir.FFmpegAudioReaderTestfailing with FFmpeg v7 (closes ffmpeg 7.0 support #142)checkwas removed (a target namedtestis available by default)cmake_minimum_requiredto 3.10. TheGoogleTestmodule was introduced in version 3.9,gtest_discover_tests(...)in 3.10.The changes were tested on Debian 12 (FFmpeg version 5.1.6) and Windows 10 (FFmpeg version 7.0.1). All combinations release/debug and shared/static were tested.
The project can be built and tested with:
Notes:
if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.21.0"). Wasn't sure if a general update ofcmake_minimum_requiredwould be ok.