Release/iron source mao rtb #527

jbsrc · 2024-08-25T11:24:52Z

No description provided.

init all ad format

LTPhantom · 2024-09-04T17:11:10Z

ThirdPartyAdapters/adaptertestkit/build.gradle

+  testImplementation "org.powermock:powermock-module-junit4:2.0.9"
+  testImplementation "org.powermock:powermock-api-mockito2:2.0.9"


Are these dependencies used somewhere?

LTPhantom · 2024-09-04T17:11:51Z

...rce/ironsource/src/main/java/com/google/ads/mediation/ironsource/IronSourceAdapterUtils.java

@@ -21,13 +21,18 @@
 import android.app.Activity;
 import android.content.Context;
 import android.text.TextUtils;
+


nit: delete empty lines between imports for this and other files.

LTPhantom · 2024-09-04T17:24:06Z

...ironsource/src/main/java/com/google/ads/mediation/ironsource/IronSourceMediationAdapter.java

@@ -243,8 +256,7 @@ public void loadRewardedAd(
      AdError adError =
          new AdError(
              ERROR_SDK_NOT_INITIALIZED,
-              "Failed to load IronSource rewarded ad since IronSource SDK is not "
-                  + "initialized.",
+              "Failed to load IronSource rewarded ad since IronSource SDK is not " + "initialized.",


If this is becoming one line, would it be better to remove the string concatenation?

LTPhantom · 2024-09-04T17:24:19Z

...ironsource/src/main/java/com/google/ads/mediation/ironsource/IronSourceMediationAdapter.java

@@ -349,8 +363,7 @@ public void loadBannerAd(
      AdError loadError =
          new AdError(
              ERROR_SDK_NOT_INITIALIZED,
-              "Failed to load IronSource banner ad since IronSource SDK is not "
-                  + "initialized.",
+              "Failed to load IronSource banner ad since IronSource SDK is not " + "initialized.",


Same as above.

LTPhantom · 2024-09-04T17:25:34Z

...ronsource/src/main/java/com/google/ads/mediation/ironsource/IronSourceRtbInterstitialAd.java

@@ -0,0 +1,184 @@
+// Copyright 2023 Google LLC


Change to 2024

LTPhantom · 2024-09-04T17:52:21Z

...ce/ironsource/src/main/java/com/google/ads/mediation/ironsource/IronSourceRtbRewardedAd.java

@@ -0,0 +1,186 @@
+// Copyright 2023 Google LLC


I think most of the comments on the IronSourceRtbInterstitialAd apply here as well.

LTPhantom · 2024-09-04T17:55:16Z

...source/src/test/kotlin/com/google/ads/mediation/ironsource/IronSourceMediationAdapterTest.kt

+    @Mock
+    private lateinit var initializationCompleteCallback: InitializationCompleteCallback


Unused variable. Also remove @mock annotation from imports

LTPhantom · 2024-09-04T17:59:41Z

...source/src/test/kotlin/com/google/ads/mediation/ironsource/IronSourceMediationAdapterTest.kt

-  fun getSDKVersionInfo_validSDKVersionFor3Digits_returnsTheSameVersion() {
-    mockStatic(IronSourceUtils::class.java).use {
-      whenever(getSDKVersion()) doReturn "7.3.2"
+    @Before


I think most of these tests are not new, it is kind of hard to review given how GitHub shows the changes. My guess it is related to the extra indentation, do you think you can update to make GitHub correctly identify what is actually new code?

From what I understand, these settings are local. I changed the settings, but they are only applied for me. You can try changing them as well in the file changes tab --> settings

LTPhantom · 2024-09-04T18:00:45Z

...ource/src/test/kotlin/com/google/ads/mediation/ironsource/IronSourceRtbInterstitialAdTest.kt

+@Config(sdk = [28])
+class IronSourceRtbInterstitialAdTest {
+
+    @Mock


Please avoid @Mock annotation and prefer kotlin mock method as the other test files. That way boilerplate code is avoided below.

LTPhantom · 2024-09-04T18:05:13Z

...ource/src/test/kotlin/com/google/ads/mediation/ironsource/IronSourceRtbInterstitialAdTest.kt

+        val captor = argumentCaptor<AdError>()
+        verify(mediationAdLoadCallback).onFailure(captor.capture())
+        val capturedError = captor.firstValue
+        assertEquals(123, capturedError.code)
+        assertEquals("An error occurred", capturedError.message)
+        assertEquals("com.google.ads.mediation.ironsource", capturedError.domain)


You can use the AdErrorMatcher to avoid all these boilerplate code. You can check an example of how to use it here. Same comment for other uses below.

unit tests: delete java annotations. style.

jbsrc added 15 commits August 22, 2024 14:36

support new init api and ad request

ffacae3

fix unit tests

4b4f0ca

return adapter version as main

60c350f

delete old loadRtb

1cc2cb4

delete helper

c4ba743

delete unnecessary log and import

4c68e19

delete internal init

a5a0329

fix unit test

8a12dcc

reforamt

f032a4f

default instance id = ""

3dae562

delete unused code

bdfb025

update unit tests

836dfe7

code format

7ada09a

init all ad format

ADAPTER_VERSION

c73e9ed

delete convert ad foramt

b85acc1

jbsrc marked this pull request as ready for review August 29, 2024 15:18

LTPhantom reviewed Sep 4, 2024

View reviewed changes

jbsrc added 3 commits September 8, 2024 16:12

update header comments.

51cd548

unit tests: delete java annotations. style.

format & renaming

09774af

Merge branch 'main' into release/IronSource-mao-rtb

e9ec824

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release/iron source mao rtb #527

Release/iron source mao rtb #527

jbsrc commented Aug 25, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

LTPhantom Sep 4, 2024

jbsrc Sep 9, 2024

		testImplementation "org.powermock:powermock-module-junit4:2.0.9"
		testImplementation "org.powermock:powermock-api-mockito2:2.0.9"

		@Mock
		private lateinit var initializationCompleteCallback: InitializationCompleteCallback

Release/iron source mao rtb #527

Are you sure you want to change the base?

Release/iron source mao rtb #527

Conversation

jbsrc commented Aug 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment