Skip to content

[ci] Use assertj/fluent exceptions for cleaner unit testing #977

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 1 commit into from
Apr 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,18 @@
<version>9.1-901-1.jdbc4</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>1.7.1</version> <!-- Stay on 1.7.1 to support Java 6 -->
<scope>test</scope>
</dependency>
<dependency>
<groupId>eu.codearte.catch-exception</groupId>
<artifactId>catch-exception</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

@hazendaz Is need catch-exception ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Some of the junit rules needed switched to something. So this was required in a number of cases. In fact, I ended up just clearing all junit rules entirely out. One major benefit to using this method is that catch exception proxies the exception so code coverage on unit test looks better. That library isn't necessary once on java 8 though. For now just needed something to fill the gap. Although direct usage of try/catch with reading the exceptions would have also worked.

<version>1.4.4</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import org.apache.ibatis.session.SqlSession;
import org.apache.ibatis.session.SqlSessionFactory;
import org.apache.ibatis.session.SqlSessionFactoryBuilder;
import org.hamcrest.CoreMatchers;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
Expand Down Expand Up @@ -109,6 +109,6 @@ public void badSubject() {

private void verifySubjects(final List<?> subjects) {
Assert.assertNotNull(subjects);
Assert.assertThat(subjects.size(), CoreMatchers.equalTo(3));
Assertions.assertThat(subjects.size()).isEqualTo(3);
}
}
9 changes: 4 additions & 5 deletions src/test/java/org/apache/ibatis/binding/FlushTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2009-2015 the original author or authors.
* Copyright 2009-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -31,9 +31,8 @@
import java.util.ArrayList;
import java.util.List;

import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.assertj.core.api.Assertions.assertThat;

public class FlushTest {
private static SqlSessionFactory sqlSessionFactory;
Expand Down Expand Up @@ -75,8 +74,8 @@ public void invokeFlushStatementsViaMapper() {
// test
List<BatchResult> results = mapper.flush();

assertThat(results.size(), is(1));
assertThat(results.get(0).getUpdateCounts().length, is(ids.size()));
assertThat(results.size()).isEqualTo(1);
assertThat(results.get(0).getUpdateCounts().length).isEqualTo(ids.size());

for (int id : ids) {
Author selectedAuthor = mapper.selectAuthor(id);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2009-2016 the original author or authors.
* Copyright 2009-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,17 +17,10 @@

import java.util.Map;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import static org.hamcrest.core.Is.is;

public class ParameterExpressionTest {

@Rule
public ExpectedException expectedException = ExpectedException.none();

@Test
public void simpleProperty() {
Map<String, String> result = new ParameterExpression("id");
Expand Down Expand Up @@ -133,16 +126,20 @@ public void shouldIgnoreLeadingAndTrailingSpaces() {

@Test
public void invalidOldJdbcTypeFormat() {
expectedException.expect(BuilderException.class);
expectedException.expectMessage(is("Parsing error in {id:} in position 3"));
new ParameterExpression("id:");
try {
new ParameterExpression("id:");
} catch (BuilderException e) {
e.getMessage().contains("Parsing error in {id:} in position 3");
Copy link
Member

Choose a reason for hiding this comment

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

@hazendaz ,
Thanks for the work!
I think there needs to be a call to fail() in the try clause, otherwise the test passes even when there is no exception thrown.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! I'll go back through and add those.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, and it also needs assertTrue or something to verify the result of contains() method XD

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it! I'll try to take care of those tomorrow night.

}
}

@Test
public void invalidJdbcTypeOptUsingExpression() {
expectedException.expect(BuilderException.class);
expectedException.expectMessage(is("Parsing error in {(expression)+} in position 12"));
new ParameterExpression("(expression)+");
try {
new ParameterExpression("(expression)+");
} catch (BuilderException e) {
e.getMessage().contains("Parsing error in {(expression)+} in position 12");
}
}

}
183 changes: 91 additions & 92 deletions src/test/java/org/apache/ibatis/builder/XmlConfigBuilderTest.java

Large diffs are not rendered by default.

106 changes: 56 additions & 50 deletions src/test/java/org/apache/ibatis/builder/XmlMapperBuilderTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2009-2016 the original author or authors.
* Copyright 2009-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -25,17 +25,14 @@
import org.apache.ibatis.mapping.StatementType;
import org.apache.ibatis.session.Configuration;
import org.apache.ibatis.type.TypeHandler;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import static org.junit.Assert.assertThat;
import static org.hamcrest.CoreMatchers.*;
import static com.googlecode.catchexception.apis.BDDCatchException.*;
import static org.assertj.core.api.BDDAssertions.then;

public class XmlMapperBuilderTest {
import static org.assertj.core.api.Assertions.assertThat;

@Rule
public ExpectedException expectedException = ExpectedException.none();
public class XmlMapperBuilderTest {

@Test
public void shouldSuccessfullyLoadXMLMapperFile() throws Exception {
Expand All @@ -44,6 +41,7 @@ public void shouldSuccessfullyLoadXMLMapperFile() throws Exception {
InputStream inputStream = Resources.getResourceAsStream(resource);
XMLMapperBuilder builder = new XMLMapperBuilder(inputStream, configuration, resource, configuration.getSqlFragments());
builder.parse();
inputStream.close();
}

@Test
Expand All @@ -55,110 +53,118 @@ public void mappedStatementWithOptions() throws Exception {
builder.parse();

MappedStatement mappedStatement = configuration.getMappedStatement("selectWithOptions");
assertThat(mappedStatement.getFetchSize(), is(200));
assertThat(mappedStatement.getTimeout(), is(10));
assertThat(mappedStatement.getStatementType(), is(StatementType.PREPARED));
assertThat(mappedStatement.getResultSetType(), is(ResultSetType.SCROLL_SENSITIVE));
assertThat(mappedStatement.isFlushCacheRequired(), is(false));
assertThat(mappedStatement.isUseCache(), is(false));

assertThat(mappedStatement.getFetchSize()).isEqualTo(200);
assertThat(mappedStatement.getTimeout()).isEqualTo(10);
assertThat(mappedStatement.getStatementType()).isEqualTo(StatementType.PREPARED);
assertThat(mappedStatement.getResultSetType()).isEqualTo(ResultSetType.SCROLL_SENSITIVE);
assertThat(mappedStatement.isFlushCacheRequired()).isFalse();
assertThat(mappedStatement.isUseCache()).isFalse();
inputStream.close();
}

@Test
public void parseExpression() {
BaseBuilder builder = new BaseBuilder(new Configuration()){{}};
{
Pattern pattern = builder.parseExpression("[0-9]", "[a-z]");
assertThat(pattern.matcher("0").find(), is(true));
assertThat(pattern.matcher("a").find(), is(false));
assertThat(pattern.matcher("0").find()).isTrue();
assertThat(pattern.matcher("a").find()).isFalse();
}
{
Pattern pattern = builder.parseExpression(null, "[a-z]");
assertThat(pattern.matcher("0").find(), is(false));
assertThat(pattern.matcher("a").find(), is(true));
assertThat(pattern.matcher("0").find()).isFalse();
assertThat(pattern.matcher("a").find()).isTrue();
}
}

@Test
public void resolveJdbcTypeWithUndefinedValue() {
BaseBuilder builder = new BaseBuilder(new Configuration()){{}};
expectedException.expect(BuilderException.class);
expectedException.expectMessage(startsWith("Error resolving JdbcType. Cause: java.lang.IllegalArgumentException: No enum"));
expectedException.expectMessage(endsWith("org.apache.ibatis.type.JdbcType.aaa"));
builder.resolveJdbcType("aaa");
when(builder).resolveJdbcType("aaa");
then(caughtException())
.isInstanceOf(BuilderException.class)
.hasMessageStartingWith("Error resolving JdbcType. Cause: java.lang.IllegalArgumentException: No enum")
.hasMessageEndingWith("org.apache.ibatis.type.JdbcType.aaa");
}

@Test
public void resolveResultSetTypeWithUndefinedValue() {
BaseBuilder builder = new BaseBuilder(new Configuration()){{}};
expectedException.expect(BuilderException.class);
expectedException.expectMessage(startsWith("Error resolving ResultSetType. Cause: java.lang.IllegalArgumentException: No enum"));
expectedException.expectMessage(endsWith("org.apache.ibatis.mapping.ResultSetType.bbb"));
builder.resolveResultSetType("bbb");
when(builder).resolveResultSetType("bbb");
then(caughtException())
.isInstanceOf(BuilderException.class)
.hasMessageStartingWith("Error resolving ResultSetType. Cause: java.lang.IllegalArgumentException: No enum")
.hasMessageEndingWith("org.apache.ibatis.mapping.ResultSetType.bbb");
}

@Test
public void resolveParameterModeWithUndefinedValue() {
BaseBuilder builder = new BaseBuilder(new Configuration()){{}};
expectedException.expect(BuilderException.class);
expectedException.expectMessage(startsWith("Error resolving ParameterMode. Cause: java.lang.IllegalArgumentException: No enum"));
expectedException.expectMessage(endsWith("org.apache.ibatis.mapping.ParameterMode.ccc"));
builder.resolveParameterMode("ccc");
when(builder).resolveParameterMode("ccc");
then(caughtException())
.isInstanceOf(BuilderException.class)
.hasMessageStartingWith("Error resolving ParameterMode. Cause: java.lang.IllegalArgumentException: No enum")
.hasMessageEndingWith("org.apache.ibatis.mapping.ParameterMode.ccc");
}

@Test
public void createInstanceWithAbstractClass() {
BaseBuilder builder = new BaseBuilder(new Configuration()){{}};
expectedException.expect(BuilderException.class);
expectedException.expectMessage(is("Error creating instance. Cause: java.lang.InstantiationException: org.apache.ibatis.builder.BaseBuilder"));
builder.createInstance("org.apache.ibatis.builder.BaseBuilder");
when(builder).createInstance("org.apache.ibatis.builder.BaseBuilder");
then(caughtException())
.isInstanceOf(BuilderException.class)
.hasMessage("Error creating instance. Cause: java.lang.InstantiationException: org.apache.ibatis.builder.BaseBuilder");
}

@Test
public void resolveClassWithNotFound() {
BaseBuilder builder = new BaseBuilder(new Configuration()){{}};
expectedException.expect(BuilderException.class);
expectedException.expectMessage(is("Error resolving class. Cause: org.apache.ibatis.type.TypeException: Could not resolve type alias 'ddd'. Cause: java.lang.ClassNotFoundException: Cannot find class: ddd"));
builder.resolveClass("ddd");
when(builder).resolveClass("ddd");
then(caughtException())
.isInstanceOf(BuilderException.class)
.hasMessage("Error resolving class. Cause: org.apache.ibatis.type.TypeException: Could not resolve type alias 'ddd'. Cause: java.lang.ClassNotFoundException: Cannot find class: ddd");
}

@Test
public void resolveTypeHandlerTypeHandlerAliasIsNull() {
BaseBuilder builder = new BaseBuilder(new Configuration()){{}};
TypeHandler<?> typeHandler = builder.resolveTypeHandler(String.class, (String)null);
assertThat(typeHandler, nullValue());
assertThat(typeHandler).isNull();
}

@Test
public void resolveTypeHandlerNoAssignable() {
BaseBuilder builder = new BaseBuilder(new Configuration()){{}};
expectedException.expect(BuilderException.class);
expectedException.expectMessage(is("Type java.lang.Integer is not a valid TypeHandler because it does not implement TypeHandler interface"));
builder.resolveTypeHandler(String.class, "integer");
when(builder).resolveTypeHandler(String.class, "integer");
then(caughtException())
.isInstanceOf(BuilderException.class)
.hasMessage("Type java.lang.Integer is not a valid TypeHandler because it does not implement TypeHandler interface");
}

@Test
public void setCurrentNamespaceValueIsNull() {
MapperBuilderAssistant builder = new MapperBuilderAssistant(new Configuration(), "resource");
expectedException.expect(BuilderException.class);
expectedException.expectMessage(is("The mapper element requires a namespace attribute to be specified."));
builder.setCurrentNamespace(null);
when(builder).setCurrentNamespace(null);
then(caughtException())
.isInstanceOf(BuilderException.class)
.hasMessage("The mapper element requires a namespace attribute to be specified.");
}

@Test
public void useCacheRefNamespaceIsNull() {
MapperBuilderAssistant builder = new MapperBuilderAssistant(new Configuration(), "resource");
expectedException.expect(BuilderException.class);
expectedException.expectMessage(is("cache-ref element requires a namespace attribute."));
builder.useCacheRef(null);
when(builder).useCacheRef(null);
then(caughtException())
.isInstanceOf(BuilderException.class)
.hasMessage("cache-ref element requires a namespace attribute.");
}

@Test
public void useCacheRefNamespaceIsUndefined() {
MapperBuilderAssistant builder = new MapperBuilderAssistant(new Configuration(), "resource");
expectedException.expect(IncompleteElementException.class);
expectedException.expectMessage(is("No cache for namespace 'eee' could be found."));
builder.useCacheRef("eee");
when(builder).useCacheRef("eee");
then(caughtException())
.hasMessage("No cache for namespace 'eee' could be found.");
}

// @Test
Expand Down
23 changes: 11 additions & 12 deletions src/test/java/org/apache/ibatis/executor/ResultExtractorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@

import java.util.*;

import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.assertThat;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.*;

@RunWith(MockitoJUnitRunner.class)
Expand All @@ -48,25 +47,25 @@ public void setUp() throws Exception {
@Test
public void shouldExtractNullForNullTargetType() {
final Object result = resultExtractor.extractObjectFromList(null, null);
assertThat(result, nullValue());
assertThat(result).isNull();
}

@Test
public void shouldExtractList() {
final List list = Arrays.asList(1, 2, 3);
final Object result = resultExtractor.extractObjectFromList(list, List.class);
assertThat(result, instanceOf(List.class));
assertThat(result).isInstanceOf(List.class);
final List resultList = (List) result;
assertThat(resultList, equalTo(list));
assertThat(resultList).isEqualTo(list);
}

@Test
public void shouldExtractArray() {
final List list = Arrays.asList(1, 2, 3);
final Object result = resultExtractor.extractObjectFromList(list, Integer[].class);
assertThat(result, instanceOf(Integer[].class));
assertThat(result).isInstanceOf(Integer[].class);
final Integer[] resultArray = (Integer[]) result;
assertThat(resultArray, equalTo(new Integer[]{1, 2, 3}));
assertThat(resultArray).isEqualTo(new Integer[]{1, 2, 3});
}

@Test
Expand All @@ -80,22 +79,22 @@ public void shouldExtractSet() {
when(configuration.newMetaObject(set)).thenReturn(metaObject);

final Set result = (Set) resultExtractor.extractObjectFromList(list, targetType);
assertThat(result, sameInstance(set));
assertThat(result).isSameAs(set);

verify(metaObject).addAll(list);
}

@Test
public void shouldExtractSingleObject() {
final List list = Collections.singletonList("single object");
assertThat((String) resultExtractor.extractObjectFromList(list, String.class), equalTo("single object"));
assertThat((String) resultExtractor.extractObjectFromList(list, null), equalTo("single object"));
assertThat((String) resultExtractor.extractObjectFromList(list, Integer.class), equalTo("single object"));
assertThat((String) resultExtractor.extractObjectFromList(list, String.class)).isEqualTo("single object");
assertThat((String) resultExtractor.extractObjectFromList(list, null)).isEqualTo("single object");
assertThat((String) resultExtractor.extractObjectFromList(list, Integer.class)).isEqualTo("single object");
}

@Test(expected = ExecutorException.class)
public void shouldFailWhenMutipleItemsInList() {
final List list = Arrays.asList("first object", "second object");
assertThat((String) resultExtractor.extractObjectFromList(list, String.class), equalTo("single object"));
assertThat((String) resultExtractor.extractObjectFromList(list, String.class)).isEqualTo("single object");
}
}
Loading