Skip to content

Commit

Permalink
Prevent improper use of testing framework APIs
Browse files Browse the repository at this point in the history
Prior to this commit, a lot of work had been done to prevent improper
use of testing Framework APIs throughout the codebase; however, there
were still some loopholes.

This commit addresses these loopholes by introducing additional
Checkstyle rules (and modifying existing rules) to prevent improper use
of testing framework APIs in production code as well as in test code.

- Checkstyle rules for banned imports have been refactored into
  multiple rules specific to JUnit 3, JUnit 4, JUnit Jupiter, and
  TestNG.
- Accidental usage of org.junit.Assume has been switched to
  org.junit.jupiter.api.Assumptions.
- All test classes now reside under org.springframework packages.
- All test classes (including abstract test classes) now conform to the
  `*Tests` naming convention.
  - As an added bonus, tests in the renamed
    ScenariosForSpringSecurityExpressionTests are now included in the
    build.
- Dead JUnit 4 parameterized code has been removed from
  DefaultServerWebExchangeCheckNotModifiedTests.

Closes spring-projectsgh-22962
  • Loading branch information
sbrannen committed Sep 12, 2019
1 parent 92d3f7e commit 30cff46
Show file tree
Hide file tree
Showing 50 changed files with 163 additions and 170 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2019 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 @@ -14,12 +14,13 @@
* limitations under the License.
*/

package com.foo;
package org.springframework.beans.factory.xml;

import java.util.ArrayList;
import java.util.List;

