Replace mocks with middlewares

[derrabus]

This PR improves a test by replacing a mock of DBAL's Connection class by a middleware. This should make the test resilient against future DBAL changes and ease the midgration path towards DBAL 4.

[morozov]

This still looks like too much code to mock a single value returned by Connection::lastInsertId(). Would it make sense to use PHPUnit mocks for that? Or the test uses other Connection APIs for which we want to use the default implementation?

[derrabus]

Yes, the test persists entities in a real database. What we want to test is if the ORM handles very large IDs returned as string correctly. If you have an idea to simplify this partial mock setup, I'm all ears.

[morozov]

It is indeed a functional test. In this case, it shouldn't use mocks. It should reproduce a real case instead. BTW, what is the corresponding code change that it covers?

For instance, initialize the autoincrement field with PHP_INT_MAX and insert a new row.

Alternatively, since it's not a real functional test, it could be converted into a unit one and cover only the corresponding component w/o mocking the whole world.

[derrabus]

It is indeed a functional test. In this case, it shouldn't use mocks. It should reproduce a real case instead. BTW, what is the corresponding code change that it covers?

That would be this seven years old PR: #1346.

For instance, initialize the autoincrement field with PHP_INT_MAX and insert a new row.

Does the schema manager give me a nice way to do that?

Alternatively, since it's not a real functional test, it could be converted into a unit one and cover only the corresponding component w/o mocking the whole world.

Yes, a unit test would not be a bad idea, however since we need to make sure that UoW and persisters talk to each other correctly, I would not want to remove the functional test either way.

[morozov]

Does the schema manager give me a nice way to do that?

I don't think so, since there's no portable API for that.

Yes, a unit test would not be a bad idea, however since we need to make sure that UoW and persisters talk to each other correctly, I would not want to remove the functional test either way.

This test doesn't cover a significant part for the functionality. Specifically, the driver returning a bigint. As we learned recently, SQLite returns integers as integers. Mocking this anything in an integration test is a bit naive IMO.

FWIW, I don't mind these changes merged in any state. But IMO the test in question causes more trouble than produces value. If it was implemented properly, it wouldn't have to be updated in the first place.