PSR-0 tests in contrib modules are not found [#1674290]

Problem

Tests of contributed modules for D8 using the PSR-0 namespace/directory structure are not found (and thus not executed) at all.

Details

E.g., Link 8.x-1.x: http://qa.drupal.org/pifr/test/297448 review log says:

Command [php -l -f './sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldCRUDTest.php' 2>&1] succeeded.
...

Command [/usr/bin/php ./core/scripts/run-tests.sh --concurrency 8 --php /usr/bin/php --url 'http://drupaltestbot654-mysql/checkout' --file  --clean 2>&1] succeeded.
No Simpletest tests found.  Skipping execution of run_tests.sh.

Command [/usr/bin/php ./core/scripts/run-tests.sh --php /usr/bin/php --url 'http://drupaltestbot654-mysql/checkout' --file  --list 2>&1] succeeded.
Review complete. test_id=297448 result code=10 details=Array

@dawehner apparently reported the identical bug to me some weeks ago (regarding Views for D8).

Comment	File	Size	Author
#13	project_issue_file_review.module.13.patch	2.32 KB	sun
#11	pifr.psr0_.11.patch	4.94 KB	jthorson
#3	project_issue_file_review.psr0_.3.patch	4.83 KB	sun
#3	interdiff.txt	2.59 KB	sun
#2	project_issue_file_review.psr0_.2.patch	5.69 KB	sun

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jthorson CreditAttribution: jthorson commented 7 July 2012 at 15:17

The current test detection algorithm is pretty crude ... IIRC, it looks for a ./tests directory off the root, and crawls through the module tree looking for any .test files.

Since the tests no longer have the extension .test, they are not picked up.

We'll need some new algorithm that can help us detect which .php files are tests and which aren't ... simply bringing in every file under every 'Tests' directory is probably not all that robust but could be a start.

Comment #2

sun

German

Karlsruhe

CreditAttribution: sun commented 7 July 2012 at 17:12

Status:

Active

» Needs review

File	Size
project_issue_file_review.psr0_.2.patch	5.69 KB

I'm confident that this patch should do it.

That said, I'm also relatively sure that I have a typo somewhere (as usual)... ;) I don't have a real PIFR stack locally, just a checkout of project_issue_file_review.

Comment #3

sun

German

Karlsruhe

CreditAttribution: sun commented 7 July 2012 at 19:39

File	Size
interdiff.txt	2.59 KB
project_issue_file_review.psr0_.3.patch	4.83 KB

If we do not want to hard-code that many assumptions about extensions and their PSR-0 namespace directory paths, then we could also take a simpler route.

Essentially, checking for the minimal conditions only; i.e., the file extension must be .php and the file path must literally contain "/Tests/".

This has the potential of false-positives. But right now, I cannot think of any.

I'm suggesting this alternative approach, because it is possible that we're going to change the directory structure for extensions and PSR-0 files in the future. The chance for that to happen might be larger than the chance for a false-positive.

Thus, providing this as an alternative/simplified solution.

Comment #4

rfay

English

Palisade, CO, USA

CreditAttribution: rfay commented 7 July 2012 at 19:46

I'm not actually sure that false positives do any damage. Could be wrong.

Comment #5

sun

German

Karlsruhe

CreditAttribution: sun commented 7 July 2012 at 20:03

That's possible, too. ;)

Can we apply this patch on a PIFR staging/test server or similar?

I'd love to resolve this issue ASAP, since it hinders a couple of core initiatives/efforts...

Comment #6

jthorson CreditAttribution: jthorson commented 10 July 2012 at 06:10

Deployed on scratchtestbot, linked to qa.staging.devdrupal.org, and passed testbot confirmation.

I just now kicked off a 8.x branch test on qa.staging to see if it passes. Once that's complete, we should probably also kick off a branch test for a project containing PSR-0 tests, and see whether they're picked up.

I'm heading to bed before the core branch test completes, but anyone with access can attempt a contrib branch test on qa.staging if you want a result before I get back to it.

Comment #7

sun

German

Karlsruhe

CreditAttribution: sun commented 10 July 2012 at 19:08

If you need a contrib project with D8 code + tests: Link: http://qa.drupal.org/pifr/test/297448

Comment #8

jthorson CreditAttribution: jthorson commented 14 July 2012 at 06:09

Appears to deliver as advertised ...

[15:27:16] Checked [12 of 12] relevant file(s) for syntax.
[15:27:16] Invoking operation [install]...
[15:27:27] Invoking operation [review]...
[15:27:30] Scanning sites/default/files/checkout/sites/default/modules/link for .test files.
[15:27:30] Scanning sites/default/files/checkout/sites/default/modules/link for /Tests/*.php files.
[15:27:30] Cleaning run-tests.sh environment.
[15:27:31] Command [/usr/bin/php ./core/scripts/run-tests.sh --concurrency 8 --php /usr/bin/php --url 'http://scratchtestbot-mysql.osuosl.test/checkout' --file sites/default/modules/link/lib/Drupal/link/Tests/LinkTokenTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldCRUDTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldValidationTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkTestBase.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldFormatterTest.php --clean 2>&1] succeeded.
[15:27:31] Initiating run-tests.sh.
[15:29:16] Command [/usr/bin/php ./core/scripts/run-tests.sh --concurrency 8 --php /usr/bin/php --url 'http://scratchtestbot-mysql.osuosl.test/checkout' --file sites/default/modules/link/lib/Drupal/link/Tests/LinkTokenTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldCRUDTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldValidationTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkTestBase.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldFormatterTest.php 2>&1] succeeded
  Duration: 105 seconds
  Directory: [/var/lib/drupaltestbot/sites/default/files/checkout]
  Completion status: [0]
  Output: [Drupal test run
---------------

Tests to be run:
 - Link CRUD - browser test (Drupal\link\Tests\LinkFieldCRUDTest)
 - Link Attribute Tests (Drupal\link\Tests\LinkFieldFormatterTest)
 - Link Validation Tests (Drupal\link\Tests\LinkFieldValidationTest)

Test run started:
 Friday, July 13, 2012 - 22:27

Test summary
------------

Link CRUD - browser test 291 passes, 3 fails, and 0 exceptions
Link Validation Tests 445 passes, 22 fails, and 10 exceptions
Link Attribute Tests 772 passes, 336 fails, and 3 exceptions

Test run duration: 1 min 43 sec].
[15:29:16] Command [/usr/bin/php ./core/scripts/run-tests.sh --php /usr/bin/php --url 'http://scratchtestbot-mysql.osuosl.test/checkout' --file sites/default/modules/link/lib/Drupal/link/Tests/LinkTokenTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldCRUDTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldValidationTest.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkTestBase.php,sites/default/modules/link/lib/Drupal/link/Tests/LinkFieldFormatterTest.php --list 2>&1] succeeded.
[15:29:16] Review complete. test_id=178423 result code=9 details=Array
(
    [@pass] => 1508
    [@fail] => 361
    [@exception] => 13
    [@debug] => 0
)

[15:29:16] Static variables reset.

Now running it through the set below, to check for unexpected impacts.

Sanity checks complete:
- D8 core branch test
- D7 core branch test
- D6 core branch test
- D7 contrib branch test
- D6 contrib branch test
- D8 core patch test

Sanity checks todo:
- Non PSR-0 D8 contrib branch test
- D8 contrib patch test
- D7 core patch test
- D7 contrib patch test
- D6 core patch test
- D6 contrib patch test

Comment #9

jthorson CreditAttribution: jthorson commented 13 July 2012 at 23:05

Duplicate of http://drupal.org/node/1642478 ... just need to determine which patch makes more sense.

Comment #10

sun

German

Karlsruhe

CreditAttribution: sun commented 14 July 2012 at 03:03

This is required nevertheless, but FWIW: #1683884: Update, simplify, and speedup run-tests.sh for PSR-0, and allow to run all tests of a module

Comment #11

jthorson CreditAttribution: jthorson commented 14 July 2012 at 06:27

Status:

Needs review

» Fixed

File	Size
pifr.psr0_.11.patch	4.94 KB

As committed to 6.x-2.x (Commit ID 8424b54).

Comment #12

sun

German

Karlsruhe

CreditAttribution: sun commented 14 July 2012 at 17:05

Yay, you even filled in the documentation gaps :) Thank you!

That said, as soon as #1683884: Update, simplify, and speedup run-tests.sh for PSR-0, and allow to run all tests of a module lands for D8, we should be able to replace this entire code with a simple

--module implode(',', $args)

That is, because it shouldn't be PIFR's job to figure this out in the first place ;)

Comment #13

sun

German

Karlsruhe

CreditAttribution: sun commented 15 July 2012 at 01:59

File	Size
project_issue_file_review.module.13.patch	2.32 KB

FWIW, this would sorta be the follow-up patch for --module, but I'm not 100% sure yet whether it's safe to rely on the new option already.

Comment #14

aspilicious CreditAttribution: aspilicious commented 25 July 2012 at 18:55

Status:

Fixed

» Active

Hmm there is still something going wrong...

PHP Fatal error: Call to undefined method Drupal\views\Tests\Field\ApiTestBase::getInfo() in /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh on line 366

Fatal error: Call to undefined method Drupal\views\Tests\Field\ApiTestBase::getInfo() in /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh on line 366
FATAL Drupal\views\Tests\Field\ApiTestBase: test runner returned a non-zero error code (255).

FATAL Drupal\views\Tests\Field\ApiTestBase: Found no database prefix for test ID 2. (Check whether setUp() is invoked correctly.)Argument: Null 5 passes, 0 fails, and 0 exceptions

Base test files that are abstract (but not rly defined as abstract) are added as real test files. That leads to fatals when searching for the getInfo() function.

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

Comment #15

jthorson CreditAttribution: jthorson commented 25 July 2012 at 19:38

If someone can define an algorithm for detecting what should or shouldn't be considered a 'real' test file, we can modify it ... for now, we could simply filter out any file with 'TestBase' in the name, but that feels more like 'workaround' than 'robust solution'.