Skip to content

MockRedisAdapter has drifted from the adapter API and is never compiled in CI #124

Description

@bigsamich

RedisAdapter version or revision

main at ff2d5b8. mock/MockRedisAdapter.hpp is byte-identical through the #108 → #109 → #111 stack.

Redis version and deployment mode

Not applicable (compile-time).

Observed behavior

With MOCK_REDIS_ADAPTER defined, RedisAdapter.hpp includes only mock/MockRedisAdapter.hpp and aliases RedisAdapter to the mock. The mock redefines RA_Time, but:

  • It declares neither RA_NOT_CONNECTED (pre-existing) nor RA_REJECTED (added by Separate rejected stream writes from transport failures #109). Code that follows docs/api.md, for example t.err() == RA_REJECTED.err(), compiles against the real adapter and fails with the mock: error: 'RA_REJECTED' was not declared in this scope (same for RA_NOT_CONNECTED). The mock also cannot simulate a rejection.
  • It lacks RA_Options and all of the stack's new API: subscribeStream, ReaderHandle and status(), ReaderStatus, getStreamSnapshot. It also lacks the new pure helpers and types (decodeScalar, decodeArray, compareStreamIds, StreamEntry/StreamBatch), which need no connection and could be shared at no cost.
  • Over-allocation in MockRedisAdapter list functions using size_bytes() instead of size() #81 is worse than filed. Under the project's C++17, no instantiation of either addSingleList overload compiles: std::vector, std::array and the spanCpp14.hpp span have no size_bytes(). Only a C++20 std::span compiles, and it allocates sizeof(T) times too much (48 bytes for 12).
  • RedisCache.hpp can't even be included in a mock build. _ra->addListsReader/getSingleList are resolved at template-definition time and the mock has neither. Under C++17 the header also fails earlier, at std::atomic<bool>, because it relies on <atomic> (and <thread>, <mutex>, <algorithm>) arriving transitively from the real adapter header. So consumers that use RedisCache can't unit-test with the mock at all.
  • The mock declares, but never defines, RA_Time(const std::string&), id() and id_or_now(), so a mock-only test that formats an ID fails to link.

Nothing in CMake or CI builds a translation unit with -DMOCK_REDIS_ADAPTER, which is how this drifted. #106/#107 show that the mock has users.

Related, affecting the real header too: t == RA_REJECTED does not compile (ambiguous operator==, because RA_Time has two implicit integer conversions). Both conversions map every error to 0, so int64_t(t) == int64_t(RA_REJECTED) is also true for a transport failure. Callers are left with the magic number err() == 2.

Reproduction

#define MOCK_REDIS_ADAPTER
#include "RedisAdapter.hpp"
bool rejected(const RA_Time& t) { return t.err() == RA_REJECTED.err(); }   // error: 'RA_REJECTED' was not declared

g++ -std=c++17 -fsyntax-only against main or the stack head, with the adapter's include directories, fails as shown. The same happens with RA_NOT_CONNECTED, and with any instantiation of MockRedisAdapter::addSingleList.

Build and runtime environment

g++ 13.3 and clang++ 18.1, C++17 and C++20.

Suggested fix

  • Move RA_Time, the sentinels, the stream types and the pure helpers into a small header that both RedisAdapter.hpp and the mock include.
  • Add friend bool operator==/!=(const RA_Time&, const RA_Time&) comparing value, or named predicates. Validated with gcc and clang; ctest 17/17.
  • Let mock writes return a configurable result, so consumers can unit-test their rejection handling.
  • Fix Over-allocation in MockRedisAdapter list functions using size_bytes() instead of size() #81: const size_t count = data.size(); int dataSize = count * sizeof(T); new T[count]. Validated: all six vector/array/span × C++17/C++20 combinations compile and run clean under ASan+UBSan.
  • Give the mock recording/replay versions of addListsReader/addValuesReader/getSingleList, or ship a MockRedisCache. Make RedisCache.hpp include what it uses and drop the unused <semaphore>. Define the mock's RA_Time helpers inline.
  • Add a CI step that compiles a small consumer translation unit with -DMOCK_REDIS_ADAPTER, including RedisAdapter.hpp and RedisCache.hpp.
  • Decide whether the mock should mirror the owned-subscription API. It needs at least a subscribeStream that stores the callback plus a hook to inject batches. Otherwise, document that it is absent.

Related: #80 (fixed by #107), #81, #106, #109.

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions