From bf81b16824b61a5ee7db7e5571402aa9d40ce00a Mon Sep 17 00:00:00 2001
From: Sam Marshall <s.marshall@open.ac.uk>
Date: Mon, 1 Nov 2010 18:00:57 +0000
Subject: [PATCH] Unit tests MDL-24981 Fixed portfolio unit test so it at least
 runs (albeit with 23 exceptions and only 2 passes)

---
 lib/portfolio/exceptions.php                  | 33 ++++++++++++++-----
 lib/simpletest/portfolio_testclass.php        | 16 +++++++--
 .../test_assignment_portfolio_callers.php     |  1 +
 .../test_data_portfolio_callers.php           |  1 +
 4 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/lib/portfolio/exceptions.php b/lib/portfolio/exceptions.php
index 2a4816d18a1..d39e6d570d5 100644
--- a/lib/portfolio/exceptions.php
+++ b/lib/portfolio/exceptions.php
@@ -49,18 +49,33 @@ class portfolio_export_exception extends portfolio_exception {
     */
     public function __construct($exporter, $errorcode, $module=null, $continue=null, $a=null) {
         global $CFG;
+        // This static variable is necessary because sometimes the code below
+        // which tries to obtain a continue link can cause one of these
+        // exceptions to be thrown. This would create an infinite loop (until
+        // PHP hits its stack limit). Using this static lets us make the
+        // nested constructor finish immediately without attempting to call
+        // methods that might fail.
+        static $inconstructor = false;
 
-        if (!empty($exporter) && $exporter instanceof portfolio_exporter) {
-            if (empty($continue)) {
-                $caller = $exporter->get('caller');
-                if (!empty($caller) && $caller instanceof portfolio_caller_base) {
-                    $continue = $exporter->get('caller')->get_return_url();
+        if (!$inconstructor && !empty($exporter) &&
+                $exporter instanceof portfolio_exporter) {
+            $inconstructor = true;
+            try {
+                if (empty($continue)) {
+                    $caller = $exporter->get('caller');
+                    if (!empty($caller) && $caller instanceof portfolio_caller_base) {
+                        $continue = $exporter->get('caller')->get_return_url();
+                    }
                 }
+                // this was previously only called if we were in cron,
+                // but I think if there's always an exception, we should clean up
+                // rather than force the user to resolve the export later.
+                $exporter->process_stage_cleanup();
+            } catch(Exception $e) {
+                // Ooops, we had an exception trying to get caller
+                // information. Ignore it.
             }
-            // this was previously only called if we were in cron,
-            // but I think if there's always an exception, we should clean up
-            // rather than force the user to resolve the export later.
-            $exporter->process_stage_cleanup();
+            $inconstructor = false;
         }
         parent::__construct($errorcode, $module, $continue, $a);
     }
diff --git a/lib/simpletest/portfolio_testclass.php b/lib/simpletest/portfolio_testclass.php
index 4135c182c54..766f7062113 100644
--- a/lib/simpletest/portfolio_testclass.php
+++ b/lib/simpletest/portfolio_testclass.php
@@ -192,10 +192,22 @@ foreach ($portfolio_plugins as $plugin) {
     Mock::generatePartial("portfolio_plugin_$plugin", "partialmock_plugin_$plugin", array('send_package'));
 }
 
-class portfoliolib_test extends FakeDBUnitTestCase {
+class portfoliolib_test extends UnitTestCaseUsingDatabase {
+    private $olduser;
 
     function setup() {
+        global $USER;
         parent::setup();
+
+        // It is necessary to store $USER object because some subclasses use generator
+        // stuff which breaks $USER
+        $this->olduser = $USER;
+    }
+
+    function tearDown() {
+        global $USER;
+        $USER = $this->olduser;
+        parent::tearDown();
     }
 
     function test_construct_dupe_instance() {
@@ -205,7 +217,7 @@ class portfoliolib_test extends FakeDBUnitTestCase {
             $plugin2 = portfolio_plugin_base::create_instance('download', 'download2', array());
             $test1 = new portfolio_plugin_download($plugin1->get('id'));
         } catch (portfolio_exception $e) {
-            $this->assertEqual('multipledisallowed', $e->errorcode);
+            $this->assertEqual('multipleinstancesdisallowed', $e->errorcode);
             $gotexception = true;
         }
         $this->assertTrue($gotexception);
diff --git a/mod/assignment/simpletest/test_assignment_portfolio_callers.php b/mod/assignment/simpletest/test_assignment_portfolio_callers.php
index f980e203ad6..7e9898087ac 100644
--- a/mod/assignment/simpletest/test_assignment_portfolio_callers.php
+++ b/mod/assignment/simpletest/test_assignment_portfolio_callers.php
@@ -2,6 +2,7 @@
 require_once("$CFG->libdir/simpletest/portfolio_testclass.php");
 require_once("$CFG->dirroot/mod/assignment/lib.php");
 require_once("$CFG->dirroot/$CFG->admin/generator.php");
+require_once("$CFG->dirroot/mod/assignment/locallib.php");
 
 Mock::generate('assignment_portfolio_caller', 'mock_caller');
 Mock::generate('portfolio_exporter', 'mock_exporter');
diff --git a/mod/data/simpletest/test_data_portfolio_callers.php b/mod/data/simpletest/test_data_portfolio_callers.php
index a17ca8df359..df247d6a281 100644
--- a/mod/data/simpletest/test_data_portfolio_callers.php
+++ b/mod/data/simpletest/test_data_portfolio_callers.php
@@ -2,6 +2,7 @@
 require_once("$CFG->libdir/simpletest/portfolio_testclass.php");
 require_once("$CFG->dirroot/mod/data/lib.php");
 require_once("$CFG->dirroot/$CFG->admin/generator.php");
+require_once("$CFG->dirroot/mod/data/locallib.php");
 
 Mock::generate('data_portfolio_caller', 'mock_caller');
 Mock::generate('portfolio_exporter', 'mock_exporter');
-- 
2.17.1