Provided below are examples of test smells that were detected in open source Android projects.
-
Conditional Test Logic TODO: update to com.dalthed.tucan / EventScraperTest.java
-
Unknown Test TODO: repleace with: net.cyclestreets / libraries/cyclestreets-core/src/test/java/net/cyclestreets/api/client/RetrofitApiClientIntegrationTest.java
-
TODO: Default Test (com.app.missednotificationsreminder / app/src/test/java/com/app/missednotificationsreminder/ExampleUnitTest.java)
-
TODO: Constructor Initialization (org.briarproject.briar.beta / bramble-core/src/test/java/org/briarproject/bramble/crypto/TagEncodingTest.java)
-
TODO: Ignored Test
-
TODO: Resource Optimism
-
TODO: Magic Number Test
App: com.madgag.agit
Test File: GitAsyncTaskTest.java
Production File: GitAsyncTask.java
@MediumTest
public void testCloneNonBareRepoFromLocalTestServer() throws Exception {
Clone cloneOp = new Clone(false, integrationGitServerURIFor("small-repo.early.git"), helper().newFolder());
Repository repo = executeAndWaitFor(cloneOp);
assertThat(repo, hasGitObject("ba1f63e4430bff267d112b1e8afc1d6294db0ccc"));
File readmeFile = new File(repo.getWorkTree(), "README");
assertThat(readmeFile, exists());
assertThat(readmeFile, ofLength(12));
}
The assertThat()
method is called 3 times within the test method. Each assert statement checks for a different condition, but the developer does not provide a explanation message for each assert statement. Hence, if one of the assert statements were to fail, identifying the cause of the failure is not straightforward.
[↑]
App: btools.routingapp
Test File: LinkedListContainerTest.java
Production File: LinkedListContainer.java
@Test
public void linkedListTest1()
{
int nlists = 553;
LinkedListContainer llc = new LinkedListContainer( nlists, null );
for ( int ln = 0; ln < nlists; ln++ )
{
for ( int i = 0; i < 10; i++ )
{
llc.addDataElement( ln, ln * i );
}
}
for ( int i = 0; i < 10; i++ )
{
for ( int ln = 0; ln < nlists; ln++ )
{
llc.addDataElement( ln, ln * i );
}
}
for ( int ln = 0; ln < nlists; ln++ )
{
int cnt = llc.initList( ln );
Assert.assertTrue( "list size test", cnt == 20 );
for ( int i = 19; i >= 0; i-- )
{
int data = llc.getDataElement();
Assert.assertTrue( "data value test", data == ln * ( i % 10 ) );
}
}
try
{
llc.getDataElement();
Assert.fail( "no more elements expected" );
}
catch (IllegalArgumentException e)
{
}
}
The test method, linkedListTest1()
, contains multiple for
loop statements (i.e. control flow statements). This increases the complexity of the test method and hence has a negative impact on maintenance of the test.
[↑]
Test File: LastPassParserTest.java
Production File: LastPassParser.java
public void testCredGetFullSampleV1() throws Throwable{
// ScrapedCredentials credentials = innerCredTest(FULL_SAMPLE_v1);
// assertEquals("p4ssw0rd", credentials.pass);
// assertEquals("[email protected]",credentials.user);
}
The test method, testCredGetFullSampleV1()
, contains only comments (i.e. no executable statements). A test method without executable statements will be marked as passing when executed.
[↑]
App: ch.hgdev.toposuite
Test File: AbrissTest.java
Production File: Abriss.java
@Test
public void realCase() {
Point p34 = new Point("34", 556506.667, 172513.91, 620.34, true);
Point p45 = new Point("45", 556495.16, 172493.912, 623.37, true);
Point p47 = new Point("47", 556612.21, 172489.274, 0.0, true);
Abriss a = new Abriss(p34, false);
a.removeDAO(CalculationsDataSource.getInstance());
a.getMeasures().add(new Measure(p45, 0.0, 91.6892, 23.277, 1.63));
a.getMeasures().add(new Measure(p47, 281.3521, 100.0471, 108.384, 1.63));
try {
a.compute();
} catch (CalculationException e) {
Assert.fail(e.getMessage());
}
// test intermediate values with point 45
Assert.assertEquals("233.2405",
this.df4.format(a.getResults().get(0).getUnknownOrientation()));
Assert.assertEquals("233.2435",
this.df4.format(a.getResults().get(0).getOrientedDirection()));
Assert.assertEquals("-0.1", this.df1.format(
a.getResults().get(0).getErrTrans()));
// test intermediate values with point 47
Assert.assertEquals("233.2466",
this.df4.format(a.getResults().get(1).getUnknownOrientation()));
Assert.assertEquals("114.5956",
this.df4.format(a.getResults().get(1).getOrientedDirection()));
Assert.assertEquals("0.5", this.df1.format(
a.getResults().get(1).getErrTrans()));
// test final results
Assert.assertEquals("233.2435", this.df4.format(a.getMean()));
Assert.assertEquals("43", this.df0.format(a.getMSE()));
Assert.assertEquals("30", this.df0.format(a.getMeanErrComp()));
}
In this example, the developer fails the test when a specific exception occurs. Ideally, the developer should split this test method into multiple test methods that (1) knowingly generate the exception and (2) do not generate the exception. The developer should utilize the @Test expected
attribute in JUnit 4 to fail the test when the exception occurs instead of explicitly catching or throwing the exception.
[↑]
App: at.bitfire.cadroid
Test File: CertificateInfoTest.java
Production File: CertificateInfo.java
protected void setUp() throws Exception {
assetManager = getInstrumentation().getContext().getAssets();
certificateFactory = CertificateFactory.getInstance("X.509");
infoDebianTestCA = loadCertificateInfo("DebianTestCA.pem");
infoDebianTestNoCA = loadCertificateInfo("DebianTestNoCA.pem");
infoGTECyberTrust = loadCertificateInfo("GTECyberTrustGlobalRoot.pem");
// user-submitted test cases
infoMehlMX = loadCertificateInfo("mehl.mx.pem");
}
public void testIsCA() {
assertTrue(infoDebianTestCA.isCA());
assertFalse(infoDebianTestNoCA.isCA());
assertNull(infoGTECyberTrust.isCA());
assertFalse(infoMehlMX.isCA());
}
The setup/fixture method initializes a total of 6 fields (variables). However, the test method, testIsCA()
, only utilizes 4 fields.
[↑]
App: com.gmail.walles.johan.batterylogger
Test File: SystemStateTest.java
Production File: SystemState.java
public void testPersistence() throws Exception {
File tempFile = File.createTempFile("systemstate-", ".txt");
try {
SystemState a = new SystemState(then, 27, false, bootTimestamp);
a.addInstalledApp("a.b.c", "ABC", "1.2.3");
a.writeToFile(tempFile);
SystemState b = SystemState.readFromFile(tempFile);
assertEquals(a, b);
} finally {
//noinspection ConstantConditions
if (tempFile != null) {
//noinspection ResultOfMethodCallIgnored
tempFile.delete();
}
}
}
As part of the test, the test method, testPersistence()
, creates a File (tempFile) in a specific directory and then utilizes this file in the test process.
[↑]
App: org.hwyl.sexytopo
Test File: Space3DTransformerTest.java
Production File: Space3DTransformer.java
@Test
public void testTransform10mNEUAndBack() {
Leg northEastAndUp10M = new Leg(10, 45, 45);
Coord3D result = transformer.transform(Coord3D.ORIGIN, northEastAndUp10M);
System.out.println("result = " + result);
Leg reverse = new Leg(10, 225, -45);
result = transformer.transform(result, reverse);
assertEquals(Coord3D.ORIGIN, result);
}
The test method, testTransform10mNEUAndBack()
, contains a statement that prints the value of a variable to the console. This is a redundant statement that might have been added by a developer, for debugging purposes, at the time of writing the test method.
[↑]
Test File: LoginActivityTest.java
Production File: LoginActivity.java
@Test
public void testTrue() {
assertEquals(true, true);
}
The test method, testTrue()
, will always pass as since the assert statement compares a Boolean value of true against another Boolean value of true.
[↑]
App: com.liveplayergames.finneypoker
Test File: RLPTest.java
Production File: RLP.java
@Test
public void test1() throws UnknownHostException {
String peersPacket = "F8 4E 11 F8 4B C5 36 81 " +
"CC 0A 29 82 76 5F B8 40 D8 D6 0C 25 80 FA 79 5C " +
"FC 03 13 EF DE BA 86 9D 21 94 E7 9E 7C B2 B5 22 " +
"F7 82 FF A0 39 2C BB AB 8D 1B AC 30 12 08 B1 37 " +
"E0 DE 49 98 33 4F 3B CF 73 FA 11 7E F2 13 F8 74 " +
"17 08 9F EA F8 4C 21 B0";
byte[] payload = Hex.decode(peersPacket);
byte[] ip = decodeIP4Bytes(payload, 5);
assertEquals(InetAddress.getByAddress(ip).toString(), ("/54.204.10.41"));
}
Use of the default value returned by an objects toString()
method, to perform string comparisons, runs the risk of failure in the future due to changes in the objects implementation of the toString()
method.
[↑]
App: com.pcinpact
Test File: ParseurHTMLTest.java
Production File: ParseurHTML.java
In this example, the test method, testGetListeArticles()
, has over 300 executable statements. Such methods not only increase test complexity, but also have a negative impact on code maintenance.
[↑]
App: sk.baka.aedict
Test File: ResultActivityTest.java
Production File: ResultActivity.java
public void testEdictExternSearch() throws Exception {
final Intent i = new Intent(getInstrumentation().getContext(), ResultActivity.class);
i.setAction(ResultActivity.EDICT_ACTION_INTERCEPT);
i.putExtra(ResultActivity.EDICT_INTENTKEY_KANJIS, "空白");
tester.startActivity(i);
assertTrue(tester.getText(R.id.textSelectedDictionary).contains("Default"));
final ListView lv = getActivity().getListView();
assertEquals(1, lv.getCount());
DictEntry entry = (DictEntry) lv.getItemAtPosition(0);
assertEquals("Searching", entry.english);
Thread.sleep(500);
final Intent i2 = getStartedActivityIntent();
final List<DictEntry> result = (List<DictEntry>) i2.getSerializableExtra(ResultActivity.INTENTKEY_RESULT_LIST);
entry = result.get(0);
assertEquals("(adj-na,n,adj-no) blank space/vacuum/space/null (NUL)/(P)", entry.english);
assertEquals("空白", entry.getJapanese());
assertEquals("くうはく", entry.reading);
assertEquals(1, result.size());
}
The developer causes an artificial delay in test execution using Thread.sleep()
. Without comments, it is assumed that the developer performs the delay to stimulate an actual activity (i.e. searching).
[↑]
Test File: NmeaSentenceTest.java
Production File: NmeaSentence.java
@Test
public void NmeaSentence_GPGSA_ReadValidValues(){
NmeaSentence nmeaSentence = new NmeaSentence("$GPGSA,A,3,04,05,,09,12,,,24,,,,,2.5,1.3,2.1*39");
assertThat("GPGSA - read PDOP", nmeaSentence.getLatestPdop(), is("2.5"));
assertThat("GPGSA - read HDOP", nmeaSentence.getLatestHdop(), is("1.3"));
assertThat("GPGSA - read VDOP", nmeaSentence.getLatestVdop(), is("2.1"));
}
In this test method, NmeaSentence_GPGSA_ReadValidValues()
, the developer calls multiple methods of the production class. Testing multiple methods of the production class in a single test method causes confusion as to what exactly the test method is testing.
[↑]
App: com.github.marmalade.aRevelation
Test File: CryptographerTest.java
Production File: Cryptographer.java
@Test
public void testDecrypt() throws Exception {
FileInputStream file = new FileInputStream(ENCRYPTED_DATA_FILE_4_14);
byte[] enfileData = new byte[file.available()];
FileInputStream input = new FileInputStream(DECRYPTED_DATA_FILE_4_14);
byte[] fileData = new byte[input.available()];
input.read(fileData);
input.close();
file.read(enfileData);
file.close();
String expectedResult = new String(fileData, "UTF-8");
assertEquals("Testing simple decrypt",expectedResult, Cryptographer.decrypt(enfileData, "test"));
}
@Test
public void testEncrypt() throws Exception {
String xml = readFileAsString(DECRYPTED_DATA_FILE_4_14);
byte[] encrypted = Cryptographer.encrypt(xml, "test");
String decrypt = Cryptographer.decrypt(encrypted, "test");
assertEquals(xml, decrypt);
}
Both test methods, testDecrypt()
and testEncrypt()
, call the same SUT method, Cryptographer.decrypt()
[↑]
App: org.openbmap
Test File: XmlSanitizerTest.java
Production File: XmlSanitizerTest.java
@Test
public void testXmlSanitizer() {
boolean valid = XmlSanitizer.isValid("Fritzbox");
assertEquals("Fritzbox is valid", true, valid);
System.out.println("Pure ASCII test - passed");
valid = XmlSanitizer.isValid("Fritz Box");
assertEquals("Spaces are valid", true, valid);
System.out.println("Spaces test - passed");
valid = XmlSanitizer.isValid("Frützbüx");
assertEquals("Frützbüx is invalid", false, valid);
System.out.println("No ASCII test - passed");
valid = XmlSanitizer.isValid("Fritz!box");
assertEquals("Exclamation mark is valid", true, valid);
System.out.println("Exclamation mark test - passed");
valid = XmlSanitizer.isValid("Fritz.box");
assertEquals("Exclamation mark is valid", true, valid);
System.out.println("Dot test - passed");
valid = XmlSanitizer.isValid("Fritz-box");
assertEquals("Minus is valid", true, valid);
System.out.println("Minus test - passed");
valid = XmlSanitizer.isValid("Fritz-box");
assertEquals("Minus is valid", true, valid);
System.out.println("Minus test - passed");
}
In this test method, testXmlSanitizer()
, the developer tests 'Exclamation mark is valid', 'Frützbüx is invalid' and 'Minus is valid' multiple times in the same test method.
[↑]
App: de.ktran.anno1404warenrechner
Test File: LogicTest.java
Production File: Logic.java
@Test
public void testChainDependencies() throws Exception {
final Game game = Game.newGame(0, "");
game.setOtherGoods(ProductionBuilding.TOOLMAKERS_WORKSHOP, 1);
final Logic logic = new Logic(game);
List<BuildingAlternative> res = logic.calculateChainWithDependencies(Goods.TOOLS);
System.out.println(res.toString());
}
This test method, testChainDependencies()
, hence the purpose of this test is not known.
[↑]