Over in #1182290: Add boilerplate upgrade path tests @chx introduced the usage of gzip'd files in tests, we can't always count on systems to have zlib installed so the attached patch enables tests to declare a 'requirements' property in getInfo() which would contain an array of function names to pass through function_exists() before the test is run.

So for #1182290: Add boilerplate upgrade path tests, the following would be added to the getInfo() method:

+      'requirements' => array('gzopen'),
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

matason’s picture

Status: Active » Needs review
FileSize
682 bytes

Attaching the patch...

catch’s picture

Issue tags: +Needs change record

This should be added as phpdoc for getInfo() as well.

Otherwise looks great.

API addition so needs a change notification once it's in.

Dave Reid’s picture

Issue tags: -Needs change record

Why not just wrap the test class itself in function_exists('gzopen') { // Class definition }?

Dave Reid’s picture

Issue tags: +Needs change record
matason’s picture

Issue tags: -Needs change record

I am just wondering whether 'requirements' is right and perhaps even whether there's a clean way of incorporating these function_exist() checks into 'dependencies' as this is what we're really talking about.

catch’s picture

Issue tags: +Needs change record

Also what about returning a flag from getInfo() rather than array of function names. I could see checks for ini_get, classes and other stuff that's not functions being useful here.

Dave Reid’s picture

Let's just make an access static function for tests that gets called and returns a TRUE/FALSE value.

class MyTestCase extends DrupalWebTestCase {
  public static function getInfo() {
    ...
  }

  public static function getAccess() {
    return function_exists('gzopen');
  }
}

This or give a way for tests to stop executing in setUp() gracefully.

BTMash’s picture

FileSize
1.25 KB

I like the suggestion from #7 since adding in the check is actually quite easy (it can be added to DrupalTestCase so then any other type of tests that are subclasses of DrupalTestCase can also get checked for access) and any test that requires any additional requirements can implement their check fairly easily. I tested this out and got tests to appear/disappear as necessary. Attaching patch.

BTMash’s picture

FileSize
1.23 KB

Ack...whitespace. Attaching new patch.

chx’s picture

Status: Needs review » Closed (won't fix)

We dont want this apparently.

matason’s picture

To elaborate on why we don't want this: It was decided by discussion on IRC (@webchick, @chx, @davereid, @xjm) that it would be better to have tests fail with a message about unmet requirements such as zlib than have tests silently not run at all.

xjm’s picture

Issue summary: View changes
Issue tags: -Needs change record
Mile23’s picture