Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

Wanted (Maybe?): New Maintainer #217

Open
sebastianbergmann opened this issue Oct 2, 2018 · 9 comments
Open

Wanted (Maybe?): New Maintainer #217

sebastianbergmann opened this issue Oct 2, 2018 · 9 comments

Comments

@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Oct 2, 2018

A while ago I visited @beberlei at the offices of his company @tideways. Their solution for PHP application performance monitoring is the best I have seen so far and you should definitely check it out if you have not done so already.

It should come as no surprise that @tideways is committed to Open Source, @beberlei was the project lead for @doctrine2 after all. Their new tideways_xhprof extension, which started as a fork of Facebook's XHProf extension, provides a modern solution for profiling PHP applications.

An interesting topic that came up in our discussion was DbUnit. Back in 2007, @mlively ported JUnit's DbUnit extension to PHP and PHPUnit. This extension for PHPUnit provides ready-to-be-used implementations of best practices for testing database interactions. It helps with fixture management and loading test data into the database as well as cleaning it up between tests. And the assertions it provides help with verifying the state of the database after INSERT, UPDATE, or DELETE operations.

@beberlei shared that, from his experience, it is usually easier to implement a custom, project-specific solution for database fixture management than to use the generic solution provided by DbUnit. The effort to build such a custom solution is usually quite low, resulting in less than two hundred lines of code in a trait that is used by the database test case classes. He continued that when the application under test uses Doctrine then that aforementioned trait requires even less code thanks to functionality provided by the Connection object.

At first, I was surprised that @beberlei does not use DbUnit. But then I realized that over the last couple of years I suggested the use of DbUnit less and less, for the most part because of the same reasons that @beberlei mentioned. In my experience, too, fixture management is usually much easier to maintain over time when it is implemented with the project's specifics in mind.

@mlively stopped maintaining DbUnit years ago. After a while, @elazar took over the maintenance of DbUnit but he, too, is no longer active. In recent years, I made the changes required to keep DbUnit compatible with newer versions of PHPUnit. Occasionally I merge pull request to fix bugs. These pull requests, along with a bug report now and then, are the only signs I see that DbUnit is actually being used.

A big concern that I have with DbUnit is the implementation of its assertions. For instance, the output they provide in case of a failure is frequently hard to read.

(Somehow the above reads like it should have been a blog post.)

Given that I neither use nor recommend the use of DbUnit, I am no longer motivated to perform even the minimal maintainenance work I have done these past few years. I will neither cut a release that will be compatible with next year's PHPUnit 8 nor will I clean up and refactor the codebase to leverage modern PHPUnit extension points.

I will eventually archive this repository (no later than February 2019). Feel free to fork it in case you want to maintain DbUnit in the future.

@staabm
Copy link

staabm commented Oct 2, 2018

maybe you should put/link this write-up from the projects README to be more prominent.

@bartmcleod
Copy link

I only recently re-discovered DbUnit and I was thrilled to have a loading mechanism for datasets that works out of the box. However, my new usage followed a number of discussions in our team on whether or not to write our own implementation and some attempts were made to load datasets our own way. In my opinion, even 200 lines of code you have written yourself as opposed to using something off the shelve is really bad, because it adds maintenance to your own project. On the other hand, nowadays, data comes from many places, not only from a database, so you will always need to write a few other implementations, for other datasources alongside what you do with DbUnit. That said, I will regret having to adopt yet another implementation when DbUnit is going to be discontinued, but if it is inevitable, we will deal with it.

@Ocramius
Copy link

Ocramius commented Oct 2, 2018

As discussed with @sebastianbergmann in private, I don't think I've ever used DbUnit directly:

  1. tests that require a DB connection usually have their own application-specific fixture mechanism, and often rely on running SQL files directly against a running DB server
  2. better tooling based on nelmio/alice and doctrine/data-fixtures are a better fit for ORM-specific applications

@weierophinney
Copy link

Per an email discussion with @sebastianbergmann , I noted the following: We do not use DBUnit within ZF.

zend-db provides a DBAL, and, as such, developers using zend-db can mock the interfaces we provide. Within the component itself, we are able to mock the connection interfaces when testing features such as statement preparation, SQL generation, etc. We then write integration tests for the connection adapters themselves, using vanilla PHPUnit as at that point, we're not mocking the database interactions.

My experience has been that using a DBAL obviates the need for using DBUnit; the only time I've used it to any effect is when I was directly using PDO and wanted to test that class. It was far harder than using a DBAL in the first place.

@mjmunger
Copy link

mjmunger commented Oct 2, 2018

Having looked at it, it does seem trivial to setup.

Someone please correct me if I am wrong. PHPUnit\DbUnit\TestCastTrait overrides PHPUnit\Framework\TestCase::setUp() and PHPUnit\Framework\TestCase::tearDown() (among others).

To write your own DB fixture for exclusive use with MySQL, to import a known dataset, which was exported with mysqldump to a SQL file, we could do something like this:

protected function setUp(): void
{
    parent::setUp();
    exec("mysql --defaults-file=/path/to/creds.cnf $database < /path/to/fixture/dump.sql");
}

Am I missing something here?

@bartmcleod
Copy link

