RUM-17613: timeseries [5/5] Add integration tests for timeseries collection - #3718
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Please upload reports for the commit 9d15921 to get more accurate results. Additional details and impacted files@@ Coverage Diff @@
## tvaleev/feature/RUM-17613-3b-background-suspend #3718 +/- ##
================================================================================
Coverage 73.34% 73.34%
================================================================================
Files 1012 1012
Lines 37087 37087
Branches 6392 6392
================================================================================
Hits 27201 27201
Misses 8045 8045
Partials 1841 1841 🚀 New features to boost your workflow:
|
2c34b26 to
61448ac
Compare
94ca6d7 to
b3faf94
Compare
satween
changed the title
timeseries [4/4] Add integration tests for timeseries collection
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3faf94903
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| androidTestImplementation(project(":features:dd-sdk-android-trace-internal")) | ||
| androidTestImplementation( | ||
| (testFixtures(project(":features:dd-sdk-android-rum")) as ModuleDependency).apply { | ||
| exclude(mapOf("group" to "dd-sdk-android-timeseries.tools", "module" to "unit")) |
There was a problem hiding this comment.
When the repository is checked out under its normal dd-sdk-android directory, Gradle derives this subproject's group as dd-sdk-android.tools, so this dd-sdk-android-timeseries.tools exclusion does not match. The JVM-flavored transitive :tools:unit dependency therefore remains alongside the integration module's direct ART-flavored dependency, which can break androidTest variant resolution; exclude the project dependency without depending on the clone directory name.
Useful? React with 👍 / 👎.
| private const val EXPECTED_FOREGROUND_MEMORY_DATA_POINTS = | ||
| ((FOREGROUND_COLLECTION_DURATION_MS + ACTIVITY_STOP_DELAY_MS) / SAMPLE_INTERVAL_MS).toInt() | ||
| private const val EXPECTED_FOREGROUND_CPU_DATA_POINTS = | ||
| EXPECTED_FOREGROUND_MEMORY_DATA_POINTS - 1 |
There was a problem hiding this comment.
On a slower or busier emulator, the collector can be scheduled before the test method begins—or its asynchronous session startup can occur after the sleep begins—so the number of sampling intervals is not determined solely by FOREGROUND_COLLECTION_DURATION_MS + ACTIVITY_STOP_DELAY_MS. The resulting batch can contain fewer or more points than this formula predicts, making both tests intermittently fail despite correct collection; synchronize against the first sample/collector state or assert a timing-tolerant range instead.
Useful? React with 👍 / 👎.
|
|
||
| @RunWith(AndroidJUnit4::class) |
There was a problem hiding this comment.
When this module's tests run on its declared minimum SDK of API 23 without the optional Java backport property, the AssertJ calls used throughout this class fail while initializing ConfigurationProvider because java.util.Spliterators is unavailable. Since @LargeTest does not impose an SDK floor, the runner still executes both tests; add an API-24 @SdkSuppress (or ensure the backport is always enabled) so the documented integration command does not add guaranteed failures on a supported test device.
Useful? React with 👍 / 👎.
Add a playground activity and an instrumented test that assert CPU and memory timeseries are collected, flushed on backgrounding and resumed on return to foreground. Ref: RUM-17613
61448ac to
341c6b1
Compare
b3faf94 to
9d15921
Compare
satween
changed the title
timeseries [4/4] Add integration tests for timeseries collectiontimeseries [5/5] Add integration tests for timeseries collection
2 participants
What does this PR do?
Adds
TimeseriesTrackingPlaygroundActivityand an instrumentedTimeseriesCollectionTestthat runthe real collection pipeline against the mock RUM server: CPU and memory batches are emitted with
the expected sample counts, leaving the foreground flushes the pending batch, nothing is collected
while backgrounded, and collection resumes on return to the foreground.
Motivation
The unit tests cover the pipeline pieces in isolation; nothing exercised the whole executor-driven
path with real readers and real lifecycle transitions, which is where the suspend/flush timing
actually matters.
Additional Notes
The integration module consumes
testFixtures(project(":features:dd-sdk-android-rum"))and excludestools:unit, a JVM-only test module that must not land on the Android test classpath;gsonandjUnit4are added astestFixturesImplementationso the androidTest consumer still has them atruntime.
Requires a connected device or emulator on API 24+:
./gradlew :instrumented:integration:connectedDebugAndroidTest. Verified on API 28 — both testspass, three consecutive runs. The suite cannot run on API 23: every AssertJ assertion dies in
ConfigurationProvider.<clinit>withNoClassDefFoundError: Ljava/util/Spliterators;. That is notspecific to this PR — the pre-existing
security.EncryptionTestfails identically on the same AVD.Review checklist (to be filled by reviewers)
Ref: RUM-17613