Discussion on replacing the StateChecker with database truncation in tests

[connorshea]

I was hoping to start a discussion on the current StateChecker used for PHPUnit tests and whether we can replace it with a different solution.

I realize there's been a decent investment of time and energy into the feature, but I think it'd be best if we were to move from the StateChecker to truncating the database instead.

There are a few reasons I think it'd be beneficial to do this:

• It's quite a complex solution to the problem, I've spent a while reading the docs and trying to understand it, but I'm still honestly not 100% sure how it works/how to use it.
• Because of the complexity, it's harder for new contributors to write tests.
• The StateChecker can cause tests to pass inconsistently, depending on whether it's enabled or not.
• When you run a test and it fails to clean up after itself, it can be really hard to get back to the state where things started. This makes developing tests that mess with database records or files pretty difficult and error-prone.
• The StateChecker causes the tests to be much slower than they'd otherwise be, so people rarely ever actually run the tests themselves.
• Truncating the database leads to more consistent and clearer tests, because all dependencies and records that need to be created are done so explicitly, and it's generally what's done by most other test suites I've worked with (Laravel does this, for example).

The biggest problems I foresee from making this change would be:

• The time investment for moving to database truncation.
• The existing tests being reliant on certain records from other tests being existent.
• Potentially slower tests, though I'm skeptical as to whether this would actually be slower than the existing system.
• Files and global variables - and potentially other state I'm not thinking of - wouldn't be handled by the database truncation. We'd need to replace the file state checking with something else, or I guess leave part of the statechecker intact to handle that aspect. For global variables at least, this can be handled by PHPUnit itself.
• Truncating the database can cause problems due to the fact that the CRM needs to be installed before it can be used for most of these tests. However, it should be possible to use Fixtures to get to a 'known-good' starting state between each the test.

There may be things I don't know about the statechecker that are causing me to misunderstand things, and I'd be happy to be corrected if anything I said here was incorrect, but I hope this at least creates an interesting discussion to help us improve the test suite going forward.

Thanks :)

[Abuelodelanada]

Hi @connorshea

When you said:

However, it should be possible to use Fixtures to get to a 'known-good' starting state between each the test.

Are you thinking in using something like Faker PHP Library?

[connorshea]

@Abuelodelanada not necessarily, we could have fixtures without Faker and instead hardcode values (though that can be a problem if people start relying on the exact contents of the fixtures, then they become really difficult to change).

That said, I'd definitely like to have the tests run from a programmatically-created database state rather than the current SQL dump. e.g. creating the same records we do with the dump, but via PHP so it's easier to read and change. I'm not sure how much overhead that'd have over just loading the dump, though.

[connorshea]

This is now resolved in the next release! ❤️