@mjmunger I think you are missing that @sebastianbergmann is mostly looking for a new maintainer or group of maintainers and/or whether to discontinue the project for lack of interest. I use DbUnit because it allows me to enforce a standardized way to setup a connection and load standardized datasets. That said, I would volunteer to be part of a new group of maintainers, but not the sole maintainer. @weierophinney One only uses DbUnit when mocking is not an option. To test for example of filters in a query are implemented correctly, a mock would not do, because a mock gives one a predfined answer. In that case you would want integrationtests that can be based on DbUnit.

@mjmunger
Copy link

mjmunger commented Oct 3, 2018

What I got from the post was "I don't use it. I don't recommend people use it. People I respect don't use it, so I'm not going to devote time to it anymore."

That's not exactly a resounding recommendation for someone to devote time to it. That's a recommendation to forgo DbUnit and start writing the 200 lines of code for your next project(s) and do your own database mocking. @sebastianbergmann is free to correct me here, but that was my take-away from the post.

But, honestly, if the maintainer of phpUnit doesn't use or recommend DbUnit to his clients, I need to change the way I am doing things.

We rely heavily on it for database testing (in addition to Mocking objects). DbUnit is very useful for finding breaking schema changes, for example.

The one thing that I have always disliked about DbUnit is that it does not have the ability to suspend foreign key checks when importing data. We got around that by writing a script that analyzes the tables, and then determines the order in which mysqldump should dump the files so that when DbUnit reads and imports the data, it will do it in a way the supports foreign keys. So, if I can do a simple implementation that runs exec() and mysqldump, that problem is forever solved. (I did a simple test yesterday, and it's not going to be that simple. Yes exec'ing mysql works, obviously, but getting a PDO object returned with the interface that phpUnit needs to work with it seems to be more involved).

But, if DbUnit is not going to have any support in future versions of phpUnit, we have to make a change in how we do things almost immediately.

So, while I am not going to throw my hat in the ring to maintain it, I would be happy to help write tutorials on how to roll your own... I would appreciate some pointers to get me moving in the correct direction instead of having to pour over the code looking for what needs to be done...

@LastDragon-ru
Copy link
Contributor

LastDragon-ru commented Oct 26, 2018

The one thing that I have always disliked about DbUnit is that it does not have the ability to suspend foreign key checks when importing data.

Code for the previous/old version (but it should work in current with minimal adaptation)

class App_Tests_Helpers_PHPUnit_Operation_ForeignKeyChecksDisable
        implements PHPUnit_Extensions_Database_Operation_IDatabaseOperation {
    /**
     * (non-PHPdoc)
     * @see PHPUnit_Extensions_Database_Operation_IDatabaseOperation::execute()
     */
    public function execute(
            PHPUnit_Extensions_Database_DB_IDatabaseConnection $connection,
            PHPUnit_Extensions_Database_DataSet_IDataSet $dataSet) {
        $query = 'SET foreign_key_checks=0;';

        try {
            $connection->getConnection()->query($query);
        } catch (PDOException $e) {
            throw new PHPUnit_Extensions_Database_Operation_Exception('ForeignKeyChecksDisable', $query, array(), null, $e->getMessage());
        }
    }
}

class App_Tests_Helpers_PHPUnit_Operation_ForeignKeyChecksEnable
        implements PHPUnit_Extensions_Database_Operation_IDatabaseOperation {
    /**
     * (non-PHPdoc)
     * @see PHPUnit_Extensions_Database_Operation_IDatabaseOperation::execute()
     */
    public function execute(
            PHPUnit_Extensions_Database_DB_IDatabaseConnection $connection,
            PHPUnit_Extensions_Database_DataSet_IDataSet $dataSet) {
        $query = 'SET foreign_key_checks=1;';

        try {
            $connection->getConnection()->query($query);
        } catch (PDOException $e) {
            throw new PHPUnit_Extensions_Database_Operation_Exception('ForeignKeyChecksEnable', $query, array(), null, $e->getMessage());
        }
    }
}

class TestCase {
    protected function getSetUpOperation() {
        return new PHPUnit_Extensions_Database_Operation_Composite(array(
            new App_Tests_Helpers_PHPUnit_Operation_ForeignKeyChecksDisable(),
            PHPUnit_Extensions_Database_Operation_Factory::TRUNCATE(),
            PHPUnit_Extensions_Database_Operation_Factory::INSERT(),
            new App_Tests_Helpers_PHPUnit_Operation_ForeignKeyChecksEnable(),
            ));
    }
}

@LastDragon-ru
Copy link
Contributor

LastDragon-ru commented Oct 26, 2018

two hundred lines of code

This is good if you want to test several objects, but when you want to test several tables (probably this is more functional testing than unit testing?) the dbunit very useful, BUT too many important features are missed

  1. foreign_key_checks - I saw this bug in this tracker a maybe 5 years ago...
  2. No JsonDataSet, it may be useful sometimes, and, much more critical, no RawSqlDataSet - instead of just dump sql and use it, you must spend an hour or so to adapt your data for dbunit.
  3. Seems no way to ignore columns in assertDataSetsEqual. For example, laravel updates updated_at while model saving, and datasets will be always different
  4. Loading dataset before tests is very strange and very unuseful design. I never use it. Much more common task - load dataset inside test (seems I saw issue for this, but not sure), and rollback data after.
  5. Failure output is really hard to read. Probably it should dump only PKs and differences (= if columns equals they must be skipped).
  6. No other libraries support (Symfony, Laravel, etc) - each time we must write an adapter by hand.

etc

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants