Skip to content

Avoid throwing in ForcedSender.sendBlocking #4293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

mrober
Copy link
Contributor

@mrober mrober commented Nov 8, 2022

Make ForcedSender.sendBlocking do nothing, instead of throw, when given a wrong Transport type.

This will make it easier to mock Transport in tests. This method is best-effort anyway, so doing nothing instead of throwing is fine.

…en a wrong Transport type.

This will make it easier to mock Transport in tests. This method is best-effort anyway, so doing nothing instead of throwing is fine.
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 8, 2022

Coverage Report 1

Affected Products

  • transport-runtime

    Overall coverage changed from ? (d719e10) to 0.00% (c462217) by ?.

    93 individual files with coverage change

    FilenameBase (d719e10)Merge (c462217)Diff
    AlarmManagerScheduler.java?0.00%?
    AlarmManagerSchedulerBroadcastReceiver.java?0.00%?
    AutoProtoEncoderDoNotUseEncoder.java?0.00%?
    AutoValue_BackendRequest.java?0.00%?
    AutoValue_BackendResponse.java?0.00%?
    AutoValue_CreationContext.java?0.00%?
    AutoValue_EventInternal.java?0.00%?
    AutoValue_EventStoreConfig.java?0.00%?
    AutoValue_PersistedEvent.java?0.00%?
    AutoValue_SchedulerConfig.java?0.00%?
    AutoValue_SchedulerConfig_ConfigValue.java?0.00%?
    AutoValue_SendRequest.java?0.00%?
    AutoValue_TransportContext.java?0.00%?
    BackendFactory.java?0.00%?
    BackendRegistry.java?0.00%?
    BackendRegistryModule.java?0.00%?
    BackendRequest.java?0.00%?
    BackendResponse.java?0.00%?
    ClientHealthMetricsStore.java?0.00%?
    ClientMetrics.java?0.00%?
    Clock.java?0.00%?
    CreationContext.java?0.00%?
    CreationContextFactory.java?0.00%?
    CreationContextFactory_Factory.java?0.00%?
    DaggerTransportRuntimeComponent.java?0.00%?
    DefaultScheduler.java?0.00%?
    DefaultScheduler_Factory.java?0.00%?
    Destination.java?0.00%?
    EncodedDestination.java?0.00%?
    EncodedPayload.java?0.00%?
    EventInternal.java?0.00%?
    EventStore.java?0.00%?
    EventStoreConfig.java?0.00%?
    EventStoreModule.java?0.00%?
    EventStoreModule_DbNameFactory.java?0.00%?
    EventStoreModule_PackageNameFactory.java?0.00%?
    EventStoreModule_SchemaVersionFactory.java?0.00%?
    EventStoreModule_StoreConfigFactory.java?0.00%?
    ExecutionModule.java?0.00%?
    ExecutionModule_ExecutorFactory.java?0.00%?
    ForcedSender.java?0.00%?
    Function.java?0.00%?
    GlobalMetrics.java?0.00%?
    JobInfoScheduler.java?0.00%?
    JobInfoSchedulerService.java?0.00%?
    LogEventDropped.java?0.00%?
    Logging.java?0.00%?
    LogSourceMetrics.java?0.00%?
    MetadataBackendRegistry.java?0.00%?
    MetadataBackendRegistry_Factory.java?0.00%?
    Monotonic.java?0.00%?
    PersistedEvent.java?0.00%?
    PriorityMapping.java?0.00%?
    ProtoEncoderDoNotUse.java?0.00%?
    Retries.java?0.00%?
    RetryStrategy.java?0.00%?
    SafeLoggingExecutor.java?0.00%?
    Scheduler.java?0.00%?
    SchedulerConfig.java?0.00%?
    SchedulingConfigModule.java?0.00%?
    SchedulingConfigModule_ConfigFactory.java?0.00%?
    SchedulingModule.java?0.00%?
    SchedulingModule_WorkSchedulerFactory.java?0.00%?
    SchemaManager.java?0.00%?
    SchemaManager_Factory.java?0.00%?
    SendRequest.java?0.00%?
    SQLiteEventStore.java?0.00%?
    SQLiteEventStore_Factory.java?0.00%?
    StorageMetrics.java?0.00%?
    SynchronizationException.java?0.00%?
    SynchronizationGuard.java?0.00%?
    TestClock.java?0.00%?
    TimeModule.java?0.00%?
    TimeModule_EventClockFactory.java?0.00%?
    TimeModule_UptimeClockFactory.java?0.00%?
    TimeWindow.java?0.00%?
    TransportBackend.java?0.00%?
    TransportBackendDiscovery.java?0.00%?
    TransportContext.java?0.00%?
    TransportFactoryImpl.java?0.00%?
    TransportImpl.java?0.00%?
    TransportInternal.java?0.00%?
    TransportRuntime.java?0.00%?
    TransportRuntimeComponent.java?0.00%?
    TransportRuntime_Factory.java?0.00%?
    Uploader.java?0.00%?
    Uploader_Factory.java?0.00%?
    UptimeClock.java?0.00%?
    WallTime.java?0.00%?
    WallTimeClock.java?0.00%?
    WorkInitializer.java?0.00%?
    WorkInitializer_Factory.java?0.00%?
    WorkScheduler.java?0.00%?

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/T95QKR2tyX.html

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Unit Test Results

  60 files   -    335    60 suites   - 335   4m 48s ⏱️ - 14m 16s
792 tests  - 3 938  792 ✔️  - 3 916  0 💤  - 22  0 ±0 
792 runs   - 3 954  792 ✔️  - 3 932  0 💤  - 22  0 ±0 

Results for commit f640d70. ± Comparison against base commit 31ab674.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 8, 2022

Size Report 1

Affected Products

  • transport-runtime

    TypeBase (d719e10)Merge (c462217)Diff
    aar180 kB180 kB+47 B (+0.0%)
    apk (release)83.5 kB83.5 kB-24 B (-0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/rUgwa43YK9.html

}
throw new IllegalArgumentException("Expected instance of TransportImpl.");
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a warning log statement here, there's logging util available in this sdk under
com.google.android.datatransport.runtime.logging.Logging

@mrober mrober merged commit 42e5b8a into master Nov 15, 2022
@mrober mrober deleted the datatransport-forcedsender-noop branch November 15, 2022 21:00
davidmotson pushed a commit that referenced this pull request Nov 28, 2022
* Make ForcedSender.sendBlocking do nothing, instead of throw, when given a wrong Transport type.

This will make it easier to mock Transport in tests. This method is best-effort anyway, so doing nothing instead of throwing is fine.

* Log warning when given wrong type.
@firebase firebase locked and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants