From 745080671b5a22bc991fb09e58cc24dc9a084197 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 6 Jan 2023 21:20:20 +0800 Subject: [PATCH] MDL-76362 question: Refactor question number unit tests --- question/engine/tests/coverage.php | 37 ++ question/engine/tests/helpers.php | 19 - .../engine/tests/question_engine_test.php | 359 +++++++++++++----- question/tests/coverage.php | 40 ++ 4 files changed, 345 insertions(+), 110 deletions(-) create mode 100644 question/engine/tests/coverage.php create mode 100644 question/tests/coverage.php diff --git a/question/engine/tests/coverage.php b/question/engine/tests/coverage.php new file mode 100644 index 00000000000..2589273d3dc --- /dev/null +++ b/question/engine/tests/coverage.php @@ -0,0 +1,37 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * Coverage information for the question_engine. + * + * @copyright 2022 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +return new class extends phpunit_coverage_info { + /** @var array The list of files relative to the plugin root to include in coverage generation. */ + protected $includelistfiles = [ + 'bank.php', + 'datalib.php', + 'lib.php', + 'questionattempt.php', + 'questionattemptstep.php', + 'questionusage.php', + 'renderer.php', + 'states.php', + ]; +}; diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php index 33d9894c01a..c7da45ae584 100644 --- a/question/engine/tests/helpers.php +++ b/question/engine/tests/helpers.php @@ -1391,22 +1391,3 @@ class question_test_recordset extends moodle_recordset { $this->records = null; } } - -/** - * Helper class for tests that help to test core_question_renderer. - * - * @copyright 2018 Huong Nguyen - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class testable_core_question_renderer extends core_question_renderer { - - /** - * Test the private number function. - * - * @param null|string $number - * @return HTML - */ - public function number($number) { - return parent::number($number); - } -} diff --git a/question/engine/tests/question_engine_test.php b/question/engine/tests/question_engine_test.php index a46be4b1fc7..3f6d433bf76 100644 --- a/question/engine/tests/question_engine_test.php +++ b/question/engine/tests/question_engine_test.php @@ -28,18 +28,18 @@ namespace core_question; use advanced_testcase; use moodle_exception; use question_engine; -use testable_core_question_renderer; /** * Unit tests for the question_engine class. * * @copyright 2009 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @coversDefaultClass \question_engine */ class question_engine_test extends advanced_testcase { /** - * Load required libraries + * Load required libraries. */ public static function setUpBeforeClass(): void { global $CFG; @@ -47,113 +47,290 @@ class question_engine_test extends advanced_testcase { require_once("{$CFG->dirroot}/question/engine/lib.php"); } - public function test_load_behaviour_class() { - // Exercise SUT + /** + * Tests for load_behaviour_class. + * + * @covers \question_engine::load_behaviour_class + */ + public function test_load_behaviour_class(): void { + // Exercise SUT. question_engine::load_behaviour_class('deferredfeedback'); - // Verify + + // Verify. $this->assertTrue(class_exists('qbehaviour_deferredfeedback')); } - public function test_load_behaviour_class_missing() { - // Exercise SUT + /** + * Tests for load_behaviour_class when a class is missing. + * + * @covers \question_engine::load_behaviour_class + */ + public function test_load_behaviour_class_missing(): void { + // Exercise SUT. $this->expectException(moodle_exception::class); question_engine::load_behaviour_class('nonexistantbehaviour'); } - public function test_get_behaviour_unused_display_options() { - $this->assertEquals(array(), question_engine::get_behaviour_unused_display_options('interactive')); - $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'), - question_engine::get_behaviour_unused_display_options('deferredfeedback')); - $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'), - question_engine::get_behaviour_unused_display_options('deferredcbm')); - $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'), - question_engine::get_behaviour_unused_display_options('manualgraded')); + /** + * Test the get_behaviour_unused_display_options with various options. + * + * @covers \question_engine::get_behaviour_unused_display_options + * @dataProvider get_behaviour_unused_display_options_provider + * @param string $behaviour + * @param array $expected + */ + public function test_get_behaviour_unused_display_options(string $behaviour, array $expected): void { + $this->assertEquals($expected, question_engine::get_behaviour_unused_display_options($behaviour)); } - public function test_can_questions_finish_during_the_attempt() { - $this->assertFalse(question_engine::can_questions_finish_during_the_attempt('deferredfeedback')); - $this->assertTrue(question_engine::can_questions_finish_during_the_attempt('interactive')); + /** + * Data provider for get_behaviour_unused_display_options. + * + * @return array + */ + public function get_behaviour_unused_display_options_provider(): array { + return [ + 'interactive' => [ + 'interactive', + [], + ], + 'deferredfeedback' => [ + 'deferredfeedback', + ['correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'], + ], + 'deferredcbm' => [ + 'deferredcbm', + ['correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'], + ], + 'manualgraded' => [ + 'manualgraded', + ['correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'], + ], + ]; } - public function test_sort_behaviours() { - $in = array('b1' => 'Behave 1', 'b2' => 'Behave 2', 'b3' => 'Behave 3', 'b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6'); - - $out = array('b1' => 'Behave 1', 'b2' => 'Behave 2', 'b3' => 'Behave 3', 'b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6'); - $this->assertSame($out, question_engine::sort_behaviours($in, '', '', '')); - - $this->assertSame($out, question_engine::sort_behaviours($in, '', 'b4', 'b4')); - - $out = array('b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6'); - $this->assertSame($out, question_engine::sort_behaviours($in, '', 'b1,b2,b3,b4', 'b4')); - - $out = array('b6' => 'Behave 6', 'b1' => 'Behave 1', 'b4' => 'Behave 4'); - $this->assertSame($out, question_engine::sort_behaviours($in, 'b6,b1,b4', 'b2,b3,b4,b5', 'b4')); - - $out = array('b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4'); - $this->assertSame($out, question_engine::sort_behaviours($in, 'b6,b5,b4', 'b1,b2,b3', 'b4')); - - $out = array('b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4'); - $this->assertSame($out, question_engine::sort_behaviours($in, 'b1,b6,b5', 'b1,b2,b3,b4', 'b4')); - - $out = array('b2' => 'Behave 2', 'b4' => 'Behave 4', 'b6' => 'Behave 6'); - $this->assertSame($out, question_engine::sort_behaviours($in, 'b2,b4,b6', 'b1,b3,b5', 'b2')); - - // Ignore unknown input in the order argument. - $this->assertSame($in, question_engine::sort_behaviours($in, 'unknown', '', '')); - - // Ignore unknown input in the disabled argument. - $this->assertSame($in, question_engine::sort_behaviours($in, '', 'unknown', '')); + /** + * Tests for can_questions_finish_during_the_attempt. + * + * @covers \question_engine::can_questions_finish_during_the_attempt + * @dataProvider can_questions_finish_during_the_attempt_provider + * @param string $behaviour + * @param bool $expected + */ + public function test_can_questions_finish_during_the_attempt(string $behaviour, bool $expected): void { + $this->assertEquals($expected, question_engine::can_questions_finish_during_the_attempt($behaviour)); } - public function test_is_manual_grade_in_range() { - $_POST[] = array('q1:2_-mark' => 0.5, 'q1:2_-maxmark' => 1.0, - 'q1:2_:minfraction' => 0, 'q1:2_:maxfraction' => 1); + /** + * Data provider for can_questions_finish_during_the_attempt_provider. + * + * @return array + */ + public function can_questions_finish_during_the_attempt_provider(): array { + return [ + ['deferredfeedback', false], + ['interactive', true], + ]; + } + + /** + * Tests for sort_behaviours + * + * @covers \question_engine::sort_behaviours + * @dataProvider sort_behaviours_provider + * @param array $input The params passed to sort_behaviours + * @param array $expected + */ + public function test_sort_behaviours(array $input, array $expected): void { + $this->assertSame($expected, question_engine::sort_behaviours(...$input)); + } + + /** + * Data provider for sort_behaviours. + * + * @return array + */ + public function sort_behaviours_provider(): array { + $in = [ + 'b1' => 'Behave 1', + 'b2' => 'Behave 2', + 'b3' => 'Behave 3', + 'b4' => 'Behave 4', + 'b5' => 'Behave 5', + 'b6' => 'Behave 6', + ]; + + return [ + [ + [$in, '', '', ''], + $in, + ], + [ + [$in, '', 'b4', 'b4'], + $in, + ], + [ + [$in, '', 'b1,b2,b3,b4', 'b4'], + ['b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6'], + ], + [ + [$in, 'b6,b1,b4', 'b2,b3,b4,b5', 'b4'], + ['b6' => 'Behave 6', 'b1' => 'Behave 1', 'b4' => 'Behave 4'], + ], + [ + [$in, 'b6,b5,b4', 'b1,b2,b3', 'b4'], + ['b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4'], + ], + [ + [$in, 'b1,b6,b5', 'b1,b2,b3,b4', 'b4'], + ['b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4'], + ], + [ + [$in, 'b2,b4,b6', 'b1,b3,b5', 'b2'], + ['b2' => 'Behave 2', 'b4' => 'Behave 4', 'b6' => 'Behave 6'], + ], + // Ignore unknown input in the order argument. + [ + [$in, 'unknown', '', ''], + $in, + ], + // Ignore unknown input in the disabled argument. + [ + [$in, '', 'unknown', ''], + $in, + ], + ]; + } + + /** + * Tests for is_manual_grade_in_range. + * + * @dataProvider is_manual_grade_in_range_provider + * @covers \question_engine::is_manual_grade_in_range + * @param array $post The values to add to $_POST + * @param array $params The params to pass to is_manual_grade_in_range + * @param bool $expected + */ + public function test_is_manual_grade_in_range(array $post, array $params, bool $expected): void { + $_POST[] = $post; + $this->assertEquals($expected, question_engine::is_manual_grade_in_range(...$params)); + } + + /** + * Data provider for is_manual_grade_in_range tests. + * + * @return array + */ + public function is_manual_grade_in_range_provider(): array { + return [ + 'In range' => [ + 'post' => [ + 'q1:2_-mark' => 0.5, + 'q1:2_-maxmark' => 1.0, + 'q1:2_:minfraction' => 0, + 'q1:2_:maxfraction' => 1, + ], + 'range' => [1, 2], + 'expected' => true, + ], + 'Bottom end' => [ + 'post' => [ + 'q1:2_-mark' => -1.0, + 'q1:2_-maxmark' => 2.0, + 'q1:2_:minfraction' => -0.5, + 'q1:2_:maxfraction' => 1, + ], + 'range' => [1, 2], + 'expected' => true, + ], + 'Too low' => [ + 'post' => [ + 'q1:2_-mark' => -1.1, + 'q1:2_-maxmark' => 2.0, + 'q1:2_:minfraction' => -0.5, + 'q1:2_:maxfraction' => 1, + ], + 'range' => [1, 2], + 'expected' => true, + ], + 'Top end' => [ + 'post' => [ + 'q1:2_-mark' => 3.0, + 'q1:2_-maxmark' => 1.0, + 'q1:2_:minfraction' => -6.0, + 'q1:2_:maxfraction' => 3.0, + ], + 'range' => [1, 2], + 'expected' => true, + ], + 'Too high' => [ + 'post' => [ + 'q1:2_-mark' => 3.1, + 'q1:2_-maxmark' => 1.0, + 'q1:2_:minfraction' => -6.0, + 'q1:2_:maxfraction' => 3.0, + ], + 'range' => [1, 2], + 'expected' => true, + ], + ]; + } + + /** + * Tests for is_manual_grade_in_range. + * + * @covers \question_engine::is_manual_grade_in_range + */ + public function test_is_manual_grade_in_range_ungraded(): void { $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); } - public function test_is_manual_grade_in_range_bottom_end() { - $_POST[] = array('q1:2_-mark' => -1.0, 'q1:2_-maxmark' => 2.0, - 'q1:2_:minfraction' => -0.5, 'q1:2_:maxfraction' => 1); - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); + /** + * Ensure that the number renderer performs as expected. + * + * @covers \core_question_renderer::number + * @dataProvider render_question_number_provider + * @param mixed $value + * @param string $expected + */ + public function test_render_question_number($value, string $expected): void { + global $PAGE; + + $renderer = new \core_question_renderer($PAGE, 'core_question'); + $rc = new \ReflectionClass($renderer); + $rcm = $rc->getMethod('number'); + $rcm->setAccessible(true); + + $this->assertEquals($expected, $rcm->invoke($renderer, $value)); } - public function test_is_manual_grade_in_range_too_low() { - $_POST[] = array('q1:2_-mark' => -1.1, 'q1:2_-maxmark' => 2.0, - 'q1:2_:minfraction' => -0.5, 'q1:2_:maxfraction' => 1); - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); - } - - public function test_is_manual_grade_in_range_top_end() { - $_POST[] = array('q1:2_-mark' => 3.0, 'q1:2_-maxmark' => 1.0, - 'q1:2_:minfraction' => -6.0, 'q1:2_:maxfraction' => 3.0); - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); - } - - public function test_is_manual_grade_in_range_too_high() { - $_POST[] = array('q1:2_-mark' => 3.1, 'q1:2_-maxmark' => 1.0, - 'q1:2_:minfraction' => -6.0, 'q1:2_:maxfraction' => 3.0); - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); - } - - public function test_is_manual_grade_in_range_ungraded() { - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); - } - - public function test_render_question_number() { - global $CFG, $PAGE; - - require_once("{$CFG->dirroot}/question/engine/tests/helpers.php"); - $renderer = new testable_core_question_renderer($PAGE, 'core_question'); - - // Test with number is i character. - $this->assertEquals('

Information

', $renderer->number('i')); - // Test with number is empty string. - $this->assertEquals('', $renderer->number('')); - // Test with number is 0. - $this->assertEquals('

Question 0

', $renderer->number(0)); - // Test with number is numeric. - $this->assertEquals('

Question 1

', $renderer->number(1)); - // Test with number is string. - $this->assertEquals('

Question 1 of 2

', $renderer->number('1 of 2')); + /** + * Data provider for test_render_question_number. + * + * @return array + */ + public function render_question_number_provider(): array { + return [ + 'Test with number is i character' => [ + 'i', + '

Information

', + ], + 'Test with number is empty string' => [ + '', + '', + ], + 'Test with number is 0' => [ + 0, + '

Question 0

', + ], + 'Test with number is numeric' => [ + 1, + '

Question 1

', + ], + 'Test with number is string' => [ + '1 of 2', + '

Question 1 of 2

', + ], + ]; } } diff --git a/question/tests/coverage.php b/question/tests/coverage.php new file mode 100644 index 00000000000..4bcdaa33a5e --- /dev/null +++ b/question/tests/coverage.php @@ -0,0 +1,40 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * Coverage information for the core_question. + * + * @copyright 2022 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +return new class extends phpunit_coverage_info { + /** @var array The list of files relative to the plugin root to include in coverage generation. */ + protected $includelistfiles = [ + 'category_class.php', + 'category_form.php', + 'editlib.php', + 'export_form.php', + 'format.php', + 'import_form.php', + 'lib.php', + 'move_form.php', + 'previewlib.php', + 'renderer.php', + 'upgrade.php', + ]; +};