Skip to content

Fix remote configs hash caching, custom fields validation; add event tests - #55

Merged
Gogoshika-ga merged 28 commits into
mainfrom
fix/remote-configs-hash-and-listener-races
Sep 1, 2026
Merged

Fix remote configs hash caching, custom fields validation; add event tests#55
Gogoshika-ga merged 28 commits into
mainfrom
fix/remote-configs-hash-and-listener-races

Conversation

@Gogoshika-ga

@Gogoshika-ga Gogoshika-ga commented Aug 31, 2026

Copy link
Copy Markdown
Member

Fixes

  • configs_hash cleared on every launch: ensurePersistedStates erased the cached hash whenever last_used_identifier existed, instead of only when the identifier changed (matching the iOS/C# SDKs). As a result the init request always sent an empty configs_hash and the backend re-sent full configs every session.
  • Thread safety: remote configs listeners and _gameRemoteConfigsJson were written without the mutex that readers hold; listeners are now notified from a snapshot outside the lock.
  • Custom fields: a non-string value under an illegal key threw while logging the rejection, silently discarding the entire custom fields payload.
  • Coverage job: gcov counters are now updated atomically (-fprofile-update=atomic); the SDK's worker thread produced torn counters that newer lcov rejects.

Tests

  • Regression tests for configs_hash persistence (kept on same identifier, cleared on change) — verified to fail against the old code.
  • Ported the old commented-out custom fields validation test to nlohmann, asserting current behavior (booleans are now accepted) against the MAX_CUSTOM_FIELDS_* limits, plus a regression test for the discarded-payload bug.
  • New GAEventsTests.cpp: event store and send queue tests using a mock HTTP client — storing/annotating all event types, transaction/attempt counters, and the flush path (batch send, retry on no response, drop on server error).
  • Event submission is disabled suite-wide in test/main.cpp (re-enabled per-fixture against the mock), so tests can never send events to the GA servers.

Local coverage: 34.8% → 54.9% lines (GAEvents.cpp 7.6% → 54.6%).

Code coverage (new setup)

Replaces the removed lcov/genhtml setup with gcovr:

  • Local: python3 setup.py --platform <p> --build --test --coverage builds instrumented, runs the tests, and opens an HTML report (build/coverage/index.html). Requires gcovr (e.g. brew install gcovr).
  • Shared config: filters and exclusions (SDK sources only, Platform/ and vendored deps excluded, throw-branches excluded) live in gcovr.cfg, used identically by local runs and CI.
  • CI (coverage.yml): single ubuntu-24.04/gcc job on PRs to main, pushes to main, and manual dispatch. Gates: line ≥ 60, branch ≥ 50, function ≥ 80 (~5pts under the measured gcc baseline of 63.4/55.5/84.6). Emits the summary to the job log/summary, uploads the detailed HTML as an artifact (also when the gate fails), uploads to Codecov via tokenless OIDC, and publishes the report to GitHub Pages on pushes to main. Report steps are skipped when the build itself fails.
  • README: Codecov badge + --coverage docs.

@Gogoshika-ga
Gogoshika-ga force-pushed the fix/remote-configs-hash-and-listener-races branch from a0a915b to 0c1a806 Compare August 31, 2026 13:30
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

LCOV of commit 954f28d during Test Coverage #170

📊 Summary Coverage

Metric Coverage Details
📈 Lines 🔴 69.5% 2309 / 3323 lines
🔧 Functions 🟡 84.3% 418 / 496 functions
🌿 Branches 🔴 40.7% 2286 / 5613 branches

📁 Files Changed Coverage

File Lines Functions Branches
include/GameAnalytics/GAHttpClient.h 🔴 0.0% (4) 🔴 0.0% (3) 🔴 0.0% (2)
source/gameanalytics/GAState.cpp 🟡 85.6% (599) 🟢 94.0% (84) 🔴 46.9% (1323)
source/gameanalytics/GAState.h 🟢 91.2% (34) 🟡 88.9% (9) 🔴 60.0% (50)

🟢 ≥ 90% · 🟡 ≥ 70% · 🔴 below 70% · ⚪ no data
Full coverage report

@Gogoshika-ga Gogoshika-ga changed the title Fix remote configs hash caching and custom fields validation Fix remote configs hash caching, custom fields validation; add event tests Aug 31, 2026
@codecov-commenter

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@Gogoshika-ga
Gogoshika-ga requested review from dorin-ga and a lite review from Copilot September 1, 2026 08:10
@Gogoshika-ga Gogoshika-ga self-assigned this Sep 1, 2026
@Gogoshika-ga Gogoshika-ga added the bug Something isn't working label Sep 1, 2026
@Gogoshika-ga
Gogoshika-ga merged commit 870c0a5 into main Sep 1, 2026
27 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the C++ SDK’s remote-config caching and listener handling to match expected cross-session behavior, hardens custom-fields validation, and introduces broader regression/integration tests alongside a new gcovr-based coverage workflow.

Changes:

  • Fix configs_hash persistence across launches (only clear when the user identifier changes) and adjust remote-config listener notifications for safer concurrency.
  • Fix a custom-fields validation/logging edge case where an invalid entry could discard the entire payload.
  • Add new GAState / GAEvents / public-facade integration tests and replace coverage reporting with gcovr + CI gating/upload.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/main.cpp Disables event submission by default for the whole test suite.
test/helpers/GAStateTestAccessor.h Adds a test-only accessor to drive GAState transitions deterministically.
test/GAStateTests.cpp Adds regression tests for configs hash persistence and custom-fields validation behavior.
test/GameAnalyticsApiTests.cpp Adds integration tests for the public GameAnalytics facade using a mock HTTP client.
test/GAEventsTests.cpp Adds unit tests for event storage/flush behavior and retry/drop semantics.
source/gameanalytics/GAState.h Exposes GAState internals to tests via a new friend declaration.
source/gameanalytics/GAState.cpp Fixes configs-hash clearing logic; adds locking/snapshotting for remote-config listeners; hardens custom-fields logging.
source/gameanalytics/GACommon.h Bumps SDK version string to cpp 5.4.1.
setup.py Adds gcovr coverage report generation and a CI-friendly --no_cov_report flag.
README.md Adds Codecov badge and updates --coverage documentation.
gcovr.cfg Defines shared gcovr filters/exclusions for local and CI coverage runs.
CMakeLists.txt Replaces lcov targets with compiler/linker coverage instrumentation flags.
CHANGELOG.md Documents fixes included in 5.4.1.
.gitignore Ignores Python bytecode (*.pyc).
.github/workflows/coverage.yml Adds a dedicated gcc/ubuntu coverage job with gating, artifacts, Codecov upload, and Pages publishing on main.
.github/workflows/cmake.yml Adds vcpkg binary caching to speed up CI builds.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/main.cpp
Comment on lines 1 to +4
#include <gtest/gtest.h>

#include <GAState.h>

Comment on lines 78 to 83
friend class device::GADevice;
friend class logging::GALogger;
friend class store::GAStore;
friend class http::GAHTTPApi;
friend struct GAStateTestAccessor;

Comment thread CMakeLists.txt
Comment on lines +284 to +285
target_compile_options(GameAnalytics PRIVATE -g -O0 --coverage -fprofile-update=atomic)
target_link_options(GameAnalytics PUBLIC --coverage)
Comment thread README.md
Comment on lines +47 to 48
| `--coverage` | — | Build with coverage instrumentation and generate an HTML report in `build/coverage/` (requires `--build --test`; not available with `--shared`; needs [gcovr](https://gcovr.com) installed) |


// ---- health tracking facade ----

TEST_F(GameAnalyticsApiTest, HealthTrackingtogglesAreForwardedToTracker)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants