Fix failing unit tests on OracleDB

[DeepDiver1975]

fixes #2250

getDropAutoincrementSql cannot take a Table object - only string is allowed

@Ocramius here we go - alternative approach for #2251

[Ocramius]

@DeepDiver1975 by callers I mean those who call this method: a string should be given.

[DeepDiver1975]

@DeepDiver1975 by callers I mean those who call this method: a string should be given.

As in kill this unit test? 😉

https://github.com/doctrine/dbal/blob/master/tests/Doctrine/Tests/DBAL/Functional/WriteTest.php#L260-L265

[deeky666]

Yeah the thing is, our API between schema manager and platform is not strict enough. Schema manager only allows string while platform allows string|Table. If we would be strict, the WriteTest had to be fixed. Though, I don't know how people are using the schema manager, whether they are "used" to passing objects and Doctrine not complaining...
IMO to be consistent with other getDrop*() methods, OraclePlatform::getDropAutoincrementSql() should also allow Table instances, but I am not sure whether we should widen the signature there.
Otherwise I would rather fix the test that misuses SchemaManager::dropTable() by passing a table object there.
@Ocramius thoughts?

[Ocramius]

I don't know how people are using the schema manager, whether they are "used" to passing objects and Doctrine not complaining...

@deeky666 if we state string there (and there seems to be no mistake about it there), then consumers passing in a Table shouldn't be allowed to do so.

[DeepDiver1975]

@Ocramius @deeky666 where are we going from here? I'd like to get this fixed.

Just let me know in which direction we want to go - I'm happy to take care about further development.

[DeepDiver1975]

@Ocramius @deeky666 where are we going from here? I'd like to get this fixed.

Just let me know in which direction we want to go - I'm happy to take care about further development.

ping

[deeky666]

@DeepDiver1975 as confirmed by @Ocramius table objects are not allowed to be passed to dropTable. Therefore we should rather fix the WriteTest::testEmptyIdentityInsert() test case IMO.

[DeepDiver1975]

Therefore we should rather fix the WriteTest::testEmptyIdentityInsert() test case IMO.

Okay - please expect a PR by the end of the week.

[deeky666]

@DeepDiver1975 thanks, just poke me again then. Btw you can also commit to this PR and then squash commits. No need to open a third PR.

[DeepDiver1975]

@DeepDiver1975 thanks, just poke me again then. Btw you can also commit to this PR and then squash commits. No need to open a third PR.

@deeky666 done - at least this one test now passes ...

[deeky666]

@DeepDiver1975 awesome thanks!