alexruiz / fest-assert-2.x

FEST Fluent Assertions 2.x
http://fest.easytesting.org
Apache License 2.0
401 stars 69 forks source link

tests : org.fest.assertions.api contain too much classes #82

Closed joel-costigliola closed 12 years ago

joel-costigliola commented 12 years ago

Having a file by method to test is fine, the drawback is having too many files in one package which is just not handy when developing (some contributors from Paris hackergarten complained about that and I agree with them) . For example org.fest.assertions.api contains 511 test files !

We could improve this by grouping all tests related to a class (let's say StringAssert) to a subpackage with the class under test name. As en example all 17 StringAssert_*_Test files would be moved into org.fest.assertions.api.string.

It will entail making some method public to be visible for testing (or find another testing strategy).

olim7t commented 12 years ago

One way to preserve encapsulation is to leave an abstract parent test (that serves as a gateway to package-private fields) in the main package, and have the actual tests in the subpackages inherit from it.

This could also be an opportunity to factor some common code in this parent class.

olim7t commented 12 years ago

For instance:

package org.fest.assertions.api; // main package

public abstract class StringAssertTest {
  protected Strings strings; // made visible for subclasses
  protected StringAssert assertions;

  @Before public void setUp() {
    strings = mock(String.class);
    assertions = new StringAssert("Yoda");
    assertions.strings = strings;
  }

  @Test public void should_delegate_to_internal_class() {
    invoke_assertion();
    verify_internal_class_was_invoked();
  }

  @Test public void should_return_this() {
    StringAssert returned = invoke_assertion();
    assertSame(assertions, returned);
  }

  protected abstract StringAssert invoke_assertion();
  protected abstract void verify_internal_class_was_invoked();
}
package org.fest.assertions.api.string; // subpackage

public class StringAssert_contains_String_Test extends StringAssertTest {
  @Override protected StringAssert invoke_assertion() {
    return assertions.contains("od");
  }
  @Override protected void verify_internal_class_was_invoked() {
    verify(strings).assertContains(assertions.info, assertions.actual, "od");
  }
}
joel-costigliola commented 12 years ago

@olim7t love that solution ! Not only visibility is not loosened but it' an oportunity to eliminate duplicated code in tests (which I started to do few months ago in org.fest.assertions.internal package).

It's a not a difficult task but a long a not very exciting one, that's why I'm splitting this tasks in two :

If you don't want to do all the org.fest.assertions.api work, just mention what you did and I'll finish it.

olim7t commented 12 years ago

I've started work on the api package. I've pushed a temporary branch on my fork if you want to see what it looks like.

joel-costigliola commented 12 years ago

Olivier thanks for awesome work! Things are really much cleaner.