Ticket #1483 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

Ability to run unit tests with PostgreSQL

Reported by: ajmixto Assigned to: demian
Priority: normal Milestone: 0.6.3
Component: not categorised Severity: need feedback
Keywords: Cc:

Description (Last modified by demian)

Currently, unit tests only work with MySQL, but we need to make it work with PostgreSQL.

Attachments

0.6-bugfix.diff (7.3 kB) - added by ajmixto on 07/04/07 07:47:23.
Patch from sgl root
modules.diff (2.2 kB) - added by ajmixto on 07/04/07 07:47:38.
Patch from modules directory
parche_completo.diff (21.5 kB) - added by ajmixto on 07/05/07 10:02:20.

Change History

07/04/07 07:47:23 changed by ajmixto

  • attachment 0.6-bugfix.diff added.

Patch from sgl root

07/04/07 07:47:38 changed by ajmixto

  • attachment modules.diff added.

Patch from modules directory

07/05/07 03:55:07 changed by ajmixto

  • owner changed from ajmixto to demian.
  • severity changed from open to need feedback.

Hi folks,

I've been working to make the unit tests to run with PostgreSQL and finally it appears to be solved, but I couldn't try to make my patch work with MySQL.

Could you try it, please, and give me some feedback about.

07/05/07 04:54:22 changed by demian

  • owner changed from demian to ajmixto.

Hola AJ

just had a quick look at your patch, I think it could be slightly improved keeping these 2 things in mind:

  • avoid duplication, a lot of your code duplicates what's done in SGL_DB::getDsn(), pls patch this method if it's missing sthing
  • factor out pg/mysql db creation/deletion differences by creating a method called SGL_Sql::buildDbCreateStatement($driver, $dbname) or similar

Rest of changes are good :-)

07/05/07 07:40:34 changed by ajmixto

  • owner changed from ajmixto to demian.

Hola Demian

I made the changes you suggested me, please, check them.

A dude: At tests/classes/DB.php#93 there is a function called getDsn() wich duplicates code from SGL_DB::getDsn(), but I don't know if this file should depend from lib/SGL/DB.php.

What do you think?

07/05/07 08:11:03 changed by demian

  • owner changed from demian to ajmixto.
  • description changed.

good point, just have STR_DB extend SGL_DB, deps are fine here as test runner is not intended to be standalone package.

Other points:

  • regarding mysql/pg requirements, i would follow standard sgl convention and rename tests/test.conf.ini-dist to tests/test.my.conf.ini-dist and create a pg one.
  • regarding: getDsnStringFromConf(), your refactoring looks like this:
if ($type == SGL_DSN_ARRAY)
    // keep code the same
else
    // use new method to build SGL_DSN_STRING

to me it seems either leave the code as it is, or be more consistent and refactor both conditions. If you choose the latter, I'd rename to _getDsnAsString(). 'fromConf' imo is implicit since I don't know where else you would get it. The rest of the patch looks good, pls make changes and I'll test against mysql.

07/05/07 10:02:20 changed by ajmixto

  • attachment parche_completo.diff added.

07/05/07 10:12:27 changed by ajmixto

  • owner changed from ajmixto to demian.

Ok, it's finished... the only problem is that when you visit the test URL and the test.conf.ini-dist files is copied to var dir, the SGL environment it's not initialized so I can't check if you have MySQL or PostgreSQL, so I decided to copy automatically the mysql config file.

I'll write a note at Howto/DB/PostgresSQL and at Standards/UnitTesting to copy test.pg.conf.ini-dist manually.

07/09/07 05:14:21 changed by demian

  • status changed from new to closed.
  • resolution set to fixed.

Nice work dude, that's committed now. Small note on CS, please use spaces after commas for args, ie

GOOD: foo($bar, $baz) BAD : foo($foo,$baz)