tests for sqilte: enable FOREIGN_KEYS inside OpenTestConnection

[glebarez]

• Do only one thing
• Non breaking API changes
• Tested

What did this pull request do?

As of now, the Foreign Key constraint is OFF by default in sqlite3.
In GORM tests package the constraint is enabled by running following code:

gorm/tests/tests_test.go

Lines 46 to 48 in 6bef318

	if DB.Dialector.Name() == "sqlite" {
	DB.Exec("PRAGMA foreign_keys = ON")
	}

for the global DB instance.

The problem is that this code is executed once inside init, thus leaving other calls to OpenTestConnection not executing the same logic.

The code should be moved inside OpenTestConnection, since some tests do not use the global DB instance, preferring to call OpenTestConnection for their own purpose.

The issue was solved the wrong way in c10f807 by changing the connection string to "gorm.db?_foreign_keys=on", which is a syntax, specific to mattn's CGo driver, and caused regression in CGo-free driver.

This PR addresses the issue in a unified way, compatible with all sqlite drivers for GORM.

User Case Description

[glebarez]

@jinzhu it seems like tests are failing unrelated to this fix. Can you please re-run them?

[ncruces]

Are you sure this works? sqlite.Open returns a pool of connections, gorm.Open wraps that pool. Calling db.Exec will enable foreign keys on the single connection it runs on, not for the entire pool. Right?

[glebarez]

Are you sure this works? sqlite.Open returns a pool of connections, gorm.Open wraps that pool. Calling db.Exec will enable foreign keys on the single connection it runs on, not for the entire pool. Right?

Nice notice. Agreed.

[ncruces]

Tbh, I don't know of a good way to solve this across drivers, which is why I think go-gorm/sqlite#161 is a bad idea.

[ncruces]

One option is for the GORM drivers to enable this on their own regardless of the DSN, like:
https://github.com/ncruces/go-sqlite3/blob/90628ab8aa4e2d2919edd27afcd74ddfcf530e22/gormlite/sqlite.go#L43-L45

[saeidee]

I am still not sure why we changed this at all, gorm/sqlite driver is using mattn's CGo driver by default, for one of the gorm functionalities(TranslateError) we wrote a test and we want to run the tests based on the default driver to get it green, not sure why we should really think to make changes in test part if we use the mattn CGo driver by default in gorm/sqlite.

Note: This test was written for TranslateError functionality and for this functionality we don't depend on any driver(we use JSON decoding), however, to just test and verify the functionality we go for mattn CGo driver.

CC: @a631807682

[ncruces]

If foreign keys should be on by default, why doesn't each GORM driver (github.com/go-gorm/sqlite, github.com/glebarez/sqlite, github.com/ncruces/go-sqlite3/gormlite, etc) enable them as appropriate?

[saeidee]

-> foreign keys should be on by default

This is enabled only in testing, and it is for specific test cases that are relaying on github.com/go-gorm/sqlite -> github.com/mattn/go-sqlite3. It is nothing to do with Gorm functionality, it is used only in test files.

[a631807682]

As far as I know, foreign_keys only applies to the limitations of SQLite database when using a single connection. We use it to test the association between SQLite tables. This does not mean that it is necessary (even we will not use it in most cases). It can be decided by the user.
The purpose of test cases is to ensure that the drivers provided by gorm can work properly. If other drivers exist, they need special test cases to ensure that they work.
So I don't understand what impact this test case change will have.