This is a child task for #422380: Convert all core module to use new static caching API focusing on the simpletest module.

CommentFileSizeAuthor
#16 481498-16.patch4.18 KBJamesAn
#13 481498-13.patch4.18 KBJamesAn
#11 481498-simpletest-static.patch4 KBboombatower
#9 481498-simpletest-static.patch4 KBboombatower
#3 481498-3.patch4.13 KBJamesAn
#1 481498-1.patch4.05 KBJamesAn

Comments

JamesAn’s picture

JamesAn commented
Status: Active » Needs review
StatusFileSize
new 481498-1.patch4.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

JamesAn commented
Status: Needs work » Needs review
StatusFileSize
new 481498-3.patch4.13 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

boombatower’s picture

boombatower commented

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

boombatower commented

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

JamesAn commented
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

boombatower commented
Status: Postponed » Active

Issue has been committed.

boombatower’s picture

boombatower commented
Status: Active » Needs review
StatusFileSize
new 481498-simpletest-static.patch4 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

boombatower commented
Status: Needs work » Needs review
StatusFileSize
new 481498-simpletest-static.patch4 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

JamesAn commented
Status: Needs work » Needs review
StatusFileSize
new 481498-13.patch4.18 KB

+ return files;
should be
+ return $files;

Status: Needs review » Needs work

The last submitted patch failed testing.

boombatower’s picture

boombatower commented
Status: Needs work » Needs review

Test slave crapped.

JamesAn’s picture

JamesAn commented
StatusFileSize
new 481498-16.patch4.18 KB

Rerolled.

boombatower’s picture

boombatower commented
Status: Needs review » Reviewed & tested by the community

Looks good.

dries’s picture

dries commented
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.