Skip to content

CI and build hygiene: C++20 CI build is really C++17, Dependabot misconfigured, sanitizer job blocked by test races #125

Description

@bigsamich

Problem

These surfaced while reviewing #100-#103 and #108/#109/#111. Each item is small, verified, and independent of the open PRs. Checked on main at ff2d5b8 unless stated.

  • The C++20 CI configuration builds C++17. test-and-benchmark.yml passes -DCMAKE_CXX_STANDARD=20, but CMakeLists.txt:10 set(CMAKE_CXX_STANDARD 17) sets a normal variable that shadows the cache entry. Every project target still compiles with -std=gnu++17 (checked in build.ninja), so the C++20 path of RedisCache.hpp (which docs/building.md says needs C++20) is never built in CI. Fix: if(NOT DEFINED CMAKE_CXX_STANDARD) set(CMAKE_CXX_STANDARD 17) endif(). Validated: C++20 objects, suite passes.
  • Dependabot's docker ecosystem fails every month. Every run since the entry was added (2026-07-15, 2026-08-01, 2026-09-01) ends with "No Dockerfiles nor Kubernetes YAML found in /" (dependency_file_not_found). So redis:7.4.2-alpine in docker-compose.test.yml never gets update PRs. Fix: package-ecosystem: docker-compose, whose file fetcher matches docker-compose.test.yml.
  • Dependabot's gitsubmodule updater cannot follow two of our upstreams. It proposes the newest tag reachable from each submodule's default branch and does not compare it with the pinned version.
    • googletest tags v1.15+ off main, so Bump googletest from b514bdc to f8d7d77 #101 is a downgrade (v1.15.2 → v1.14.0, 149 commits behind the pin).
    • hiredis tags patch and security releases on release branches, so v1.4.1 and v1.3.1 (2026-08-06) will never be offered.
    • docs/building.md already treats pins as deliberate release inputs. Suggest ignore entries for googletest and hiredis, with manual bumps on upstream release announcements.
  • A sanitizer CI job is blocked only by test-code races. Every TSan report in the suite comes from the plain bool waiting flags in test.cpp (lines 221, 267, 372, 443), which callbacks write on worker threads while the test body polls them. With those four flags as std::atomic<bool>, the Add owned stream subscriptions and safe payload decoding #108 stack is TSan-clean (20/20 runs, 14/14 tests), while main still shows the library races that Add owned stream subscriptions and safe payload decoding #108 fixes. After that, ASan+UBSan and TSan jobs could be added. TSan binaries need setarch -R on current kernels ("unexpected memory mapping" otherwise).
  • The older gtest cases aren't parallel-safe. They share the base key TEST, so ctest -j8 fails MultiReader/MultiWorker/Data* in 6/10 runs on both main and the stack head. CI runs serially, which hides it. Per-test base keys fix it.
  • Stacked PRs get no CI. test.yml restricts pull_request to branches: [main], so Separate rejected stream writes from transport failures #109 and Recover owned readers across observed stream resets #111 have no checks, and only manual workflow_dispatch runs exist. Dropping the branch filter, or adding dev/**, would cover stacked PRs.
  • persist-credentials: false on the three checkout steps. release.yml runs with contents: write, and no step needs the persisted token: the git fetch works unauthenticated on this public repo, and gh uses GH_TOKEN.
  • redis-start.sh reports success when port 6379 or /tmp/redis.sock is already in use. It then leaves the existing server in place, and ctest runs against it. With Recover owned readers across observed stream resets #111's recovery test (which kills every client connection on the server; see the Recover owned readers across observed stream resets #111 review) that becomes disruptive on a workstation with a live Redis. Make the script fail when the port or socket is taken, and repeat docs/building.md's "isolated machine" warning in README.md.

Proposed behavior

Each checkbox above names its fix.

Compatibility scope

Documentation or tooling only.

Compatibility considerations

None for consumers. These are build, CI and test changes only; the googletest/hiredis decisions affect only when pins move.

Activity

  1. derekste commented on Oct 1, 2026

    @derekste
    Member

    @bigsamich the eight items now have concrete fixes in #127/#108 and follow-up #128:

    The Linux parallel suite and GCC 15.2 TSan suite each pass all 107 standalone cases on Redis 7.4.2. GitHub checks are running on #128; its normal and TSan jobs are already green. Review requested on the follow-up. Leaving this issue open until the changes are reviewed and merged.

  2. derekste commented on Oct 1, 2026

    @derekste
    Member

    @bigsamich all checks on #128 at 15c6734612f77b77019d9dc1e96fb101e0c43b0b are now green: normal build/test, ASan+UBSan, and TSan. The normal job exercises parallel CTest and the private Cluster fixture. #129's security-release replacement and #102's benchmark update are also green at their posted heads.

    The changes are ready for another review; #125 remains open for review and merge tracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions