Skip to content

Commit

Permalink
Fixes #238 : Can use extracting with SoftAssertions
Browse files Browse the repository at this point in the history
Extracting was failing because the resulting Assert class was not
a proxy collecting errors.

This commit register a new method interceptor for cglib proxies: ProxifyExtractingResult.
If the intercepted method name contains "extracting", a filter choose to use ProxifyExtractingResult.
In all the other cases the interceptor used is the former ErrorCollector.
  • Loading branch information
jcgay authored and joel-costigliola committed Feb 17, 2015
1 parent fe0ddf0 commit d802115
Show file tree
Hide file tree
Showing 14 changed files with 442 additions and 66 deletions.
6 changes: 3 additions & 3 deletions src/ide-support/assertj-eclipse-formatter.xml
Expand Up @@ -17,7 +17,7 @@
<setting id="org.eclipse.jdt.core.formatter.insert_space_between_empty_parens_in_annotation_type_member_declaration" value="do not insert"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_before_comma_in_method_declaration_throws" value="do not insert"/>
<setting id="org.eclipse.jdt.core.formatter.comment.format_javadoc_comments" value="true"/>
<setting id="org.eclipse.jdt.core.formatter.indentation.size" value="2"/>
<setting id="org.eclipse.jdt.core.formatter.indentation.size" value="4"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_postfix_operator" value="do not insert"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_for_increments" value="insert"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_type_arguments" value="insert"/>
Expand Down Expand Up @@ -166,7 +166,7 @@
<setting id="org.eclipse.jdt.core.compiler.source" value="1.8"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_synchronized" value="do not insert"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_constructor_declaration_throws" value="insert"/>
<setting id="org.eclipse.jdt.core.formatter.tabulation.size" value="4"/>
<setting id="org.eclipse.jdt.core.formatter.tabulation.size" value="2"/>
<setting id="org.eclipse.jdt.core.formatter.insert_new_line_in_empty_enum_constant" value="insert"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_allocation_expression" value="insert"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_after_opening_bracket_in_array_reference" value="do not insert"/>
Expand Down Expand Up @@ -285,7 +285,7 @@
<setting id="org.eclipse.jdt.core.formatter.insert_space_before_question_in_conditional" value="insert"/>
<setting id="org.eclipse.jdt.core.formatter.comment.indent_parameter_description" value="true"/>
<setting id="org.eclipse.jdt.core.formatter.insert_new_line_before_finally_in_try_statement" value="do not insert"/>
<setting id="org.eclipse.jdt.core.formatter.tabulation.char" value="mixed"/>
<setting id="org.eclipse.jdt.core.formatter.tabulation.char" value="space"/>
<setting id="org.eclipse.jdt.core.formatter.insert_space_before_comma_in_multiple_field_declarations" value="do not insert"/>
<setting id="org.eclipse.jdt.core.formatter.blank_lines_between_import_groups" value="1"/>
<setting id="org.eclipse.jdt.core.formatter.lineSplit" value="120"/>
Expand Down
23 changes: 12 additions & 11 deletions src/main/java/org/assertj/core/api/AbstractIterableAssert.java
Expand Up @@ -12,16 +12,6 @@
*/
package org.assertj.core.api;

import static org.assertj.core.extractor.Extractors.byName;
import static org.assertj.core.extractor.Extractors.resultOf;
import static org.assertj.core.util.Iterables.toArray;
import static org.assertj.core.util.Lists.newArrayList;

import java.util.Collection;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;

import org.assertj.core.api.iterable.Extractor;
import org.assertj.core.groups.FieldsOrPropertiesExtractor;
import org.assertj.core.groups.Tuple;
Expand All @@ -37,6 +27,16 @@
import org.assertj.core.util.VisibleForTesting;
import org.assertj.core.util.introspection.IntrospectionError;

import java.util.Collection;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;

import static org.assertj.core.extractor.Extractors.byName;
import static org.assertj.core.extractor.Extractors.resultOf;
import static org.assertj.core.util.Iterables.toArray;
import static org.assertj.core.util.Lists.newArrayList;

/**
* Base class for implementations of <code>{@link ObjectEnumerableAssert}</code> whose actual value type is
* <code>{@link Collection}</code>.
Expand Down Expand Up @@ -822,7 +822,8 @@ public <P> ListAssert<P> extracting(String propertyOrField, Class<P> extractingT
* public) in one of the initial Iterable's element.
*/
public ListAssert<Tuple> extracting(String... propertiesOrFields) {
return extracting(byName(propertiesOrFields));
List<Tuple> values = FieldsOrPropertiesExtractor.extract(actual, byName(propertiesOrFields));
return new ListAssert<>(values);
}

/**
Expand Down
14 changes: 3 additions & 11 deletions src/main/java/org/assertj/core/api/AbstractSoftAssertions.java
Expand Up @@ -12,24 +12,16 @@
*/
package org.assertj.core.api;

import static org.assertj.core.util.Arrays.array;
import net.sf.cglib.proxy.Enhancer;

public class AbstractSoftAssertions {

protected final ErrorCollector collector;
protected final SoftProxies proxies;

public AbstractSoftAssertions() {
super();
this.collector = new ErrorCollector();
proxies = new SoftProxies();
}

@SuppressWarnings("unchecked")
protected <T, V> V proxy(Class<V> assertClass, Class<T> actualClass, T actual) {
Enhancer enhancer = new Enhancer();
enhancer.setSuperclass(assertClass);
enhancer.setCallback(collector);
return (V) enhancer.create(array(actualClass), array(actual));
return proxies.create(assertClass, actualClass, actual);
}

}
6 changes: 3 additions & 3 deletions src/main/java/org/assertj/core/api/BDDSoftAssertions.java
Expand Up @@ -12,10 +12,10 @@
*/
package org.assertj.core.api;

import static org.assertj.core.groups.Properties.extractProperty;

import java.util.List;

import static org.assertj.core.groups.Properties.extractProperty;

/**
* <p>
* Suppose we have a test case and in it we'd like to make numerous BDD assertions. In this case, we're hosting a dinner
Expand Down Expand Up @@ -135,7 +135,7 @@ public BDDSoftAssertions() {
* @throws SoftAssertionError if any proxied assertion objects threw
*/
public void assertAll() {
List<Throwable> errors = collector.errors();
List<Throwable> errors = proxies.errorsCollected();
if (!errors.isEmpty()) {
throw new SoftAssertionError(extractProperty("message", String.class).from(errors));
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/assertj/core/api/ErrorCollector.java
Expand Up @@ -12,16 +12,16 @@
*/
package org.assertj.core.api;

import net.sf.cglib.proxy.MethodInterceptor;
import net.sf.cglib.proxy.MethodProxy;

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import net.sf.cglib.proxy.MethodInterceptor;
import net.sf.cglib.proxy.MethodProxy;

/** Collects error messages of all AssertionErrors thrown by the proxied method. */
public class ErrorCollector implements MethodInterceptor {
class ErrorCollector implements MethodInterceptor {

private final List<Throwable> errors = new ArrayList<Throwable>();

Expand Down
Expand Up @@ -18,6 +18,8 @@
import org.junit.runners.model.MultipleFailureException;
import org.junit.runners.model.Statement;

import java.util.List;

/**
* Same as {@link SoftAssertions}, but with the following differences: <br/>
* First, it's a junit rule, which can be used without having to call {@link SoftAssertions#assertAll() assertAll()},
Expand Down Expand Up @@ -46,7 +48,7 @@ public Statement apply(final Statement base, Description description) {
@Override
public void evaluate() throws Throwable {
base.evaluate();
MultipleFailureException.assertEmpty(collector.errors());
MultipleFailureException.assertEmpty(proxies.errorsCollected());
}
};
}
Expand All @@ -55,9 +57,8 @@ public JUnitBDDSoftAssertions() {
super();
}

@VisibleForTesting
ErrorCollector getCollector() {
return collector;
@VisibleForTesting List<Throwable> getErrors() {
return proxies.errorsCollected();
}

}
9 changes: 5 additions & 4 deletions src/main/java/org/assertj/core/api/JUnitSoftAssertions.java
Expand Up @@ -18,6 +18,8 @@
import org.junit.runners.model.MultipleFailureException;
import org.junit.runners.model.Statement;

import java.util.List;

/**
* Same as {@link SoftAssertions}, but with the following differences: <br/>
* First, it's a junit rule, which can be used without having to call {@link SoftAssertions#assertAll() assertAll()},
Expand Down Expand Up @@ -46,7 +48,7 @@ public Statement apply(final Statement base, Description description) {
@Override
public void evaluate() throws Throwable {
base.evaluate();
MultipleFailureException.assertEmpty(collector.errors());
MultipleFailureException.assertEmpty(proxies.errorsCollected());
}
};
}
Expand All @@ -55,9 +57,8 @@ public JUnitSoftAssertions() {
super();
}

@VisibleForTesting
ErrorCollector getCollector() {
return collector;
@VisibleForTesting List<Throwable> getErrors() {
return proxies.errorsCollected();
}

}
66 changes: 66 additions & 0 deletions src/main/java/org/assertj/core/api/ProxifyExtractingResult.java
@@ -0,0 +1,66 @@
/**
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* Copyright 2012-2014 the original author or authors.
*/
package org.assertj.core.api;

import net.sf.cglib.proxy.MethodInterceptor;
import net.sf.cglib.proxy.MethodProxy;

import java.lang.reflect.Array;
import java.lang.reflect.Method;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;

class ProxifyExtractingResult implements MethodInterceptor {

private final SoftProxies proxies;

ProxifyExtractingResult(SoftProxies proxies) {
this.proxies = proxies;
}

@SuppressWarnings("unchecked")
@Override
public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable {

Object result = proxy.invokeSuper(obj, args);
return proxies.create(result.getClass(), actualClass(result), actual(result));
}

@SuppressWarnings("rawtypes")
private static Class actualClass(Object result) {
if (result instanceof ObjectArrayAssert) {
return Array.newInstance(Object.class, 0).getClass();
}

// Trying to create a proxy with cglib will only match exact constructor argument types.
// To initialize one for ListAssert for example we can't use an ArrayList, we have to use a List.
// So we can't just return actual.getClass() as we could read a concrete class whereas
// *Assert classes define a constructor using interface (@see ListAssert for example).
//
// Instead we can read generic types from *Assert definition.
// Inspecting: class ListAssert<T> extends AbstractListAssert<ListAssert<T>, List<? extends T>, T>
// will return the generic defined by the super class AbstractListAssert at index 1, which is a List<? extends T>
Type actualType = ((ParameterizedType) result.getClass().getGenericSuperclass()).getActualTypeArguments()[1];
if (actualType instanceof ParameterizedType) {
return (Class<?>) ((ParameterizedType) actualType).getRawType();
}

return (Class<?>) actualType;
}

private static Object actual(Object result) {
if (result instanceof AbstractAssert) return ((AbstractAssert<?, ?>) result).actual;
throw new IllegalStateException("We should be trying to make a proxy of an *Assert class.");
}

}
6 changes: 3 additions & 3 deletions src/main/java/org/assertj/core/api/SoftAssertions.java
Expand Up @@ -12,10 +12,10 @@
*/
package org.assertj.core.api;

import static org.assertj.core.groups.Properties.extractProperty;

import java.util.List;

import static org.assertj.core.groups.Properties.extractProperty;

/**
* <p>
* Suppose we have a test case and in it we'd like to make numerous assertions. In this case, we're hosting a dinner
Expand Down Expand Up @@ -134,7 +134,7 @@ public SoftAssertions() {
* @throws SoftAssertionError if any proxied assertion objects threw
*/
public void assertAll() {
List<Throwable> errors = collector.errors();
List<Throwable> errors = proxies.errorsCollected();
if (!errors.isEmpty()) {
throw new SoftAssertionError(extractProperty("message", String.class).from(errors));
}
Expand Down
55 changes: 55 additions & 0 deletions src/main/java/org/assertj/core/api/SoftProxies.java
@@ -0,0 +1,55 @@
/**
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* Copyright 2012-2014 the original author or authors.
*/
package org.assertj.core.api;

import net.sf.cglib.proxy.Callback;
import net.sf.cglib.proxy.CallbackFilter;
import net.sf.cglib.proxy.Enhancer;

import java.lang.reflect.Method;
import java.util.List;

import static org.assertj.core.util.Arrays.array;

class SoftProxies {

private final ErrorCollector collector = new ErrorCollector();

List<Throwable> errorsCollected() {
return collector.errors();
}

@SuppressWarnings("unchecked")
<V, T> V create(Class<V> assertClass, Class<T> actualClass, T actual) {
Enhancer enhancer = new Enhancer();
enhancer.setSuperclass(assertClass);
enhancer.setCallbackFilter(CollectErrorsOrCreateExtractedProxy.FILTER);
enhancer.setCallbacks(new Callback[] { collector, new ProxifyExtractingResult(this) });
return (V) enhancer.create(array(actualClass), array(actual));
}

private enum CollectErrorsOrCreateExtractedProxy implements CallbackFilter {
FILTER;

private static final int ERROR_COLLECTOR_INDEX = 0;
private static final int PROXIFY_EXTRACTING_INDEX = 1;

public int accept(Method method) {
return isExtractingMethod(method) ? PROXIFY_EXTRACTING_INDEX : ERROR_COLLECTOR_INDEX;
}

private boolean isExtractingMethod(Method method) {
return method.getName().toLowerCase().contains("extracting");
}
}
}
Expand Up @@ -12,15 +12,15 @@
*/
package org.assertj.core.api;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.fail;

import java.util.List;

import org.assertj.core.util.Lists;
import org.junit.Test;
import org.junit.runners.model.MultipleFailureException;

import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.fail;

public class JUnitBDDSoftAssertionsFailureTest {

// we cannot make it a rule here, because we need to test the failure without this test failing!
Expand All @@ -33,7 +33,7 @@ public void should_report_all_errors() throws Throwable {
softly.then(1).isEqualTo(1);
softly.then(1).isEqualTo(2);
softly.then(Lists.newArrayList(1, 2)).containsOnly(1, 3);
MultipleFailureException.assertEmpty(softly.getCollector().errors());
MultipleFailureException.assertEmpty(softly.getErrors());
fail("Should not reach here");
} catch (MultipleFailureException e) {
List<Throwable> failures = e.getFailures();
Expand Down

0 comments on commit d802115

Please sign in to comment.