Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JamesAn’s picture

Status: Active » Needs review
FileSize
4.05 KB

A wrapper function as per #422362-20: convert form.inc to use new static caching API was created to reset the static var in file_test_file_scan_callback().

Status: Needs review » Needs work

The last submitted patch failed testing.

JamesAn’s picture

Status: Needs work » Needs review
FileSize
4.13 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

boombatower’s picture

Technically this is a duplicate of: #348455: Provide a scalable way to ensure a clean testing environment for SimpleTest, but we can just prioritize this one.

boombatower’s picture

Please hold off on this patch though, longer standing patch with better cleanup of loading stuff #449198: SimpleTest: Clean up test loading and related API which also adds statics stuff. (just needs committing) [perhaps you want to review]

JamesAn’s picture

Status: Needs work » Postponed

Postponed on account of #449198: SimpleTest: Clean up test loading and related API. Thanks for the heads up!

I'm not sure about reviewing that large patch... I've never reviewed anything before, much less a large patch of an unfamiliar module.. ^^"

boombatower’s picture

Status: Postponed » Active

Issue has been committed.

boombatower’s picture

Status: Active » Needs review
FileSize
4 KB

I added drupal_static() in checkPermissions(), but I am not sure where to go from there. In order to complete the process setUp() needs to take advantage of drupal_static_reset(), but I am not sure that is the scope of this particular issue.

Status: Needs review » Needs work

The last submitted patch failed testing.

boombatower’s picture

Status: Needs work » Needs review
FileSize
4 KB

Suppose this would be a bad idea (carried over from original) :)

function file_test_file_scan_callback($filepath == NULL) {

Status: Needs review » Needs work

The last submitted patch failed testing.

JamesAn’s picture

Status: Needs work » Needs review
FileSize
4.18 KB

+ return files;
should be
+ return $files;

Status: Needs review » Needs work

The last submitted patch failed testing.

boombatower’s picture

Status: Needs work » Needs review

Test slave crapped.

JamesAn’s picture

FileSize
4.18 KB

Rerolled.

boombatower’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Small code style issue: + } else {. Fixed that and committed to CVS HEAD.

Status: Fixed » Closed (fixed)

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