To continue our discussion from before...

On alpha6, any dbtng-driven queries run during simpletests use the normal site prefix, rather than the simpletest prefix. I noticed this when I realized that my tests were failing when my test module was pretending like it couldn't get any data out, and real data from the site itself appeared in the verbose output. Yowza.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marvil07’s picture

The related issue at vcs api queue is #893010: Fix tests (there is a little debug patch that let shows that prefix is not the right one)

sdboyer’s picture

Issue tags: +git phase 2, +git sprint 3
mikey_p’s picture

Assigned: Unassigned » mikey_p
Issue tags: -git phase 2, -git sprint 3

This is probably due to prefixes in D7 being per connection instead of global, which is why things worked previously. I'll try to roll a patch for this tonight.

mikey_p’s picture

Issue tags: +git phase 2, +git sprint 3

Blargh. retagging

mikey_p’s picture

Status: Active » Needs review
FileSize
931 bytes

I don't have any tests that use dbtng (yet), so I'll need someone else to try this.

neclimdul’s picture

applied patch and ran the VersioncontrolAccountStatusTestCase test. Everything passed!

Did get a small exception at the end though:

Exception  Uncaught e database.inc                   1516                      
    The specified database connection is not defined: default

Looks like maybe we're trying to reset to the default database connection at the end of the test but it isn't working correctly.

eliza411’s picture

Status: Needs review » Fixed

I verified with neclimdul that the blocker this issue represents is resolved. I'm going to mark this fixed and move the small exception to its own issue and encourage that testing open tickets for new issues as they arise.

eliza411’s picture

mikey_p’s picture

Status: Fixed » Reviewed & tested by the community

Setting back to RTBC so that I don't forget to commit this. It may be a few days (but should definitely happen before the end of sprint 3) as I'd like to wrap this up in a beta2 release.

eliza411’s picture

Great, thanks. I am getting the hang of process and what status means to module maintainers.

marvil07’s picture

I confirmed that neclimdul could run the versioncontrol tests, but I was not able to reproduce it, I mean make tests run.

He also told me he is using one path on top of its simpletest module: #890440: Backport latest SimpleTest code from D7, which is a patch for the simpletest core patch, so to test it, it's needed to re-apply it. But at the end I could not run successfully the tests.

So, here I am attaching a really tiny test module for dbtng based on the database_test module at D7 test suite. It only try to test a select with dbtng, that is what is used at the end on VersioncontrolEntityController::load(), so if that test pass, the VersioncontrolRepositoryUnitTestCase should be passing.

I am on latest cvs versions of all(core, autoload, dbtng, versioncontrol), so if someone can confirm the tests are running, the problem should be something in my sandbox, please help me to confirm it.

marvil07’s picture

Status: Reviewed & tested by the community » Needs work

Please note that the last patch with the simple dbtng select on a test depends on the patch at #957262: Make sure autoload registry is setup, but after it I still can not see dbtng is working on simpletest.

Changing status, because I can not reproduce test working(hopefully the right thing to do).

mikeryan’s picture

I'm struggling with getting Migrate tests working again with the latest dbtng/autoload, and even with the patch in this issue I'm getting nowhere. It appears to me that dbtng_table_exists() and $connection->schema()->createTable() do not respect the simpletest prefix - they seem to reference the non-prefixed db. The same code works in D7, and this works in D6 when run from a drush command. I can provide more details tomorrow, fading fast...

Thanks.

mikey_p’s picture

Mike I think the issue you are running into may be separate from some of the other issues that we are experiencing. Are you getting this when running a simpletest, or just in testing with a prefixed DB?

mikeryan’s picture

FileSize
1.96 KB
1.89 KB

I don't have a prefixed DB. When running migration commands in drush, table creation and dbtng_table_exists() work fine. In simpletest, createTable() creates table unprefixed, and dbtng_table_exists() matches the unprefixed tables. To demonstrate this, I'm attaching equivalent D7 and D6 tests of createTable() and db[tng]_table_exists(). At least for me, the D7 tests pass, but the D6 tests show the dummy table created without a prefix.

This is with the patch in comment #5. But, since hook_init() isn't called in simpletests, I guess that doesn't matter, and this is just an instance of the basic problem?

neclimdul’s picture

Test passes for me, what is the error?

marvil07’s picture

Status: Needs work » Needs review
Issue tags: +interoperability
FileSize
4.97 KB

After coming to a conclusion on #955996: Exception: The specified database connection is not defined: default I have updated this patch to include a base class which deal with two problems:
- at setUp() call hook_init() manually
- at teardown() remove the default connection to avoid having problems with the prefixes(see the comment at the patch)

I am moving back to "needs review", but note that I am now sure that patch at #5 is RTBC and it's actually a requirement for the patch I am attaching here.

Please note that after making versioncontrol tests inherit from DBTNGTestCase they also work for me finally \o/

PS: I don't really reviewed patches at #15 :-(, so if tests are relevant please include them(but maybe it is a good idea to add the whole testing stuff from D7 at once).

marvil07’s picture

Oopps, a minor comment removing :-p

neclimdul’s picture

Seconding RTBC.
Updating #5 though and replacing the @ with an empty check.

So this patch + #18 = RTBC as far as I'm concerned.

Note: I still needed #957262: Make sure autoload registry is setup to get the test in #18 to pass.

mikey_p’s picture

Issue tags: -git sprint 3 +git sprint 4

tagging for sprint 4

mikey_p’s picture

I just added all the tests from D7. This should greatly help in testing things and making sure that everything works as it should. There are still a number of failures, but most of the tests seem to be passing okay. Currently the only thing required right now is the tests must require autoload and dbtng modules, in the order. For some reason the dependency system doesn't seem to work correctly, or work if the modules are out of order.

All of this code is in the DRUPAL-6--1 branch, and I plan to tag a beta2, or possibly beta3 as soon as it's had some folks using it and can confirm that it works.

mikey_p’s picture

Status: Needs review » Fixed
marvil07’s picture

Status: Fixed » Needs review
FileSize
382 bytes

A minor follow-up about looking at the static class data member self::$databaseInfo instead of the global $databases variable.

marvil07’s picture

Status: Needs review » Fixed

mikey_p just committed this on http://drupal.org/cvs?commit=448674

moving back to fixed

mikey_p’s picture

@mikeryan: FWIW the only way that seems possible to get this to work is to call Database::removeConnection('default') in the tear down method for your tests. While I haven't run into this issue, others have, and this seems to be what simpletest does in D7 to handle prefixing differently for each test case when the prefix info is stored with the DB connection.

mikeryan’s picture

All's well with the latest update from CVS, thanks!

Status: Fixed » Closed (fixed)
Issue tags: -interoperability, -git phase 2, -git sprint 4

Automatically closed -- issue fixed for 2 weeks with no activity.