commit 3636c2273492455c8167fc9fc00ba82b7de9569d parent d323952646cd954d96b87e701faa69fd5ff545ba Author: Ted Campbell <tcampbell@mozilla.com> Date: Thu, 8 Jan 2026 15:38:44 +0000 Bug 2008762 - Avoid mocks for data classes in android-components. r=mcarare,android-reviewers,nalexander Remove some unnecessary spies. Cleanup how TranslationsBrowserState is mocked to avoid monkeying with immutable BrowserState. While we are at it, convert SearchConfigIconsParserTest from JVM to Robolectric test so it can use JSONObject directly. Differential Revision: https://phabricator.services.mozilla.com/D278075 Diffstat:
5 files changed, 86 insertions(+), 74 deletions(-)
diff --git a/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt b/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddlewareTest.kt @@ -42,7 +42,6 @@ import org.junit.Before import org.junit.Test import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.Mockito.atLeastOnce -import org.mockito.Mockito.doReturn import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.verify @@ -76,16 +75,14 @@ class TranslationsMiddlewareTest { fun setup() { engine = mock() engineSession = mock() - tab = spy( - createTab( - url = "https://www.firefox.com", - title = "Firefox", - id = "1", - engineSession = engineSession, - ), - ) - tabs = spy(listOf(tab)) - state = spy(BrowserState(tabs = tabs, selectedTabId = tab.id)) + tab = createTab( + url = "https://www.firefox.com", + title = "Firefox", + id = "1", + engineSession = engineSession, + ) + tabs = listOf(tab) + state = BrowserState(tabs = tabs, selectedTabId = tab.id) translationsMiddleware = TranslationsMiddleware(engine = engine, scope = scope) store = spy(BrowserStore(middleware = listOf(translationsMiddleware), initialState = state)) @@ -103,14 +100,23 @@ class TranslationsMiddlewareTest { supportedDocumentLang = true, userPreferredLangTag = mockTo.code, ) - val mockSessionState = TranslationsState( + val mockTranslationsState = TranslationsState( translationEngineState = TranslationEngineState(mockDetectedLanguages), ) + val mockTranslationEngine = TranslationsBrowserState( + isEngineSupported = true, + supportedLanguages = mockSupportedLanguages, + languageModels = mockLanguageModels, + ) - val mockBrowserState = TranslationsBrowserState(isEngineSupported = true, supportedLanguages = mockSupportedLanguages, languageModels = mockLanguageModels) + // Replace the TabSessionState/BrowserState with mocked translation engines + tab = tab.copy(translationsState = mockTranslationsState) + tabs = listOf(tab) + state = state.copy( + tabs = tabs, + translationEngine = mockTranslationEngine, + ) - doReturn(mockSessionState).`when`(tab).translationsState - doReturn(mockBrowserState).`when`(state).translationEngine whenever(store.state).thenReturn(state) } diff --git a/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt b/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt @@ -1276,7 +1276,7 @@ class AbstractFetchDownloadServiceTest { @Test fun `performDownload - use the client response when the download response NOT available`() { val responseFromClient = mock<Response>() - val download = spy(DownloadState("https://example.com/file.txt", "file.txt", response = null, contentLength = 1000)) + val download = DownloadState("https://example.com/file.txt", "file.txt", response = null, contentLength = 1000) val downloadJob = DownloadJobState(state = download, status = DOWNLOADING) doReturn(404).`when`(responseFromClient).status @@ -1291,7 +1291,7 @@ class AbstractFetchDownloadServiceTest { fun `performDownload - use the client response when resuming a download`() { val responseFromDownloadState = mock<Response>() val responseFromClient = mock<Response>() - val download = spy(DownloadState("https://example.com/file.txt", "file.txt", response = responseFromDownloadState, contentLength = 1000)) + val download = DownloadState("https://example.com/file.txt", "file.txt", response = responseFromDownloadState, contentLength = 1000) val downloadJob = DownloadJobState(currentBytesCopied = 100, state = download, status = DOWNLOADING) doReturn(404).`when`(responseFromClient).status @@ -1306,7 +1306,7 @@ class AbstractFetchDownloadServiceTest { @Test fun `performDownload - don't make a client request when download is completed`() { val responseFromDownloadState = mock<Response>() - val download = spy(DownloadState("https://example.com/file.txt", "file.txt", response = responseFromDownloadState, contentLength = 1000)) + val download = DownloadState("https://example.com/file.txt", "file.txt", response = responseFromDownloadState, contentLength = 1000) val downloadJob = DownloadJobState(currentBytesCopied = 1000, state = download, status = DOWNLOADING) service.performDownload(downloadJob) diff --git a/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt b/mobile/android/android-components/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt @@ -1449,7 +1449,6 @@ class DownloadsFeatureTest { ) doReturn(arrayOf(INTERNET, WRITE_EXTERNAL_STORAGE)).`when`(downloadManager).permissions - doReturn(testContext.packageName).`when`(spy(ourApp)).packageName doReturn(dialog).`when`(fragmentManager).findFragmentByTag(DownloadAppChooserDialog.FRAGMENT_TAG) doReturn(consumeDownloadUseCase).`when`(downloadsUseCases).consumeDownload diff --git a/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/icons/SearchConfigIconsParserTest.kt b/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/icons/SearchConfigIconsParserTest.kt @@ -10,48 +10,44 @@ import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Before import org.junit.Test -import org.mockito.Mockito.mock -import org.mockito.Mockito.`when` +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +@RunWith(RobolectricTestRunner::class) class SearchConfigIconsParserTest { private lateinit var parser: SearchConfigIconsParser - private lateinit var mockRecord: RemoteSettingsRecord - private lateinit var mockFields: JSONObject - private lateinit var mockAttachment: Attachment @Before fun setUp() { parser = SearchConfigIconsParser() - mockRecord = mock(RemoteSettingsRecord::class.java) - mockFields = mock(JSONObject::class.java) - mockAttachment = mock(Attachment::class.java) - - `when`(mockRecord.fields).thenReturn(mockFields) - `when`(mockRecord.attachment).thenReturn(null) } @Test fun `Given record with all fields and attachment When parseRecord is called Then valid model is returned`() { - val mockJsonArray = mock(JSONArray::class.java) - `when`(mockJsonArray.length()).thenReturn(2) - `when`(mockJsonArray.get(0)).thenReturn("google") - `when`(mockJsonArray.get(1)).thenReturn("bing") - - `when`(mockFields.getLong("schema")).thenReturn(1L) - `when`(mockFields.getInt("imageSize")).thenReturn(64) - `when`(mockFields.getJSONArray("engineIdentifiers")).thenReturn(mockJsonArray) - `when`(mockFields.optString("filter_expression")).thenReturn("test-filter") - - `when`(mockAttachment.filename).thenReturn("icon.png") - `when`(mockAttachment.mimetype).thenReturn("image/png") - `when`(mockAttachment.location).thenReturn("location/path") - `when`(mockAttachment.hash).thenReturn("abc123hash") - `when`(mockAttachment.size).thenReturn(1024u) - - `when`(mockRecord.attachment).thenReturn(mockAttachment) - - val result = parser.parseRecord(mockRecord) + val fields = JSONObject() + .put("schema", 1L) + .put("imageSize", 64) + .put("engineIdentifiers", JSONArray().put("google").put("bing")) + .put("filter_expression", "test-filter") + + val attachment = Attachment( + filename = "icon.png", + mimetype = "image/png", + location = "location/path", + hash = "abc123hash", + size = 1024u, + ) + + val record = RemoteSettingsRecord( + id = "test-id", + lastModified = 123u, + deleted = false, + attachment = attachment, + fields = fields, + ) + + val result = parser.parseRecord(record) assertNotNull(result) assertEquals(1L, result!!.schema) @@ -69,16 +65,21 @@ class SearchConfigIconsParserTest { @Test fun `Given record with missing optional fields When parseRecord is called Then valid model with null attachment is returned`() { - val mockJsonArray = mock(JSONArray::class.java) - `when`(mockJsonArray.length()).thenReturn(1) - `when`(mockJsonArray.get(0)).thenReturn("duckduckgo") - - `when`(mockFields.getLong("schema")).thenReturn(2L) - `when`(mockFields.getInt("imageSize")).thenReturn(32) - `when`(mockFields.getJSONArray("engineIdentifiers")).thenReturn(mockJsonArray) - `when`(mockFields.optString("filter_expression")).thenReturn("") - - val result = parser.parseRecord(mockRecord) + val fields = JSONObject() + .put("schema", 2L) + .put("imageSize", 32) + .put("engineIdentifiers", JSONArray().put("duckduckgo")) + .put("filter_expression", "") + + val record = RemoteSettingsRecord( + id = "test-id", + lastModified = 123u, + deleted = false, + attachment = null, + fields = fields, + ) + + val result = parser.parseRecord(record) assertNotNull(result) assertEquals(2L, result!!.schema) @@ -90,9 +91,19 @@ class SearchConfigIconsParserTest { @Test fun `Given record that causes JSONException during field parsing When parseRecord is called Then null is returned`() { - `when`(mockFields.getLong("schema")).thenThrow(JSONException("Test exception on schema")) - - val result = parser.parseRecord(mockRecord) + val fields = JSONObject() + .put("schema", "NOT_AN_INTEGER") + .put("imageSize", "NOT_AN_INTEGER") + + val record = RemoteSettingsRecord( + id = "test-id", + lastModified = 123u, + deleted = false, + attachment = null, + fields = fields, + ) + + val result = parser.parseRecord(record) assertNull(result) } diff --git a/mobile/android/android-components/components/feature/toolbar/src/test/java/mozilla/components/feature/toolbar/ToolbarBehaviorControllerTest.kt b/mobile/android/android-components/components/feature/toolbar/src/test/java/mozilla/components/feature/toolbar/ToolbarBehaviorControllerTest.kt @@ -34,12 +34,10 @@ class ToolbarBehaviorControllerTest { fun `Controller should check the status of the provided custom tab id`() { val customTabContent: ContentState = mock() val normalTabContent: ContentState = mock() - val state = spy( - BrowserState( - tabs = listOf(TabSessionState("123", normalTabContent)), - customTabs = listOf(CustomTabSessionState("ct", customTabContent, config = mock())), - selectedTabId = "123", - ), + val state = BrowserState( + tabs = listOf(TabSessionState("123", normalTabContent)), + customTabs = listOf(CustomTabSessionState("ct", customTabContent, config = mock())), + selectedTabId = "123", ) val store = BrowserStore(state) val controller = ToolbarBehaviorController(mock(), store, "ct") @@ -58,12 +56,10 @@ class ToolbarBehaviorControllerTest { fun `Controller should check the status of the currently selected tab if not initialized with a custom tab id`() { val customTabContent: ContentState = mock() val normalTabContent: ContentState = mock() - val state = spy( - BrowserState( - tabs = listOf(TabSessionState("123", normalTabContent)), - customTabs = listOf(CustomTabSessionState("ct", customTabContent, config = mock())), - selectedTabId = "123", - ), + val state = BrowserState( + tabs = listOf(TabSessionState("123", normalTabContent)), + customTabs = listOf(CustomTabSessionState("ct", customTabContent, config = mock())), + selectedTabId = "123", ) val store = BrowserStore(state) val controller = ToolbarBehaviorController(mock(), store)