Repository navigation
test: [SDK-5538] guard stand-ins and the public API against drift - #2828
Draft
abdulraqeeb33 wants to merge 4 commits into
Draft
abdulraqeeb33 wants to merge 4 commits into
abdulraqeeb33 wants to merge 4 commits into
Conversation
This was referenced Nov 2, 2026
Contributor
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)23/31 touched executable lines covered (74.2% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
❌ Coverage Check FailedAggregate coverage on touched lines is 74.2% (minimum 80%). |
abdulraqeeb33
force-pushed
the
never-crash/5-api-coverage-tests
branch
from
November 2, 2026 17:46
c7b0773 to
7d39a53
Compare
8 of 18 tasks
DeferredManagersCoverageTests: every member of the six manager interfaces must be declared on its stand-in, not inherited from a default body. Every add...Listener/Observer and every var must have a replay case, and each case must queue before init and reach the real manager on replay. OneSignalImpV2ApiSweepTests: every IOneSignal member, and every member of the managers handed out before init, is called reflectively with dummy args and must not throw or hang past 5s. Members that need a non-null Context are skipped, since they would start a real init. Co-authored-by: Cursor <cursoragent@cursor.com>
…egacy and V2 behavior The sweep now runs after a failed init as well, enforces its deadline from a separate thread so a blocking wait cannot escape withTimeout, and passes null for every required parameter to pin the Java boundary: nothing hangs and nothing but the Kotlin NullPointerException surfaces. OneSignalImpParityTests runs the same scenarios on OneSignalImp and OneSignalImpV2 and asserts the app-visible results match. Co-authored-by: Cursor <cursoragent@cursor.com>
abdulraqeeb33
force-pushed
the
never-crash/5-api-coverage-tests
branch
from
November 5, 2026 21:12
7d39a53 to
135f3a2
Compare
This was referenced Nov 5, 2026
…y points V2 logs and returns. Legacy still throws the same NullPointerException. Drop the isMissing overload that only built the log string. Co-authored-by: Cursor <cursoragent@cursor.com>
V2 returns after the log. When the SDK is not initialized, the log says so. After a successful init, a null externalId is still rejected without that sentence. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack (base
feat/never-crash, merged tomainas one unit; split out of #2793):Description
One Line Summary
Tests that fail when a new public method is added without deciding how it behaves before init, and a parity harness that fails when V2 drifts from the legacy implementation after init.
externalId,context, andappIdare the nullable public arguments.Details
Motivation
Part of SDK-5538. The compiler already forces every new abstract interface method into its stand-in, but not a method with a default body, not whether a new listener is queued before init, and not whether a new
IOneSignalmethod checks the init gate. These tests cover those gaps.Scope
Public entry points:
externalIdonlogin,loginSuspend,updateUserJwt, andupdateUserJwtSuspend, andcontexton the single-argumentinitWithContext, are nullable.contextandappIdon the two-argument init methods already were. Token, listeners, and the user profile stay non-null.context,appId, orexternalIdin the method that accepts it and returns. The log names the argument. When the SDK is not initialized, the log says so. A nullappIdwith a cached app ID uses that ID and the log says so. After a successful init, a nullexternalIdis still rejected, without claiming the SDK is down. Empty and null-byte values still log and return.OneSignalImpthrows the sameNullPointerExceptionmessage Kotlin used to throw for a null argument.isMissing(value, operation, parameter)overload is removed. Call sites pass the log string ("login: externalId").Tests, three files:
DeferredManagersCoverageTestsadd...Listener,add...Observerand settable property must be listed in the replay table. A new one fails until someone decides whether it queues before init.OneSignalImpV2ApiSweepTestsIOneSignalmember, called with dummy arguments before init and again after a failed init, must not throw or hang past 5 seconds. The deadline is enforced from a separate thread, so a blocking wait inside the member cannot escape it.User,pushSubscription,Notifications,InAppMessages,Location,Session).nullin turn (the Java boundary). Nothing may hang, and nothing but aNullPointerExceptionmay surface.externalId,context, andappIdare nullable: V2 logs and returns, and legacy throws the sameNullPointerException.OneSignalImpParityTestsOneSignalImpandOneSignalImpV2with clean prefs and asserts the app-visible results are equal: init then every top-level read; consent and GMS flags set before init reach the config model; a secondinitWithContextkeeps the first app ID; login, tags, logout; empty app ID falls back to the cache; null app ID (suspend) resolves from the cache.Not covered
Contextare skipped, since they would start a real init. The nullable-context variants are swept.IDebugManageris a real object before init; only thedebuggetter is called.Testing
Unit testing
Each drift test was checked against a deliberate breakage, and all went red naming it: a new default-body method on
ISessionManager, a new default-body listener onIInAppMessagesManager,isSharedno longer queued, a stand-in method that throws,logout()throwing before init, and a suspend getter that hangs. The parity harness went red on test pollution from the 200 ms prefs flush before the scenarios were isolated, which is the failure shape it is meant to produce. The earlier full run was 1854 tests across core, location, in-app-messages, and notifications, with spotless, detekt, and apiCheck.The nullable-argument commit re-ran
InputGuardTests,OneSignalImpTests,OneSignalImpV2Tests, andOneSignalImpV2ApiSweepTests(49 tests, 0 failures), plus spotless and detekt.This commit re-ran
OneSignalImpV2Tests(18) andOneSignalImpTests(27), plus spotless and detekt. The new test checks that a null context and a null externalId name the argument, and that the log says the SDK is not initialized only when it is not.Manual testing
Not needed. Behavior is covered by the unit tests above.
Affected code checklist
Checklist
Overview
Testing
Final pass