From 122e80fae9afc2e5bc75aa3cd73af027391358d0 Mon Sep 17 00:00:00 2001 From: XiangRongLin <41164160+XiangRongLin@users.noreply.github.com> Date: Sat, 19 Dec 2020 16:28:10 +0100 Subject: [PATCH] Remove subclasses from ContentSettingsManagerTest ExportTest provides no value. ImportTest creates temporary files even if not needed. --- .../settings/ContentSettingsFragment.java | 7 +- .../settings/ContentSettingsManager.kt | 2 +- .../settings/ContentSettingsManagerTest.kt | 251 ++++++++---------- .../test/resources/settings/newpipe.settings | Bin 0 -> 2445 bytes 4 files changed, 106 insertions(+), 154 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java index 2339de53a..18d49b507 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java @@ -32,14 +32,9 @@ import org.schabi.newpipe.util.FilePickerActivityHelper; import org.schabi.newpipe.util.ZipHelper; import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; -import java.io.ObjectInputStream; import java.text.SimpleDateFormat; import java.util.Date; import java.util.Locale; -import java.util.Map; -import java.util.zip.ZipFile; import static org.schabi.newpipe.util.Localization.assureCorrectAppLanguage; @@ -241,7 +236,7 @@ public class ContentSettingsFragment extends BasePreferenceFragment { } //If settings file exist, ask if it should be imported. - if (manager.containSettings(filePath)) { + if (manager.extractSettings(filePath)) { final AlertDialog.Builder alert = new AlertDialog.Builder(getContext()); alert.setTitle(R.string.import_settings); diff --git a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsManager.kt b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsManager.kt index 0e779edbf..bab29a30a 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsManager.kt +++ b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsManager.kt @@ -57,7 +57,7 @@ class ContentSettingsManager(private val fileLocator: NewPipeFileLocator) { return success } - fun containSettings(filePath: String): Boolean { + fun extractSettings(filePath: String): Boolean { return ZipHelper .extractFileFromZip(filePath, fileLocator.settings.path, "newpipe.settings") } diff --git a/app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt b/app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt index e139dad7b..875ef758a 100644 --- a/app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt +++ b/app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt @@ -1,187 +1,144 @@ package org.schabi.newpipe.settings import android.content.SharedPreferences -import org.junit.Assert import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue -import org.junit.Assume import org.junit.Before -import org.junit.BeforeClass import org.junit.Test import org.junit.runner.RunWith -import org.junit.runners.Suite import org.mockito.Mockito import org.mockito.Mockito.`when` import org.mockito.Mockito.anyString import org.mockito.Mockito.anyBoolean +import org.mockito.Mockito.anyInt import org.mockito.Mockito.atLeastOnce import org.mockito.Mockito.verify import org.mockito.Mockito.withSettings import org.mockito.junit.MockitoJUnitRunner -import org.schabi.newpipe.settings.ContentSettingsManagerTest.* import java.io.File import java.io.ObjectInputStream import java.nio.file.Files import java.util.zip.ZipFile -@RunWith(Suite::class) -@Suite.SuiteClasses(ExportTest::class, ImportTest::class) +@RunWith(MockitoJUnitRunner::class) class ContentSettingsManagerTest { - @RunWith(MockitoJUnitRunner::class) - class ExportTest { + companion object { + private val classloader = ContentSettingsManager::class.java.classLoader!! + } - companion object { - private lateinit var fileLocator: NewPipeFileLocator - private lateinit var newpipeDb: File - private lateinit var newpipeSettings: File + private lateinit var fileLocator: NewPipeFileLocator - @JvmStatic - @BeforeClass - fun setupFiles() { - val dbPath = ExportTest::class.java.classLoader?.getResource("settings/newpipe.db")?.file - val settingsPath = ExportTest::class.java.classLoader?.getResource("settings/newpipe.settings")?.path - Assume.assumeNotNull(dbPath) - Assume.assumeNotNull(settingsPath) + @Before + fun setupFileLocator() { + fileLocator = Mockito.mock(NewPipeFileLocator::class.java, withSettings().stubOnly()) + } - newpipeDb = File(dbPath!!) - newpipeSettings = File(settingsPath!!) + @Test + fun `The settings must be exported successfully in the correct format`() { + val db = File(classloader.getResource("settings/newpipe.db")!!.file) + val newpipeSettings = File.createTempFile("newpipe_", "") + `when`(fileLocator.db).thenReturn(db) + `when`(fileLocator.settings).thenReturn(newpipeSettings) - fileLocator = Mockito.mock(NewPipeFileLocator::class.java, withSettings().stubOnly()) - `when`(fileLocator.db).thenReturn(newpipeDb) - `when`(fileLocator.settings).thenReturn(newpipeSettings) + val expectedPreferences = mapOf("such pref" to "much wow") + val sharedPreferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly()) + `when`(sharedPreferences.all).thenReturn(expectedPreferences) + + val output = File.createTempFile("newpipe_", "") + ContentSettingsManager(fileLocator).exportDatabase(sharedPreferences, output.absolutePath) + + val zipFile = ZipFile(output) + val entries = zipFile.entries().toList() + assertEquals(2, entries.size) + + zipFile.getInputStream(entries.first { it.name == "newpipe.db" }).use { actual -> + db.inputStream().use { expected -> + assertEquals(expected.reader().readText(), actual.reader().readText()) } } - private lateinit var preferences: SharedPreferences - - @Before - fun setupMocks() { - preferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly()) - } - - @Test - fun `The settings must be exported successfully in the correct format`() { - val expectedPreferences = mapOf("such pref" to "much wow") - `when`(preferences.all).thenReturn(expectedPreferences) - - val manager = ContentSettingsManager(fileLocator) - - val output = File.createTempFile("newpipe_", "") - manager.exportDatabase(preferences, output.absolutePath) - - val zipFile = ZipFile(output.absoluteFile) - val entries = zipFile.entries().toList() - assertEquals(2, entries.size) - - zipFile.getInputStream(entries.first { it.name == "newpipe.db" }).use { actual -> - newpipeDb.inputStream().use { expected -> - assertEquals(expected.reader().readText(), actual.reader().readText()) - } - } - - zipFile.getInputStream(entries.first { it.name == "newpipe.settings" }).use { actual -> - val actualPreferences = ObjectInputStream(actual).readObject() - assertEquals(expectedPreferences, actualPreferences) - } + zipFile.getInputStream(entries.first { it.name == "newpipe.settings" }).use { actual -> + val actualPreferences = ObjectInputStream(actual).readObject() + assertEquals(expectedPreferences, actualPreferences) } } - @RunWith(MockitoJUnitRunner::class) - class ImportTest { - - companion object { - private lateinit var fileLocator: NewPipeFileLocator - private lateinit var zip: File - private lateinit var emptyZip: File - private lateinit var db: File - private lateinit var dbJournal: File - private lateinit var dbWal: File - private lateinit var dbShm: File - private lateinit var settings: File - - @JvmStatic - @BeforeClass - fun setupReadOnlyFiles() { - val zipPath = ImportTest::class.java.classLoader?.getResource("settings/newpipe.zip")?.file - val emptyZipPath = ImportTest::class.java.classLoader?.getResource("settings/empty.zip")?.file - Assume.assumeNotNull(zipPath) - Assume.assumeNotNull(emptyZipPath) - - zip = File(zipPath!!) - emptyZip = File(emptyZipPath!!) - } - } - - @Before - fun setupWriteFiles() { - db = File.createTempFile("newpipe_", "") - dbJournal = File.createTempFile("newpipe_", "") - dbWal = File.createTempFile("newpipe_", "") - dbShm = File.createTempFile("newpipe_", "") - settings = File.createTempFile("newpipe_", "") - - fileLocator = Mockito.mock(NewPipeFileLocator::class.java, withSettings().stubOnly()) - `when`(fileLocator.db).thenReturn(db) - `when`(fileLocator.dbJournal).thenReturn(dbJournal) - `when`(fileLocator.dbShm).thenReturn(dbShm) - `when`(fileLocator.dbWal).thenReturn(dbWal) - `when`(fileLocator.settings).thenReturn(settings) - } - - @Test - fun `The database must be extracted from the zip file`() { - val success = ContentSettingsManager(fileLocator).extractDb(zip.path) - - assertTrue(success) - assertFalse(dbJournal.exists()) - assertFalse(dbWal.exists()) - assertFalse(dbShm.exists()) - assertTrue("database file size is zero", Files.size(db.toPath()) > 0) - } - - @Test - fun `Extracting the database from an empty zip must not work`() { - val success = ContentSettingsManager(fileLocator).extractDb(emptyZip.path) - - assertFalse(success) - assertTrue(dbJournal.exists()) - assertTrue(dbWal.exists()) - assertTrue(dbShm.exists()) - assertEquals(0, Files.size(db.toPath())) - } - - @Test - fun `Contain setting must return true, if a settings file exists in the zip`() { - val contains = ContentSettingsManager(fileLocator).containSettings(zip.path) - - assertTrue(contains) - } - - @Test - fun `Contain setting must return false, if a no settings file exists in the zip`() { - val contains = ContentSettingsManager(fileLocator).containSettings(emptyZip.path) - - assertFalse(contains) - } - - @Test - fun `Preferences must be set from the settings file`() { - val preferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly()) - val editor = Mockito.mock(SharedPreferences.Editor::class.java) - `when`(preferences.edit()).thenReturn(editor) - - - val manager = ContentSettingsManager(fileLocator) - manager.containSettings(zip.path) - manager.loadSharedPreferences(preferences) - - verify(editor, atLeastOnce()).putBoolean(anyString(), anyBoolean()) - } + @Test + fun `The database must be extracted from the zip file`() { + val db = File.createTempFile("newpipe_", "") + val dbJournal = File.createTempFile("newpipe_", "") + val dbWal = File.createTempFile("newpipe_", "") + val dbShm = File.createTempFile("newpipe_", "") + `when`(fileLocator.db).thenReturn(db) + `when`(fileLocator.dbJournal).thenReturn(dbJournal) + `when`(fileLocator.dbShm).thenReturn(dbShm) + `when`(fileLocator.dbWal).thenReturn(dbWal) + val zip = File(classloader.getResource("settings/newpipe.zip")?.file!!) + val success = ContentSettingsManager(fileLocator).extractDb(zip.path) + assertTrue(success) + assertFalse(dbJournal.exists()) + assertFalse(dbWal.exists()) + assertFalse(dbShm.exists()) + assertTrue("database file size is zero", Files.size(db.toPath()) > 0) } + @Test + fun `Extracting the database from an empty zip must not work`() { + val db = File.createTempFile("newpipe_", "") + val dbJournal = File.createTempFile("newpipe_", "") + val dbWal = File.createTempFile("newpipe_", "") + val dbShm = File.createTempFile("newpipe_", "") + `when`(fileLocator.db).thenReturn(db) + val emptyZip = File(classloader.getResource("settings/empty.zip")?.file!!) + val success = ContentSettingsManager(fileLocator).extractDb(emptyZip.path) + + assertFalse(success) + assertTrue(dbJournal.exists()) + assertTrue(dbWal.exists()) + assertTrue(dbShm.exists()) + assertEquals(0, Files.size(db.toPath())) + } + + @Test + fun `Contains setting must return true if a settings file exists in the zip`() { + val settings = File.createTempFile("newpipe_", "") + `when`(fileLocator.settings).thenReturn(settings) + + val zip = File(classloader.getResource("settings/newpipe.zip")?.file!!) + val contains = ContentSettingsManager(fileLocator).extractSettings(zip.path) + + assertTrue(contains) + } + + @Test + fun `Contains setting must return false if a no settings file exists in the zip`() { + val settings = File.createTempFile("newpipe_", "") + `when`(fileLocator.settings).thenReturn(settings) + + val emptyZip = File(classloader.getResource("settings/empty.zip")?.file!!) + val contains = ContentSettingsManager(fileLocator).extractSettings(emptyZip.path) + + assertFalse(contains) + } + + @Test + fun `Preferences must be set from the settings file`() { + val settings = File(classloader.getResource("settings/newpipe.settings")!!.path) + `when`(fileLocator.settings).thenReturn(settings) + + val preferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly()) + val editor = Mockito.mock(SharedPreferences.Editor::class.java) + `when`(preferences.edit()).thenReturn(editor) + + ContentSettingsManager(fileLocator).loadSharedPreferences(preferences) + + verify(editor, atLeastOnce()).putBoolean(anyString(), anyBoolean()) + verify(editor, atLeastOnce()).putString(anyString(), anyString()) + verify(editor, atLeastOnce()).putInt(anyString(), anyInt()) + } } diff --git a/app/src/test/resources/settings/newpipe.settings b/app/src/test/resources/settings/newpipe.settings index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..56e6c5d5dd53ba66b0144215b928f6137f67fc30 100644 GIT binary patch literal 2445 zcmaJ@zi%8x7#-VjLL8FV`Qea26rcba>tWC@AwsfT$@bXs8f9v`AFEZ+6$YOH59>lXiFZn>X*h?|t{^FSQuFTKUxM znYEg2zINM$?2f4xkNp1A&#Mo9T~PD4)KczDddnp2{FXY(nWvDsJRMi>zNh5#VDub* zAL4tR)oUR!Icat}?AFGUwuKqux74L$JFtmi>&mddYi`Qn+I8o0GUYEX`-i`M_wV2H zYVH%YxM%X3#?>6F3)rzPNV`&-I<-y@Tv7);G`0-8>T|WASe>bZ>_TJU)PZP6@6bdZ z9%l6-@Q$>=Kq2-vVM=x4N~V(AB`+la;Pq}zHOZra^+iNFU8ZtXU7UAfw;Se;JYs1z#%U6?Ueu%DpT+pJEuT!Of|jCT)*Nnk2iU7A*1hui^( zD=HDGC;QI&U<;BA$)Ec3`yc-K;;T*gV5{?iSIg0*(Jyyv5#Y;5-(EcS%^zRSD>bg* zKDgjyR2nW|y=PN$t-8x{N_n#u6JT3b`v}iIK30IkWq}DhEse*!hq^MHv3f&jeP_L0 zxK2f#gT!>bw|>X%SqkfSX>`w4w9e{^t8Q-p483ZdsB*vt+ynCUU$oh43)KX42h z!4@eLKP2YDYl(LuXn0`c{EqGrcCQ_eRY9_--rYPe-2qm}mnLthiEbp{GPG*)96gee zsdQQwl`V%;ws?k$m!pbO;ErW9c(hCS))im5}2_^Pw<68fK2z8dISMv0Drk&AjoMs#>b9it&?BwVX2r$E{;3Vfb9jr!2~|;gdEOfJ zRg-wLmebHAlWVyrlTL29au110>3z4cwz<}8C4S|ZE8h>UY!@zahID4Nlp3^hdvU(< zG$8MR!&QWtmk5#u=!(flW)dW#+6HDd+uY5B6oU7g`vA$VNV({1Y1^Z4NS7l(@kHy2 zpk0{7RK@(2>B#D6b6HdytS;sTafKF5b%USEgn|Y=0qEttZ%)w|ni~KpkK%laVjRSU^ zjkP0)wMJ5$@#E%1cYR0S!jG8J9etsl{&8xzDqo6d(dI)d@yix%W`<6)%&E>_