I think it is important to test the main regex expression in this module for all possible situations to make sure it always works correctly.
We can gather test cases in collection, create a unit test and attach it to this module. If it will be one more issue with problem of this regex, we could add another test case to automatic tests and so on.
Will it be okey if i help you with it?

Comments

fizk’s picture

Thanks Nikita, that's a great idea. In fact, I don't think the next release will be made without automated testing being done first.

nikita petrov’s picture

Status: Active » Needs review
StatusFileSize
new3.67 KB

I think the best approach for testing smiley text filter (function smiley_filter_process) is the Unit testing (DrupalUnitTestCase), not Functional testing (DrupalWebTestCase), because in fact it is a unit - we should give it an input (text with smileys codes) and get an output (text with smiles converted into images). But we have a problem -
current version of this function requires a database to fetch an array of smileys objects (function smiley_get_all()). So we need to slightly refactor this function like this:

function smiley_filter_process($text, $filter, $format) {
  $smileys = smiley_get_all();
  return smiley_process_text($text, $smileys);
}

function smiley_process_text($text, $smileys) {
... rest of the code 
}

After thaе we should define an array with objects of smiles in our test cases manually, like this:

    $smileys = array (
      (object) array (
        'sid' => '45',
        'uri' => 'kolobok/smile.gif',
        'status' => '1',
        'acronyms' => '*smile* :-) :) +) =) :smile:',
        'description' => 'Smile',
      ),    
    );

and then we could run tests:

$original_text = 'simple text with :) in it';
$expected_text = 'simple text with <img[^>]*> in it';
$new_text = smiley_process_text($original_text, $smileys);
$this->assertTrue(preg_match($expected_text, $new_text), t('All smileys were replaced correctly.'));

How do you think, is my approach ok? Should i further work in that direction?
I attached my current working code for you for better understanding what i mean.

fizk’s picture

Status: Needs review » Active

That looks like a great place to start. I've committed it here:

http://drupalcode.org/project/smiley.git/commit/b6b6f83cef1f6dc47ef77cf8...

If we're going to use <img[^>]*> to match the image tag, we should do one test that specifically checks that the img src attribute is correct.

A few other checks we could do are:

  • boundary cases, like :)), :))), etc, assuming $smileys only contains :).
  • a lot of smileys: does the parser crash? take too long to finish?
  • smileys intermingled with HTML
fizk’s picture

I just noticed that we have to use DrupalWebTestCase to enable the smiley module before running test cases.

fizk’s picture

Status: Active » Closed (fixed)

Several test cases have been committed.