DrupalWebTestCase need a full cleanup, let's start by documenting class members, and adding proper visibility modifiers.

CommentFileSizeAuthor
#11 338239-follow-up-1.patch1.22 KBc960657
#9 338239-cleanup-drupal-web-test-case.patch47.92 KBdamien tournoud
#5 338239-cleanup-drupal-web-test-case.patch46.15 KBdamien tournoud
#3 338239-cleanup-drupal-web-test-case.patch45.1 KBdamien tournoud
#1 338239-cleanup-drupal-web-test-case.patch44.72 KBdamien tournoud

Comments

damien tournoud’s picture

damien tournoud commented
Assigned: Unassigned » damien tournoud
Status: Active » Needs review
StatusFileSize
new 338239-cleanup-drupal-web-test-case.patch44.72 KB

A first pass of clean-up. This is likely to break some stuff around.

damien tournoud’s picture

damien tournoud commented
Title: Cleanup DrupalWebTestCase: pass #1 » Cleaning-up DrupalWebTestCase: pass #1
damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 338239-cleanup-drupal-web-test-case.patch45.1 KB

Unbreak Simpletest self-test.

Status: Needs review » Needs work

The last submitted patch failed testing.

damien tournoud’s picture

damien tournoud commented
Status: Needs work » Needs review
StatusFileSize
new 338239-cleanup-drupal-web-test-case.patch46.15 KB

Fix session tests, and reroll for this night changes. Looks good to go for this first pass.

catch’s picture

catch
he/him
Primary language English
commented

So everything in here is just code comments, and private/protected corrections. Comments look fine to me, private/protected etc. also looks fine, but then with that stuff I'd probably not notice if it wasn't.

damien tournoud’s picture

damien tournoud commented

About visibility modifiers: basically all methods are protected, except assert() that is very private and a public command: run().

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

That explanation makes sense, so I'm marking to RTBC.

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 338239-cleanup-drupal-web-test-case.patch47.92 KB

Rerolled, and adding a fix for DatabaseTemporaryQueryTestCase that the test bot apparently missed. Also cleaned-up DrupalHTTPRequestTestCase that had a reference to ->_content in its comments.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

c960657’s picture

c960657 commented
Status: Fixed » Needs review
StatusFileSize
new 338239-follow-up-1.patch1.22 KB

This broke the File > File scan directory test. Fix attached.

hswong3i’s picture

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

+1 for #11

hswong3i’s picture

hswong3i commented
Priority: Normal » Critical

Pump it to critical since it break simpletest.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed. :( Really wish testing bot was catching these... :(

Status: Fixed » Closed (fixed)

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