Analytics: Log triple find times and game completion stats#220
Conversation
- Added DURATION and TRIPLES_FOUND to AnalyticsConstants. - Updated BaseGameActivity to log individual triple find durations. - Updated BaseGameActivity to log total game duration and triples found on completion. - Added verification in GameFlowTest and GameEndFlowTest. Co-authored-by: amorris13 <4523811+amorris13@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe changes add analytics tracking for game timing and triple counts by introducing two new parameter constants ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~14 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/test/java/com/antsapps/triples/GameFlowTest.java (1)
74-74: Assertdurationvalue, not just key presenceThis currently passes even if
durationis present but wrong. Add a value-level assertion (e.g., non-negative) to make the test meaningful.Proposed test tightening
- assertThat(capturedBundle.containsKey(AnalyticsConstants.Param.DURATION)).isTrue(); + assertThat(capturedBundle.containsKey(AnalyticsConstants.Param.DURATION)).isTrue(); + assertThat(capturedBundle.getLong(AnalyticsConstants.Param.DURATION)).isAtLeast(0L);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/test/java/com/antsapps/triples/GameFlowTest.java` at line 74, The test currently only checks the presence of the DURATION key; update GameFlowTest to also assert the actual duration value is valid by retrieving the value from capturedBundle (e.g., capturedBundle.getLong(AnalyticsConstants.Param.DURATION) or getDouble depending on type) and using assertThat(...) to assert it is non-negative (assertThat(duration).isGreaterThanOrEqualTo(0)). Ensure you reference AnalyticsConstants.Param.DURATION and capturedBundle when adding the assertion.app/src/test/java/com/antsapps/triples/GameEndFlowTest.java (1)
103-107: Strengthen completion analytics assertions with value checksPresence checks are helpful, but asserting actual values will catch payload regressions more reliably.
Proposed assertion upgrade
Bundle capturedBundle = bundleCaptor.getValue(); assertThat(capturedBundle.getString(AnalyticsConstants.Param.GAME_TYPE)) .isEqualTo(ClassicGame.GAME_TYPE_FOR_ANALYTICS); assertThat(capturedBundle.containsKey(AnalyticsConstants.Param.DURATION)).isTrue(); assertThat(capturedBundle.containsKey(AnalyticsConstants.Param.TRIPLES_FOUND)).isTrue(); + assertThat(capturedBundle.getLong(AnalyticsConstants.Param.DURATION)).isAtLeast(0L); + assertThat(capturedBundle.getLong(AnalyticsConstants.Param.TRIPLES_FOUND)) + .isEqualTo(game.getNumTriplesFound());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/test/java/com/antsapps/triples/GameEndFlowTest.java` around lines 103 - 107, The test currently only asserts presence of DURATION and TRIPLES_FOUND keys; update GameEndFlowTest to assert their actual values: retrieve values from capturedBundle (e.g., capturedBundle.getLong(AnalyticsConstants.Param.DURATION) and capturedBundle.getInt(AnalyticsConstants.Param.TRIPLES_FOUND) or getString if appropriate) and compare them to the expected values used in the test (use existing variables or compute expectedDuration/expectedTriplesFound from the test setup) with assertThat(...).isEqualTo(...); keep the existing assert for GAME_TYPE using ClassicGame.GAME_TYPE_FOR_ANALYTICS and replace the containsKey assertions with these value equality assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/test/java/com/antsapps/triples/GameEndFlowTest.java`:
- Around line 103-107: The test currently only asserts presence of DURATION and
TRIPLES_FOUND keys; update GameEndFlowTest to assert their actual values:
retrieve values from capturedBundle (e.g.,
capturedBundle.getLong(AnalyticsConstants.Param.DURATION) and
capturedBundle.getInt(AnalyticsConstants.Param.TRIPLES_FOUND) or getString if
appropriate) and compare them to the expected values used in the test (use
existing variables or compute expectedDuration/expectedTriplesFound from the
test setup) with assertThat(...).isEqualTo(...); keep the existing assert for
GAME_TYPE using ClassicGame.GAME_TYPE_FOR_ANALYTICS and replace the containsKey
assertions with these value equality assertions.
In `@app/src/test/java/com/antsapps/triples/GameFlowTest.java`:
- Line 74: The test currently only checks the presence of the DURATION key;
update GameFlowTest to also assert the actual duration value is valid by
retrieving the value from capturedBundle (e.g.,
capturedBundle.getLong(AnalyticsConstants.Param.DURATION) or getDouble depending
on type) and using assertThat(...) to assert it is non-negative
(assertThat(duration).isGreaterThanOrEqualTo(0)). Ensure you reference
AnalyticsConstants.Param.DURATION and capturedBundle when adding the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 628d5511-5e2f-4bdd-adc1-d78d6bf858bc
📒 Files selected for processing (4)
app/src/main/java/com/antsapps/triples/AnalyticsConstants.javaapp/src/main/java/com/antsapps/triples/BaseGameActivity.javaapp/src/test/java/com/antsapps/triples/GameEndFlowTest.javaapp/src/test/java/com/antsapps/triples/GameFlowTest.java
1 participant
This PR enhances analytics logging by adding duration information to triple discovery events and total duration and triple counts to game completion events. This provides better insight into user performance and game progression across different modes.
Fixes #219
PR created automatically by Jules for task 7196037304184557660 started by @amorris13
Changes Overview
This PR enhances analytics logging to capture duration metrics and completion statistics for improved insight into user performance and game progression.
Key Changes
Analytics Constants (
AnalyticsConstants.java)DURATIONparameter constant ("duration")TRIPLES_FOUNDparameter constant ("triples_found")Game Activity Logging (
BaseGameActivity.java)logTripleFoundEvent()to compute and log the duration for each triple discovery, calculated as the time difference between consecutive entries in the triple find times listgameFinished()to log comprehensive game completion analytics including:Test Coverage
FIND_TRIPLEevents include thedurationparameterFINISH_GAMEevents contain the expectedGAME_TYPE,DURATION, andTRIPLES_FOUNDparametersHigh Priority Review Comments
CC: @jules