As the title says, only class methods starting with "assert" are ignored + simpletest walks higher up the debug_backtrace() to find the test case, method name, and code line that asserts something.

Currently, we see the method name and code line of _assertEqual() all over again; not very helpful.

FYI: Our improved assertEqual() did not go into D7 core, because it's an insane amount of work to manually update all assertEqual() invocations in all tests. That's one giant downside of writing plenty of tests. That said, DrupalWebTestCase in core should probably have an API version number appended, so test cases can choose between versions and upgrade/migrate over time, i.e., MollomTestCase extends DrupalWebTestCase1 - or similar. I'm no OOP expert.

Anyway, having to choose between a l33t-ish assertEqualXT() and an amusing assertSame(), I went for the latter ;)

CommentFileSizeAuthor
#12 mollom-HEAD.assertSame.12.patch12.24 KBsun
#10 mollom-HEAD.assertSame.10.patch12.2 KBsun
#7 mollom-DRUPAL-6--1.assertsame.7.patch11.14 KBsun
mollom-DRUPAL-6--1.assertsame.0.patch9.45 KBsun

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

mollom-DRUPAL-6--1.assertsame.0.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, mollom-DRUPAL-6--1.assertsame.0.patch, failed testing.

dries’s picture

dries commented
Status: Needs work » Needs review

I'm OK with the more amusing assertSame(). Maybe it is wort extending the @todo with some more information so we remember why we went with this approach for the time being?

dries’s picture

dries commented

I applied this patch and ran the tests locally. They pass for me too.

dries’s picture

dries commented
Status: Needs review » Fixed

I committed the patch to CVS HEAD. Hopefully the test was failing because another problem. Let's see ...

dries’s picture

dries commented
Status: Fixed » Needs work

Sorry, I did not commit this patch -- I committed #487454: E-mails cannot be reported instead. This patch will need a reroll because of #487454.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review
StatusFileSize
new mollom-DRUPAL-6--1.assertsame.7.patch11.14 KB

Improved documentation/@todo and also fixed output escaping.

dries’s picture

dries commented
Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Needs work

Committed #7 to DRUPAL-6--1. Moving to CVS HEAD.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Patch (to be ported)
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new mollom-HEAD.assertSame.10.patch12.2 KB

Straight port. Let's see.

Status: Needs review » Needs work

The last submitted patch, mollom-HEAD.assertSame.10.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new mollom-HEAD.assertSame.12.patch12.24 KB 
+++ tests/mollom.test	2 Aug 2010 00:22:47 -0000
@@ -2117,11 +2129,11 @@ class MollomDataTestCase extends MollomW
-    $this->_assertEqual('post_body', $data['post_body'], $edit['comment_body[und][0][value]']);
...
+    $this->assertSame('post_body', $data['post_body'], $edit['comment']);

@@ -2163,11 +2175,11 @@ class MollomDataTestCase extends MollomW
-    $this->_assertEqual('post_body', $data['post_body'], $edit['comment_body[und][0][value]']);
...
+    $this->assertSame('post_body', $data['post_body'], $edit['comment']);

Bad idea: Blatantly copying D6 code ;)

Powered by Dreditor.

dries’s picture

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

Victory! Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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

  • Commit 38b4198 on master, fai6, 8.x-2.x, fbajs, actions by Dries: 
    - Patch #868712 by sun: rename improved _assertEqual() into assert*()...
  • Commit 7eba363 on master, fai6, 8.x-2.x, fbajs, actions by Dries:
    - Patch #868712 by sun: rename improved _assertEqual() into assert*()...

  • Commit 38b4198 on master, fai6, 8.x-2.x, fbajs, actions by Dries: 
    - Patch #868712 by sun: rename improved _assertEqual() into assert*()...
  • Commit 7eba363 on master, fai6, 8.x-2.x, fbajs, actions by Dries:
    - Patch #868712 by sun: rename improved _assertEqual() into assert*()...