public class Component {

private String name;
private List<Component> components = new ArrayList<>();

Expand All @@ -39,4 +40,5 @@ public String getName() {
public void setName(String name) {
this.name = name;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2019 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 @@ -14,7 +14,7 @@
* limitations under the License.
*/

package com.foo;
package org.springframework.beans.factory.xml;

import java.util.List;

Expand All @@ -24,8 +24,6 @@
import org.springframework.beans.factory.support.AbstractBeanDefinition;
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
import org.springframework.beans.factory.support.ManagedList;
import org.springframework.beans.factory.xml.AbstractBeanDefinitionParser;
import org.springframework.beans.factory.xml.ParserContext;
import org.springframework.util.CollectionUtils;
import org.springframework.util.xml.DomUtils;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,52 +14,49 @@
* limitations under the License.
*/

package com.foo;

package org.springframework.beans.factory.xml;

import java.util.List;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;

import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
import org.springframework.core.io.ClassPathResource;

import static org.assertj.core.api.Assertions.assertThat;


/**
* @author Costin Leau
*/
public class ComponentBeanDefinitionParserTests {
@TestInstance(Lifecycle.PER_CLASS)
class ComponentBeanDefinitionParserTests {

private final DefaultListableBeanFactory bf = new DefaultListableBeanFactory();

private static DefaultListableBeanFactory bf;

@BeforeAll
public static void setUpBeforeClass() throws Exception {
bf = new DefaultListableBeanFactory();
new XmlBeanDefinitionReader(bf).loadBeanDefinitions(new ClassPathResource("com/foo/component-config.xml"));
void setUp() throws Exception {
new XmlBeanDefinitionReader(bf).loadBeanDefinitions(
new ClassPathResource("component-config.xml", ComponentBeanDefinitionParserTests.class));
}

@AfterAll
public static void tearDownAfterClass() throws Exception {
void tearDown() {
bf.destroySingletons();
}

private Component getBionicFamily() {
return bf.getBean("bionic-family", Component.class);
}

@Test
public void testBionicBasic() throws Exception {
void testBionicBasic() {
Component cp = getBionicFamily();
assertThat("Bionic-1").isEqualTo(cp.getName());
}

@Test
public void testBionicFirstLevelChildren() throws Exception {
void testBionicFirstLevelChildren() {
Component cp = getBionicFamily();
List<Component> components = cp.getComponents();
assertThat(2).isEqualTo(components.size());
Expand All @@ -68,11 +65,17 @@ public void testBionicFirstLevelChildren() throws Exception {
}

@Test
public void testBionicSecondLevelChildren() throws Exception {
void testBionicSecondLevelChildren() {
Component cp = getBionicFamily();
List<Component> components = cp.getComponents().get(0).getComponents();
assertThat(2).isEqualTo(components.size());
assertThat("Karate-1").isEqualTo(components.get(0).getName());
assertThat("Sport-1").isEqualTo(components.get(1).getName());
}

private Component getBionicFamily() {
return bf.getBean("bionic-family", Component.class);
}

}

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2019 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 @@ -14,13 +14,14 @@
* limitations under the License.
*/

package com.foo;
package org.springframework.beans.factory.xml;

import java.util.List;

import org.springframework.beans.factory.FactoryBean;

public class ComponentFactoryBean implements FactoryBean<Component> {

private Component parent;
private List<Component> children;

Expand Down Expand Up @@ -51,4 +52,5 @@ public Class<Component> getObjectType() {
public boolean isSingleton() {
return true;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2019 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 @@ -14,14 +14,12 @@
* limitations under the License.
*/

package com.foo;

import org.springframework.beans.factory.xml.NamespaceHandlerSupport;
package org.springframework.beans.factory.xml;

public class ComponentNamespaceHandler extends NamespaceHandlerSupport {

@Override
public void init() {
registerBeanDefinitionParser("component",
new ComponentBeanDefinitionParser());
registerBeanDefinitionParser("component", new ComponentBeanDefinitionParser());
}
}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
http\://www.foo.example/schema/component=com.foo.ComponentNamespaceHandler
http\://www.foo.example/schema/component=org.springframework.beans.factory.xml.ComponentNamespaceHandler
Original file line number Diff line number Diff line change
@@ -1 +1 @@
http\://www.foo.example/schema/component/component.xsd=com/foo/component.xsd
http\://www.foo.example/schema/component/component.xsd=org/springframework/beans/factory/xml/component.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIOException;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.junit.Assume.assumeTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

/**
* @author Rob Harrop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import reactor.test.StepVerifier;

import org.springframework.core.ResolvableType;
import org.springframework.core.io.buffer.AbstractLeakCheckingTestCase;
import org.springframework.core.io.buffer.AbstractLeakCheckingTests;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
Expand All @@ -44,18 +44,18 @@
* @since 5.1.3
*/
@SuppressWarnings("ProtectedField")
public abstract class AbstractDecoderTestCase<D extends Decoder<?>> extends AbstractLeakCheckingTestCase {
public abstract class AbstractDecoderTests<D extends Decoder<?>> extends AbstractLeakCheckingTests {

/**
* The decoder to test.
*/
protected D decoder;

/**
* Construct a new {@code AbstractDecoderTestCase} for the given decoder.
* Construct a new {@code AbstractDecoderTests} instance for the given decoder.
* @param decoder the decoder
*/
protected AbstractDecoderTestCase(D decoder) {
protected AbstractDecoderTests(D decoder) {
Assert.notNull(decoder, "Encoder must not be null");

this.decoder = decoder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import reactor.test.StepVerifier;

import org.springframework.core.ResolvableType;
import org.springframework.core.io.buffer.AbstractLeakCheckingTestCase;
import org.springframework.core.io.buffer.AbstractLeakCheckingTests;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferUtils;
import org.springframework.lang.Nullable;
Expand All @@ -45,7 +45,7 @@
* @since 5.1.3
*/
@SuppressWarnings("ProtectedField")
public abstract class AbstractEncoderTestCase<E extends Encoder<?>> extends AbstractLeakCheckingTestCase {
public abstract class AbstractEncoderTests<E extends Encoder<?>> extends AbstractLeakCheckingTests {

/**
* The encoder to test.
Expand All @@ -57,7 +57,7 @@ public abstract class AbstractEncoderTestCase<E extends Encoder<?>> extends Abst
* Construct a new {@code AbstractEncoderTestCase} for the given parameters.
* @param encoder the encoder
*/
protected AbstractEncoderTestCase(E encoder) {
protected AbstractEncoderTests(E encoder) {

Assert.notNull(encoder, "Encoder must not be null");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
/**
* @author Arjen Poutsma
*/
class ByteArrayDecoderTests extends AbstractDecoderTestCase<ByteArrayDecoder> {
class ByteArrayDecoderTests extends AbstractDecoderTests<ByteArrayDecoder> {

private final byte[] fooBytes = "foo".getBytes(StandardCharsets.UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
/**
* @author Arjen Poutsma
*/
class ByteArrayEncoderTests extends AbstractEncoderTestCase<ByteArrayEncoder> {
class ByteArrayEncoderTests extends AbstractEncoderTests<ByteArrayEncoder> {

private final byte[] fooBytes = "foo".getBytes(StandardCharsets.UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
/**
* @author Sebastien Deleuze
*/
class ByteBufferDecoderTests extends AbstractDecoderTestCase<ByteBufferDecoder> {
class ByteBufferDecoderTests extends AbstractDecoderTests<ByteBufferDecoder> {

private final byte[] fooBytes = "foo".getBytes(StandardCharsets.UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
/**
* @author Sebastien Deleuze
*/
class ByteBufferEncoderTests extends AbstractEncoderTestCase<ByteBufferEncoder> {
class ByteBufferEncoderTests extends AbstractEncoderTests<ByteBufferEncoder> {

private final byte[] fooBytes = "foo".getBytes(StandardCharsets.UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
/**
* @author Sebastien Deleuze
*/
class CharSequenceEncoderTests extends AbstractEncoderTestCase<CharSequenceEncoder> {
class CharSequenceEncoderTests extends AbstractEncoderTests<CharSequenceEncoder> {

private final String foo = "foo";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
/**
* @author Sebastien Deleuze
*/
class DataBufferDecoderTests extends AbstractDecoderTestCase<DataBufferDecoder> {
class DataBufferDecoderTests extends AbstractDecoderTests<DataBufferDecoder> {

private final byte[] fooBytes = "foo".getBytes(StandardCharsets.UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
/**
* @author Sebastien Deleuze
*/
class DataBufferEncoderTests extends AbstractEncoderTestCase<DataBufferEncoder> {
class DataBufferEncoderTests extends AbstractEncoderTests<DataBufferEncoder> {

private final byte[] fooBytes = "foo".getBytes(StandardCharsets.UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
/**
* @author Arjen Poutsma
*/
class ResourceDecoderTests extends AbstractDecoderTestCase<ResourceDecoder> {
class ResourceDecoderTests extends AbstractDecoderTests<ResourceDecoder> {

private final byte[] fooBytes = "foo".getBytes(StandardCharsets.UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
/**
* @author Arjen Poutsma
*/
class ResourceEncoderTests extends AbstractEncoderTestCase<ResourceEncoder> {
class ResourceEncoderTests extends AbstractEncoderTests<ResourceEncoder> {

private final byte[] bytes = "foo".getBytes(UTF_8);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
* @author Brian Clozel
* @author Mark Paluch
*/
class StringDecoderTests extends AbstractDecoderTestCase<StringDecoder> {
class StringDecoderTests extends AbstractDecoderTests<StringDecoder> {

private static final ResolvableType TYPE = ResolvableType.forClass(String.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* @since 5.1.3
* @see LeakAwareDataBufferFactory
*/
public abstract class AbstractLeakCheckingTestCase {
public abstract class AbstractLeakCheckingTests {

/**
* The data buffer factory.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
* Implementation of the {@code DataBufferFactory} interface that keeps track of
* memory leaks.
* <p>Useful for unit tests that handle data buffers. Simply inherit from
* {@link AbstractLeakCheckingTestCase} or call {@link #checkForLeaks()} in
* {@link AbstractLeakCheckingTests} or call {@link #checkForLeaks()} in
* a JUnit <em>after</em> method yourself, and any buffers that have not been
* released will result in an {@link AssertionError}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
* @author Arjen Poutsma
* @author Sam Brannen
*/
abstract class AbstractStaxHandlerTestCase {
abstract class AbstractStaxHandlerTests {

private static final String COMPLEX_XML =
"<?xml version='1.0' encoding='UTF-8'?>" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
/**
* @author Arjen Poutsma
*/
abstract class AbstractStaxXMLReaderTestCase {
abstract class AbstractStaxXMLReaderTests {

protected static XMLInputFactory inputFactory;

Expand Down
Loading

0 comments on commit 30cff46

Please sign in to comment.