always return Postgres table names with schema to avoid awfulness

[WattsInABox]

• without the schema, the migrations table gets truncated since the default AR method returns ‘public.schema_migrations’
• also a possibility is truncation of tables other than desired once since without the schema, Postgres has no way of knowing which table to truncate if two schemas have the same table

[JonRowe]

I'd prefer we didn't set an expectation on the internals of the class, but test the desired output state.

[JonRowe]

Thanks for this @BillyWatson, could you possibly tweak the spec like I suggested (roll them together) we don't need to test private methods, and we definitely don't need to partially mock this.

[WattsInABox]

Will do

[JonRowe]

👍

[vrinek]

Running the specs twice makes the new spec to fail. It begins with PG::DuplicateDatabase: ERROR: database "database_cleaner_test" already exists when trying to active_record_pg_setup and fails on the comparison of tables in the cache:

 Failure/Error: connection.database_cleaner_table_cache.should eq(['public.users'])

   expected: ["public.users"]
        got: ["public.precious_stones", "public.users", "public.worthless_junk", "public.replaceable_trifles"]

   (compared using ==)

I think that the problem does not lie with this PR though, it just got surfaced because of it.

UPDATE: I have prepared a fix for this in a local branch. I will wait until this PR has been merged to push my branch and prepare a new PR.

[WattsInABox]

It could be a problem with mine, if I was supposed to trigger some cleanup or yeah, you could be right if the cleanup is supposed to be automagical.

[JonRowe]

We shouldn't do this... It'd be better to figure out a way to test this without tampering with internals.

[WattsInABox]

Updated the diff by changing the test:

connection.database_cleaner_table_cache.first.should match(/^public\./)

[JonRowe]

LGTM