Erased TestBase.php; refactored unit tests to use proper data providers

[voorproever]

The TestBase class in this library is useless (exception assertion is provided by PHPUnit) and conflicts with the (legacy) TestBase class in our FCP project. PhpStorm is unable to provide correct auto-completion in such case:

Travis will now also execute the test suite on PHP 7.0 and PHP 7.1 (see a4090da).

[davidroth]

@voorproever Exception asserting via attributes can be problematic. For example see this explanation from xunit (a popular .net unit testing library)

Long-term use of [ExpectedException] has uncovered various problems with it. First, it doesn’t specifically say which line of code should throw the exception, which allows subtle and difficult-to-track failures that show up as passing tests. Second, it doesn’t offer the opportunity to fully inspect details of the exception itself, since the handling is outside the normal code flow of the test. Assert.Throws allows you to test a specific set of code for throwing an exception, and returns the exception during success so you can write further asserts against the exception instance itself.

https://xunit.github.io/docs/comparisons.html#note1

See also:

If you are throwing a custom exception, then the chances of the same exception occurring in your code in the wrong place are minimal. However, things change when you are testing for system exceptions.

http://hadihariri.com/2008/10/17/testing-exceptions-with-xunit/

[davidroth]

@voorproever I am ok with these changes although personally I`d avoid the attribute approach.

So no need for changes ... just a general remark ;)

[voorproever]

I (partially) agree with your first statement. However, there is no difference between the annotation and the old wrapper method. No unit test used the exception object afterwards. If I want to make sure that the correct exception is thrown during testing I would additionally use @expectedExceptionMessage; example from FCP project:

/**
 * @covers ModuleSetup::createFromDatabaseVersion
 *
 * @expectedException \LogicException
 * @expectedExceptionMessage Invalid config file for "johndoe" module.
 */
public function testCreateFromDatabaseVersion_corruptedModuleConfigFile()
{
    // "johndoe" module does not exist.

    ModuleSetup::createFromDatabaseVersion(
        "johndoe",
        \Core::getDB()->getConnection()
    );
}

A lot of unit tests in fusonic/linq could use @expectedExceptionMessage to fully eliminate the possibility of incorrect asserts, but I just wanted to get rid of the TestBase class 😉

[davidroth]

Yeah sure, narrowing it down further with exception message can make sense ;)
There is no black and white here ... just different approaches with pros&cons

Thx for the PR by the way ;)