logging error messages in importers is tricky

[bradfordcondon]

    $importer = new \tripal_expression_data_loader();
    $importer->create($run_args, $file);
    $importer->prepareFiles();


    $message = "ERROR: More than one feature matches the feature name:";
    $output = silent(function() use ($importer) {
      $importer->run();
    });

    $output->assertSee($message);

If we use $this->logmessage in the importer to log the message, the above test will fail. If we print the message instead, it will pass.

I dont mock a job for the importer. because of that, it doesnt execute the job's logMessage method (see below, the importers logmessage)

    // If we have a job then pass along the messaging to the job.
    if ($this->job) {
      $this->job->logMessage($message, $variables, $severity);
    }
    // If we don't have a job then just use the drpual_set_message.
    else {
      // Report this message to watchdog or set a message.
      if ($severity == TRIPAL_CRITICAL or $severity == TRIPAL_ERROR) {
        drupal_set_message($tmessage, 'error');
      }
      if ($severity == TRIPAL_WARNING) {
        drupal_set_message($tmessage, 'warning');
      }
    }

TripalTestSuite needs a simple way to see error logging. That might mean mocking the job. It might mean the importer does something like return false

[almasaeed2010]

I think it's really important that the developer designs functions and classes as testable.

The problem we are facing here is mainly caused by trying to test the entire class using a single method run. If our smaller functions were a little more independent and testable (can either throw exceptions or return null/false), we'd have a much better testing coverage. I do get though that it's a lot of work to rewrite classes likes these. Therefore, we might be able to allow the silent collector to inspect and pull the drupal messages from the $_SESSION variable.

[bradfordcondon]

i see what you're saying though. I could rewrite the test to, instead of running the whole importer, just run the method. And we can encourage that.

Then for when you use the run method you should really just be testing that the whole thing works.