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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Assigned: Unassigned » Damien Tournoud
Status: Active » Needs review
FileSize
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 CreditAttribution: Damien Tournoud commented
Title: Cleanup DrupalWebTestCase: pass #1 » Cleaning-up DrupalWebTestCase: pass #1
Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
FileSize
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 CreditAttribution: Damien Tournoud commented
Status: Needs work » Needs review
FileSize
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
CreditAttribution: catch 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 CreditAttribution: 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
CreditAttribution: catch 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 CreditAttribution: Damien Tournoud commented
FileSize
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 CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

c960657’s picture

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

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

hswong3i’s picture

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

+1 for #11

hswong3i’s picture

hswong3i CreditAttribution: hswong3i commented
Priority: Normal » Critical

Pump it to critical since it break simpletest.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick 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.