From bf81b16824b61a5ee7db7e5571402aa9d40ce00a Mon Sep 17 00:00:00 2001 From: Sam Marshall 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.43.0