test: fix forgetting to restore DBDebug value

[kenjis]

Description

• add disableDBDebug() and enableDBDebug() in DatabaseTestTrait
• add missing enableDBDebug()
• refactor

Related #6113

Checklist:

• Securely signed commits
• [] Component(s) with PHPDoc blocks, only if necessary or adds value
• [] Unit testing, with >80% coverage
• [] User guide updated
• Conforms to style guide

[kenjis]

All tests failed except MySQL. 😅

There was 1 error:

1. CodeIgniter\Models\UpdateModelTest::testUpdateWithEntityNoAllowedFields
   ErrorException: pg_query(): Query failed: ERROR: null value in column "email" of relation "db_user" violates not-null constraint
   DETAIL: Failing row contains (5, Jones Martin, null, India, null, null, null).

The failures show changing the behavior according to the DBDebug value is bad practice.

Now in the development environment CI throws Exceptions when DB errors occur,
but in the production environment, CI returns false.
So most devs don't check query return values, and DB errors in the production environment
will be hidden by the configuration.

[MGatner]

Wow I had no idea we did this in so many cases. Very good consolidation.

Something I would like to see in a future version is a way to "schedule" tear-down actions better - so something that knows it will need to be undone can go ahead and put that in place. It would something akin to this:

    protected function enableDBDebug(): void
    {
        $this->setPrivateProperty($this->db, 'DBDebug', true);

        $this->tearDownMethods[] = 'disableDBDebug`;
    }

[MGatner]

the development environment CI throws Exceptions when DB errors occur,
but in the production environment, CI returns false.

This is pretty bad behavior. Does DBDebug control anything else? Or is it just error handling?

[kenjis]

This is pretty bad behavior. Does DBDebug control anything else? Or is it just error handling?

Probably major behavior change is throwing a Exception or returning false.
But we need to investigate.

[MGatner]

For sure. I favor exceptions more than most of the team, so take it with a grain of salt but it seems to me anytime you "return false" to indicate failure and have a hidden error log that should have been an exception.

[kenjis]

I think this should be changed to always throw an exception on this matter.

Since it is a BC, I think it needs to be configurable for exception mode and false mode.

[MGatner]

What about something like Config\Feature::$databaseExceptions that we recommend turning on in 4.3?

[kenjis]

Yes, something like that.