In Examples D7 and some related other contrib, the full suite of tests is being run (all of Drupal core).

We recently changed the branch of D7 Examples from master to 7.x-1.x. I wonder if this is the cause.

Also, in the process (due to my bad advice) the actual release node was deleted.

CommentFileSizeAuthor
#37 pifr.fix_iscoretest_D8_check-1126112-37.patch991 bytesjthorson
#33 pifr.fix_plural_argument_reference-1126112-33.patch843 bytesjthorson
#24 pifr.fix-core-test-detection-1126112-24.patch1.17 KBjthorson
#22 pifr.detect-core-tests-before-allowing-all-1126112-22.patch2.4 KBjthorson
#21 pifr.detect-core-tests-before-allowing-all-1126112-20.patch2.42 KBjthorson
#16 pifr.detect-core-tests-before-allowing-all-1126112-15.patch2.45 KBjthorson
#14 pifr.detect-core-tests-before-allowing-all-1126112-13.patch2.73 KBjthorson
#11 pifr.use-scan-instead-of-syntax-files_for-simpletest-patch-runs-1126112-11.patch1.56 KBjthorson
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Libraries API patches are tested against master, so this doesn't seem to be the cause.

In other words: Same issue, subscribing, and see http://qa.drupal.org/pifr/test/137504

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

It looks to me like this is a #fail related to pifr_client_review_pifr_simpletest::test_list(). It has a number of ways to find the tests it ought to run, but at the end, since the arguments simpletest.tests and drupal.modules are both empty arrays, it just goes with --all on run-test.sh

I have turned on debug on the testbots so we can see the full test delivered to each of these. (It's in the collapsed fieldset at the bottom of the test results)

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

A bit more study and it seems this isn't PIFR... or even PIFT. The project_info module's update of project_info_module table (on drupal.org) hasn't succeeded since January 16, 2011. So all release nodes created since then have this behavior.

mysql> select name,version,n.nid,from_unixtime(created) as created from project_info_module i, project_release_nodes p, node n where p.nid=i.rid and p.nid=n.nid order by n.nid desc limit 10; 
+-----------------------+-------------+---------+---------------------+
| name                  | version     | nid     | created             |
+-----------------------+-------------+---------+---------------------+
| droptor               | 6.x-2.71    | 1029066 | 2011-01-16 19:46:11 | 
| osmplayer             | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
| mediafront_audio      | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
| mediafront_emfield    | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
| mediafront_s3         | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
| mediafront_statistics | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
| mediafront_tagging    | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
| mediafront_user       | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
| favorites_service     | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
| tagging_service       | 6.x-1.0-rc3 | 1029036 | 2011-01-16 19:13:35 | 
+-----------------------+-------------+---------+---------------------+
10 rows in set (0.66 sec)

So pifr.cron.inc tries to get the list of modules to put into the drupal.modules argument, but there's nothing there:

      $item['plugin_argument']['drupal.modules'] = array_keys(project_info_module_list_get($branch->nid));

Now I guess the thing is to find out what triggers an update of the project_info information.

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

Looks to me like the work of populating project_info_module is done by project_info_package_info_process_all() and probably only there. But I can't for the life of me find where that would get called from. (in project_info.package.inc)

Do we have a one-off operation that's supposed to happen here that's not happening any more?

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Oh that's lovely...

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Subscribe, seeing this too with Privatemsg 7.x-2.x.

jthorson’s picture

jthorson CreditAttribution: jthorson commented

Population of project_info_module (in the CVS days) could also be triggered via admin/project/project_info/batch. There's a call to project_info_package_list_store() on line 237 of that file.

Unfortunately, it looks like project_info.batch.inc (and the form at admin/project/project_info/batch) were missed during the Git migration; as they still reference cvs.

In other news ... the entire project_info module appears to be missing from drupal.org; which might explain how it got overlooked.

It also might be nice if someone could go back and grep an old version of d.o code (available via bzr?) for project_info_package_info_process_all, to see if/where it used to be called ...

On the other hand, the only things that appear to reference the project_module_info table are project_info and the pift cron run. Is project_info.module dead code, and is there another table the required info is available?

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

Yeah, we decided to go ahead and try to get #102102: Parse project .info files: present module list and dependency information done... and it's months later. But http://drupal.org/project_dependency is out there waiting for review.

What we need to do is get a test environment set up for it and ask for deployment.

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

Wondering if the strategy in #1231918: Full test being done with manual interface when only the branch in question should be tested can solve this one, even before we get dependencies.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Active » Needs review

Possible fix available with patch in #1231918-20: Full test being done with manual interface when only the branch in question should be tested.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
FileSize
pifr.use-scan-instead-of-syntax-files_for-simpletest-patch-runs-1126112-11.patch1.56 KB

Patch moved over from #1231918: Full test being done with manual interface when only the branch in question should be tested to this issue (where it probably belonged in the first place.)

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

Marked #1169082: Don't run all core tests with every patch as a duplicate.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Needs review » Needs work

Patch addresses one specific scenario, but not all.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
FileSize
pifr.detect-core-tests-before-allowing-all-1126112-13.patch2.73 KB

If it works, this approach should address the most common scenario. (Posting before testing for easy transfer to testbot ... feeling lazy!)

jthorson’s picture

jthorson CreditAttribution: jthorson commented

Patch result: Hung on 'review'. Will investigate after food.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
FileSize
pifr.detect-core-tests-before-allowing-all-1126112-15.patch2.45 KB

New version ... a few fixes, a few less '$this->log' debug lines, and verified to work at http://qa.scratch.drupal.org/pifr/test/136969 .

This was applied on top of the other outstanding patches for the test, but the patch is rolled against 6.x-2.x directly.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Needs work » Needs review
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Curious as to why test runners need chart and multivariate to run SimpleTest?

jthorson’s picture

jthorson CreditAttribution: jthorson commented

Good question ... I've just been pulling those along based on their being included in the canned 'review tests locally' testbot D6 tests ... but haven't actually looked at whether they were required.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

I think that was because of corrupted dependency data - they're not actually dependencies for running tests in D6.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
FileSize
pifr.detect-core-tests-before-allowing-all-1126112-20.patch2.42 KB

Looking back at the original code, I misunderstood it's purpose ... it was testing 'D6 with dependencies' ... not testing 'D6' with required dependencies.

New patch with chart/multivariate removed.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
FileSize
pifr.detect-core-tests-before-allowing-all-1126112-22.patch2.4 KB

Fixed a bad reference reference to $this->vcs[main][repository][type].

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Needs review » Fixed

Patches in #11 and #22 committed to 6.x-2.x (aa4b4685 and 9ed42d0c) as per #1236228-1: August 1st PIFR Testing Results

jthorson’s picture

jthorson CreditAttribution: jthorson commented
FileSize
pifr.fix-core-test-detection-1126112-24.patch1.17 KB

Core test detection wasn't quite working ... this patch is applied on top of #22.

Committed to 6.x-2.x (0267c17).

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented
Status: Fixed » Needs work

Sadly, it looks to me like this one didn't work. I just ran a branch test of Examples D7 (from the qa ui) and it's doing a full test.

jthorson’s picture

jthorson CreditAttribution: jthorson commented

Looks pretty good here ... http://qa.scratch.drupal.org/pifr/test/26934

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

Agreed - it worked fine on scratchtestbot. I don't know why it wouldn't work right on the newly deployed 2.6 testbots unless I just messed something up in the deployment.

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

Here's the same test I just triggered by a commit on qa: http://qa.drupal.org/pifr/test/136969; it ran an enormous number of tests.

It was done on drupaltestbot664; I checked and drupaltestbot664 was in fact updated to 6.x-2.6, and I checked the 6.x-2.6 tag, and it is in fact the latest commit.

jthorson’s picture

jthorson CreditAttribution: jthorson commented

Yup ... running with an --all parameter. :(

Can you pull the $test array from the watchdog log and paste it here? (Or I suppose we could enable debug, and pull it from the log ...)

jthorson’s picture

jthorson CreditAttribution: jthorson commented

Grabbed the test array ... can finally do some debugging.

[17:35:12] Test: Array
(
    [test_id] => 136969
    [review] => Array
        (
            [plugin] => pifr_simpletest
            [argument] => Array
                (
                    [test.title] =>
                    [test.directory.review] => Array
                        (
                            [0] =>
                        )

                    [test.directory.apply] =>
                    [test.directory.dependency] =>
                    [test.files] => Array
                        (
                        )

                    [test.extensions] => Array
                        (
                            [0] => php
                            [1] => inc
                            [2] => install
                            [3] => module
                            [4] => test
                        )

                    [drupal.core.version] => 7
                    [drupal.core.url] => git://git.drupal.org/project/drupal.git
                    [drupal.modules] => Array
                        (
                        )

                    [drupal.user] => admin
                    [drupal.pass] =>
                    [drupal.variables] => Array
                        (
                        )

                    [simpletest.tests] => Array
                        (
                        )

                    [simpletest.db] =>
                )
        )

    [vcs] => Array
        (
            [main] => Array
                (
                    [repository] => Array
                        (
                            [type] => git
                            [url] => git://git.drupal.org/project/drupal.git
                        )

                    [vcs_identifier] => 7.x
                )

            [dependencies] => Array
                (
                    [1] => Array
                        (
                            [repository] => Array
                                (
                                    [type] => git
                                    [url] => git://git.drupal.org/project/examples.git
                                )

                            [vcs_identifier] => 7.x-1.x
                        )

                )

        )

    [files] => Array
        (
        )

)
jthorson’s picture

jthorson CreditAttribution: jthorson commented

Damn ... patch in #24 isn't in the code ... the bad reference to $this->vcs['dependencies'] still exists.

Scratchtestbot, on the other hand, received the proper patch.

Edit: Never mind ... I'm just totally confused.

jthorson’s picture

jthorson CreditAttribution: jthorson commented

Looks like the qa.scratch test has the full suite of modules explicitly set in the $test array, while the qa.d.o version does not have any drupal.modules entries. Thus, they are not taking the same path through the PIFR code. I plan on modifying the qa.scratch database entry to mimic the production $test array, and will troubleshoot from there. (Once I can get qa.scratch working again, that is!)

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Needs work » Needs review
FileSize
pifr.fix_plural_argument_reference-1126112-33.patch843 bytes

The littlest things cause the biggest headaches ...

Probably breaking the rules here, but I've manually patched the d.o testbots (654, 659, and 664) to speed up contrib patch responses until this gets into an 'official' version.

jthorson’s picture

jthorson CreditAttribution: jthorson commented

Examples 7.x-1.x now correctly running 2,031 tests.

http://qa.drupal.org/pifr/test/136969

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented
Status: Needs review » Reviewed & tested by the community

Ah, I see - you patched them on the fly. Well it worked!

This is good to go, IMO.

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

I think we're not quite out of the woods on this yet.

http://qa.drupal.org/pifr/test/136969 is Examples 8.x-1.x and it's currently doing the whole suite.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
FileSize
pifr.fix_iscoretest_D8_check-1126112-37.patch991 bytes

Lol ... just realized the 'switch' condition only covers D7 or D6 ... not D8. Trivial fix. I'll manually adjust the testbots again.

This patch replaces the one in #33.

Perhaps replacing the 'switch' condition with an 'if-else' for the D6 case would be better moving forward.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Reviewed & tested by the community » Fixed

Committed to 6.x-2.x (c4f92ab).

Status: Fixed » Closed (fixed)

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