From 298c13ac3b25823a71b9f48e74ac187fbe06be41 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 6 Feb 2024 13:05:49 +0800 Subject: [PATCH 1/2] MDL-80838 core: Add PSR-20/Clock support This commit adds the PSR-20 ClockInterface to core, with a moodle-specific extension to the Interface at `\core\clock`, and a standard clock at `\core\system_clock`. Further clocks are provided as `\incrementing_clock` and `\frozen_clock` which are available to unit tests using: - `$this->mock_clock_with_incrementing(?int $starttime = null);` - `$this->mock_clock_with_frozen(?int $time = null);` For the incrementing clock, every call to fetch the time will bump the current time by one second. For the frozen clock the time will not change, but can be modified with: - `$clock->set_to(int $time);`; and - `$clock->bump(int $seconds = 1);` --- lib/classes/clock.php | 33 +++++++ lib/classes/component.php | 1 + lib/classes/di.php | 5 + lib/classes/system_clock.php | 34 +++++++ lib/phpunit/classes/advanced_testcase.php | 34 +++++++ lib/phpunit/tests/advanced_test.php | 65 +++++++++++++ lib/psr/clock/LICENSE | 19 ++++ lib/psr/clock/README.md | 61 ++++++++++++ lib/psr/clock/readme_moodle.txt | 12 +++ lib/psr/clock/src/ClockInterface.php | 13 +++ lib/testing/classes/frozen_clock.php | 69 ++++++++++++++ lib/testing/classes/incrementing_clock.php | 67 +++++++++++++ lib/testing/tests/clock_test.php | 106 +++++++++++++++++++++ lib/tests/system_clock_test.php | 37 +++++++ lib/thirdpartylibs.xml | 8 ++ 15 files changed, 564 insertions(+) create mode 100644 lib/classes/clock.php create mode 100644 lib/classes/system_clock.php create mode 100644 lib/psr/clock/LICENSE create mode 100644 lib/psr/clock/README.md create mode 100644 lib/psr/clock/readme_moodle.txt create mode 100644 lib/psr/clock/src/ClockInterface.php create mode 100644 lib/testing/classes/frozen_clock.php create mode 100644 lib/testing/classes/incrementing_clock.php create mode 100644 lib/testing/tests/clock_test.php create mode 100644 lib/tests/system_clock_test.php diff --git a/lib/classes/clock.php b/lib/classes/clock.php new file mode 100644 index 00000000000..cbb0885d674 --- /dev/null +++ b/lib/classes/clock.php @@ -0,0 +1,33 @@ +. + +namespace core; + +/** + * Moodle Clock interface. + * + * @package core + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +interface clock extends \Psr\Clock\ClockInterface { + /** + * Return the unix time stamp for the current representation of the time. + * + * @return int + */ + public function time(): int; +} diff --git a/lib/classes/component.php b/lib/classes/component.php index bc13015f3a7..592aa6a6728 100644 --- a/lib/classes/component.php +++ b/lib/classes/component.php @@ -116,6 +116,7 @@ class core_component { 'lib/psr/http-factory/src', ], 'Psr\\EventDispatcher' => 'lib/psr/event-dispatcher/src', + 'Psr\\Clock' => 'lib/psr/clock/src', 'Psr\\Container' => 'lib/psr/container/src', 'GuzzleHttp\\Psr7' => 'lib/guzzlehttp/psr7/src', 'GuzzleHttp\\Promise' => 'lib/guzzlehttp/promises/src', diff --git a/lib/classes/di.php b/lib/classes/di.php index 7ce9ac29dcf..c8c1147c9d0 100644 --- a/lib/classes/di.php +++ b/lib/classes/di.php @@ -117,6 +117,11 @@ class di { // The string manager. \core_string_manager::class => fn() => get_string_manager(), + + // The Moodle Clock implementation, which itself is an extension of PSR-20. + // Alias the PSR-20 clock interface to the Moodle clock. They are compatible. + \core\clock::class => fn() => new \core\system_clock(), + \Psr\Clock\ClockInterface::class => \DI\get(\core\clock::class), ]); // Add any additional definitions using hooks. diff --git a/lib/classes/system_clock.php b/lib/classes/system_clock.php new file mode 100644 index 00000000000..32cee16898d --- /dev/null +++ b/lib/classes/system_clock.php @@ -0,0 +1,34 @@ +. + +namespace core; + +/** + * Standard system clock implementation. + * + * @package core + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class system_clock implements clock { + public function now(): \DateTimeImmutable { + return new \DateTimeImmutable(); + } + + public function time(): int { + return $this->now()->getTimestamp(); + } +} diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php index 9674a480156..4f039900f6c 100644 --- a/lib/phpunit/classes/advanced_testcase.php +++ b/lib/phpunit/classes/advanced_testcase.php @@ -729,4 +729,38 @@ abstract class advanced_testcase extends base_testcase { \core\task\manager::adhoc_task_complete($task); } } + + /** + * Mock the clock with an incrementing clock. + * + * @param null|int $starttime + * @return \incrementing_clock + */ + public function mock_clock_with_incrementing( + ?int $starttime = null, + ): \incrementing_clock { + require_once(dirname(__DIR__, 2) . '/testing/classes/incrementing_clock.php'); + $clock = new \incrementing_clock($starttime); + + \core\di::set(\core\clock::class, $clock); + + return $clock; + } + + /** + * Mock the clock with a frozen clock. + * + * @param null|int $time + * @return \frozen_clock + */ + public function mock_clock_with_frozen( + ?int $time = null, + ): \frozen_clock { + require_once(dirname(__DIR__, 2) . '/testing/classes/frozen_clock.php'); + $clock = new \frozen_clock($time); + + \core\di::set(\core\clock::class, $clock); + + return $clock; + } } diff --git a/lib/phpunit/tests/advanced_test.php b/lib/phpunit/tests/advanced_test.php index 530fdd04af9..818bbe51bda 100644 --- a/lib/phpunit/tests/advanced_test.php +++ b/lib/phpunit/tests/advanced_test.php @@ -735,4 +735,69 @@ class advanced_test extends \advanced_testcase { $this->runAdhocTasks(); $this->expectOutputRegex("/Task was run as {$user->id}/"); } + + /** + * Test the incrementing mock clock. + * + * @covers ::mock_clock_with_incrementing + * @covers \incrementing_clock + */ + public function test_mock_clock_with_incrementing(): void { + $standard = \core\di::get(\core\clock::class); + $this->assertInstanceOf(\Psr\Clock\ClockInterface::class, $standard); + $this->assertInstanceOf(\core\clock::class, $standard); + + $newclock = $this->mock_clock_with_incrementing(0); + $mockedclock = \core\di::get(\core\clock::class); + $this->assertInstanceOf(\incrementing_clock::class, $newclock); + $this->assertSame($newclock, $mockedclock); + + // Test the functionality. + $this->assertEquals(0, $mockedclock->now()->getTimestamp()); + $this->assertEquals(1, $newclock->now()->getTimestamp()); + $this->assertEquals(2, $mockedclock->now()->getTimestamp()); + + // Specify a specific start time. + $newclock = $this->mock_clock_with_incrementing(12345); + $mockedclock = \core\di::get(\core\clock::class); + $this->assertSame($newclock, $mockedclock); + + $this->assertEquals(12345, $mockedclock->now()->getTimestamp()); + $this->assertEquals(12346, $newclock->now()->getTimestamp()); + $this->assertEquals(12347, $mockedclock->now()->getTimestamp()); + + $this->assertEquals($newclock->time, $mockedclock->now()->getTimestamp()); + } + + /** + * Test the incrementing mock clock. + * + * @covers ::mock_clock_with_frozen + * @covers \frozen_clock + */ + public function test_mock_clock_with_frozen(): void { + $standard = \core\di::get(\core\clock::class); + $this->assertInstanceOf(\Psr\Clock\ClockInterface::class, $standard); + $this->assertInstanceOf(\core\clock::class, $standard); + + $newclock = $this->mock_clock_with_frozen(0); + $mockedclock = \core\di::get(\core\clock::class); + $this->assertInstanceOf(\frozen_clock::class, $newclock); + $this->assertSame($newclock, $mockedclock); + + // Test the functionality. + $initialtime = $mockedclock->now()->getTimestamp(); + $this->assertEquals($initialtime, $newclock->now()->getTimestamp()); + $this->assertEquals($initialtime, $mockedclock->now()->getTimestamp()); + + // Specify a specific start time. + $newclock = $this->mock_clock_with_frozen(12345); + $mockedclock = \core\di::get(\core\clock::class); + $this->assertSame($newclock, $mockedclock); + + $initialtime = $mockedclock->now(); + $this->assertEquals($initialtime, $mockedclock->now()); + $this->assertEquals($initialtime, $newclock->now()); + $this->assertEquals($initialtime, $mockedclock->now()); + } } diff --git a/lib/psr/clock/LICENSE b/lib/psr/clock/LICENSE new file mode 100644 index 00000000000..be683421232 --- /dev/null +++ b/lib/psr/clock/LICENSE @@ -0,0 +1,19 @@ +Copyright (c) 2017 PHP Framework Interoperability Group + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/lib/psr/clock/README.md b/lib/psr/clock/README.md new file mode 100644 index 00000000000..7dedc2d0661 --- /dev/null +++ b/lib/psr/clock/README.md @@ -0,0 +1,61 @@ +# PSR Clock + +This repository holds the interface for [PSR-20][psr-url]. + +Note that this is not a clock of its own. It is merely an interface that +describes a clock. See the specification for more details. + +## Installation + +```bash +composer require psr/clock +``` + +## Usage + +If you need a clock, you can use the interface like this: + +```php +clock = $clock; + } + + public function doSomething() + { + /** @var DateTimeImmutable $currentDateAndTime */ + $currentDateAndTime = $this->clock->now(); + // do something useful with that information + } +} +``` + +You can then pick one of the [implementations][implementation-url] of the interface to get a clock. + +If you want to implement the interface, you can require this package and +implement `Psr\Clock\ClockInterface` in your code. + +Don't forget to add `psr/clock-implementation` to your `composer.json`s `provide`-section like this: + +```json +{ + "provide": { + "psr/clock-implementation": "1.0" + } +} +``` + +And please read the [specification text][specification-url] for details on the interface. + +[psr-url]: https://www.php-fig.org/psr/psr-20 +[package-url]: https://packagist.org/packages/psr/clock +[implementation-url]: https://packagist.org/providers/psr/clock-implementation +[specification-url]: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-20-clock.md diff --git a/lib/psr/clock/readme_moodle.txt b/lib/psr/clock/readme_moodle.txt new file mode 100644 index 00000000000..476fdc7e433 --- /dev/null +++ b/lib/psr/clock/readme_moodle.txt @@ -0,0 +1,12 @@ +# PSR-20 Clock + +## Installation + +1. Visit https://github.com/php-fig/clock +2. Download the latest release +3. Unzip in this folder +4. Update `thirdpartylibs.xml` +5. Remove any unnecessary files, including: + - Any tests + - CHANGELOG.md + - composer.json diff --git a/lib/psr/clock/src/ClockInterface.php b/lib/psr/clock/src/ClockInterface.php new file mode 100644 index 00000000000..7b6d8d8aae2 --- /dev/null +++ b/lib/psr/clock/src/ClockInterface.php @@ -0,0 +1,13 @@ +. + +/** + * Frozen clock for testing purposes. + * + * @package core + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @property-read \DateTimeImmutable $time The current time of the clock + */ +class frozen_clock implements \core\clock { + /** @var DateTimeImmutable The next time of the clock */ + public DateTimeImmutable $time; + + /** + * Create a new instance of the frozen clock. + * + * @param null|int $time The initial time to use. If not specified, the current time is used. + */ + public function __construct( + ?int $time = null, + ) { + if ($time) { + $this->time = new \DateTimeImmutable("@{$time}"); + } else { + $this->time = new \DateTimeImmutable(); + } + } + + public function now(): \DateTimeImmutable { + return $this->time; + } + + public function time(): int { + return $this->time->getTimestamp(); + } + + /** + * Set the time of the clock. + * + * @param int $time + */ + public function set_to(int $time): void { + $this->time = new \DateTimeImmutable("@{$time}"); + } + + /** + * Bump the time by a number of seconds. + * + * @param int $seconds + */ + public function bump(int $seconds = 1): void { + $this->time = $this->time->modify("+{$seconds} seconds"); + } +} diff --git a/lib/testing/classes/incrementing_clock.php b/lib/testing/classes/incrementing_clock.php new file mode 100644 index 00000000000..e124c7b7d19 --- /dev/null +++ b/lib/testing/classes/incrementing_clock.php @@ -0,0 +1,67 @@ +. + +/** + * Incrementing clock for testing purposes. + * + * @package core + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @property-read int $time The current time of the clock + */ +class incrementing_clock implements \core\clock { + /** @var int The next time of the clock */ + public int $time; + + /** + * Create a new instance of the incrementing clock. + * + * @param null|int $starttime The initial time to use. If not specified, the current time is used. + */ + public function __construct( + ?int $starttime = null, + ) { + $this->time = $starttime ?? time(); + } + + public function now(): \DateTimeImmutable { + return new \DateTimeImmutable('@' . $this->time++); + } + + public function time(): int { + return $this->now()->getTimestamp(); + } + + /** + * Set the time of the clock. + * + * @param int $time + */ + public function set_to(int $time): void { + $this->time = $time; + } + + /** + * Bump the time by a number of seconds. + * + * Note: The act of fetching the time will also bump the time by one second. + * + * @param int $seconds + */ + public function bump(int $seconds = 1): void { + $this->time += $seconds; + } +} diff --git a/lib/testing/tests/clock_test.php b/lib/testing/tests/clock_test.php new file mode 100644 index 00000000000..3f88ca9597c --- /dev/null +++ b/lib/testing/tests/clock_test.php @@ -0,0 +1,106 @@ +. + +namespace core; + +use frozen_clock; +use incrementing_clock; + +/** + * Tests for testing clocks. + * + * @package core + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class clock_test extends \advanced_testcase { + /** + * Test the incrementing mock clock. + * + * @covers \incrementing_clock + */ + public function test_clock_with_incrementing(): void { + require_once(__DIR__ . '/../classes/incrementing_clock.php'); + + $clock = new incrementing_clock(); + $this->assertInstanceOf(\incrementing_clock::class, $clock); + + $initialtime = $clock->now()->getTimestamp(); + + // Test the functionality. + $this->assertEquals($initialtime + 1, $clock->now()->getTimestamp()); + $this->assertEquals($initialtime + 2, $clock->time()); + $this->assertEquals($initialtime + 3, $clock->now()->getTimestamp()); + + // Specify a specific start time. + $clock = new incrementing_clock(12345); + + $this->assertEquals(12345, $clock->now()->getTimestamp()); + $this->assertEquals(12346, $clock->time()); + $this->assertEquals(12347, $clock->now()->getTimestamp()); + + $clock->set_to(12345); + $this->assertEquals(12345, $clock->time()); + $this->assertEquals(12346, $clock->time()); + + $clock->bump(); + $this->assertEquals(12348, $clock->time()); + $clock->bump(); + $this->assertEquals(12350, $clock->time()); + $clock->bump(5); + $this->assertEquals(12356, $clock->time()); + } + + /** + * Test the incrementing mock clock. + * + * @covers \frozen_clock + */ + public function test_mock_clock_with_frozen(): void { + require_once(__DIR__ . '/../classes/frozen_clock.php'); + + $clock = new frozen_clock(); + + // Test the functionality. + $initialtime = $clock->now()->getTimestamp(); + $this->assertEquals($initialtime, $clock->now()->getTimestamp()); + $this->assertEquals($initialtime, $clock->now()->getTimestamp()); + $this->assertEquals($initialtime, $clock->now()->getTimestamp()); + $this->assertEquals($initialtime, $clock->time()); + + // Specify a specific start time. + $clock = new frozen_clock(12345); + + $initialtime = $clock->now(); + $this->assertEquals($initialtime, $clock->now()); + $this->assertEquals($initialtime, $clock->now()); + $this->assertEquals($initialtime, $clock->now()); + + $clock->set_to(12345); + $this->assertEquals(12345, $clock->now()->getTimestamp()); + $this->assertEquals(12345, $clock->now()->getTimestamp()); + $this->assertEquals(12345, $clock->now()->getTimestamp()); + + $this->assertEquals(12345, $clock->time()); + + $clock->bump(); + $this->assertEquals(12346, $clock->time()); + $clock->bump(); + $this->assertEquals(12347, $clock->time()); + $clock->bump(5); + $this->assertEquals(12352, $clock->time()); + } +} diff --git a/lib/tests/system_clock_test.php b/lib/tests/system_clock_test.php new file mode 100644 index 00000000000..c8f5555303d --- /dev/null +++ b/lib/tests/system_clock_test.php @@ -0,0 +1,37 @@ +. + +namespace core; + +/** + * Tests for the standard ClockInterface implementation. + * + * @package core + * @category test + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core\system_clock + */ +final class system_clock_test extends \advanced_testcase { + public function test_now(): void { + $starttime = time(); + + $clock = new system_clock(); + $now = $clock->now(); + $this->assertInstanceOf(\DateTimeImmutable::class, $now); + $this->assertGreaterThanOrEqual($starttime, $now->getTimestamp()); + } +} diff --git a/lib/thirdpartylibs.xml b/lib/thirdpartylibs.xml index e96b97ac564..24cb3d8432c 100644 --- a/lib/thirdpartylibs.xml +++ b/lib/thirdpartylibs.xml @@ -612,6 +612,14 @@ All rights reserved. MIT https://github.com/php-fig/container + + psr/clock + clock + Clock Interface (PHP FIG PSR-20). + 1.0.0 + MIT + https://github.com/php-fig/clock + psr/http-client http-client From 1eef3605cc8bf5f2656fd7790a7d7c082ce82fda Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 6 Feb 2024 15:40:30 +0800 Subject: [PATCH 2/2] MDL-80838 mod_forum: Use \core\clock for forum generators --- mod/forum/tests/externallib_test.php | 20 ++-- mod/forum/tests/generator/lib.php | 39 ++++--- mod/forum/tests/generator_test.php | 148 ++++++++++++++++++++------- 3 files changed, 149 insertions(+), 58 deletions(-) diff --git a/mod/forum/tests/externallib_test.php b/mod/forum/tests/externallib_test.php index 786f350c8cc..8a1df829d43 100644 --- a/mod/forum/tests/externallib_test.php +++ b/mod/forum/tests/externallib_test.php @@ -35,7 +35,7 @@ require_once($CFG->dirroot . '/mod/forum/lib.php'); * @copyright 2012 Mark Nelson * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class externallib_test extends externallib_advanced_testcase { +final class externallib_test extends externallib_advanced_testcase { /** * Tests set up @@ -1056,6 +1056,8 @@ class externallib_test extends externallib_advanced_testcase { $this->resetAfterTest(true); + $clock = $this->mock_clock_with_frozen(); + // Set the CFG variable to allow track forums. $CFG->forum_trackreadposts = true; @@ -1105,7 +1107,7 @@ class externallib_test extends externallib_advanced_testcase { $record->userid = $user1->id; $record->forum = $forum1->id; $discussion1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); - sleep(1); + $clock->bump(); // Add three replies to the discussion 1 from different users. $record = new \stdClass(); @@ -1113,16 +1115,16 @@ class externallib_test extends externallib_advanced_testcase { $record->parent = $discussion1->firstpost; $record->userid = $user2->id; $discussion1reply1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); - sleep(1); + $clock->bump(); $record->parent = $discussion1reply1->id; $record->userid = $user3->id; $discussion1reply2 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); - sleep(1); + $clock->bump(); $record->userid = $user4->id; $discussion1reply3 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); - sleep(1); + $clock->bump(); // Create discussion2. $record2 = new \stdClass(); @@ -1130,7 +1132,7 @@ class externallib_test extends externallib_advanced_testcase { $record2->userid = $user1->id; $record2->forum = $forum1->id; $discussion2 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record2); - sleep(1); + $clock->bump(); // Add one reply to the discussion 2. $record2 = new \stdClass(); @@ -1138,7 +1140,7 @@ class externallib_test extends externallib_advanced_testcase { $record2->parent = $discussion2->firstpost; $record2->userid = $user2->id; $discussion2reply1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record2); - sleep(1); + $clock->bump(); // Create discussion 3. $record3 = new \stdClass(); @@ -1146,7 +1148,7 @@ class externallib_test extends externallib_advanced_testcase { $record3->userid = $user1->id; $record3->forum = $forum1->id; $discussion3 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record3); - sleep(1); + $clock->bump(); // Add two replies to the discussion 3. $record3 = new \stdClass(); @@ -1154,7 +1156,7 @@ class externallib_test extends externallib_advanced_testcase { $record3->parent = $discussion3->firstpost; $record3->userid = $user2->id; $discussion3reply1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record3); - sleep(1); + $clock->bump(); $record3->parent = $discussion3reply1->id; $record3->userid = $user3->id; diff --git a/mod/forum/tests/generator/lib.php b/mod/forum/tests/generator/lib.php index 2a078c1b9a4..de276702cc5 100644 --- a/mod/forum/tests/generator/lib.php +++ b/mod/forum/tests/generator/lib.php @@ -14,17 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * mod_forum data generator - * - * @package mod_forum - * @category test - * @copyright 2012 Petr Skoda {@link http://skodak.org} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); - /** * Forum module data generator class @@ -35,7 +24,6 @@ defined('MOODLE_INTERNAL') || die(); * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class mod_forum_generator extends testing_module_generator { - /** * @var int keep track of how many forum discussions have been created. */ @@ -51,6 +39,15 @@ class mod_forum_generator extends testing_module_generator { */ protected $forumsubscriptionscount = 0; + /** + * Get the clock implementation to use when generating data. + * + * @return \core\clock + */ + protected function get_clock(): \core\clock { + return \core\di::get(\core\clock::class); + } + /** * To be called from data reset code only, * do not use in tests. @@ -188,8 +185,14 @@ class mod_forum_generator extends testing_module_generator { $record['mailnow'] = "0"; } + if (!isset($record['timecreated'])) { + $record['timecreated'] = $this->get_clock()->now()->getTimestamp(); + } + if (isset($record['timemodified'])) { $timemodified = $record['timemodified']; + } else { + $timemodified = $record['timecreated']; } if (!isset($record['pinned'])) { @@ -276,11 +279,19 @@ class mod_forum_generator extends testing_module_generator { } if (!isset($record['created'])) { - $record['created'] = $time; + // If we are using the system clock, then revert to the time + count approach. + // Unfortunately a lot of Forum code relies on things not happening at the same time. + // See MDL-80838 for more information on this issue. + + if ($this->get_clock() instanceof \core\system_clock) { + $record['created'] = $time; + } else { + $record['created'] = $this->get_clock()->now()->getTimestamp(); + } } if (!isset($record['modified'])) { - $record['modified'] = $time; + $record['modified'] = $record['created']; } if (!isset($record['mailed'])) { diff --git a/mod/forum/tests/generator_test.php b/mod/forum/tests/generator_test.php index ba190831095..daf9c01009d 100644 --- a/mod/forum/tests/generator_test.php +++ b/mod/forum/tests/generator_test.php @@ -20,12 +20,12 @@ namespace mod_forum; * PHPUnit data generator testcase * * @package mod_forum - * @category phpunit + * @category test * @copyright 2012 Petr Skoda {@link http://skodak.org} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \mod_forum_generator */ -class generator_test extends \advanced_testcase { - +final class generator_test extends \advanced_testcase { public function setUp(): void { // We must clear the subscription caches. This has to be done both before each test, and after in case of other // tests using these functions. @@ -38,7 +38,7 @@ class generator_test extends \advanced_testcase { \mod_forum\subscriptions::reset_forum_cache(); } - public function test_generator() { + public function test_generator(): void { global $DB; $this->resetAfterTest(true); @@ -52,9 +52,9 @@ class generator_test extends \advanced_testcase { $this->assertInstanceOf('mod_forum_generator', $generator); $this->assertEquals('forum', $generator->get_modulename()); - $generator->create_instance(array('course'=>$course->id)); - $generator->create_instance(array('course'=>$course->id)); - $forum = $generator->create_instance(array('course'=>$course->id)); + $generator->create_instance(['course' => $course->id]); + $generator->create_instance(['course' => $course->id]); + $forum = $generator->create_instance(['course' => $course->id]); $this->assertEquals(3, $DB->count_records('forum')); $cm = get_coursemodule_from_instance('forum', $forum->id); @@ -65,9 +65,12 @@ class generator_test extends \advanced_testcase { $context = \context_module::instance($cm->id); $this->assertEquals($forum->cmid, $context->instanceid); - // test gradebook integration using low level DB access - DO NOT USE IN PLUGIN CODE! - $forum = $generator->create_instance(array('course'=>$course->id, 'assessed'=>1, 'scale'=>100)); - $gitem = $DB->get_record('grade_items', array('courseid'=>$course->id, 'itemtype'=>'mod', 'itemmodule'=>'forum', 'iteminstance'=>$forum->id)); + // Test gradebook integration using low level DB access - DO NOT USE IN PLUGIN CODE. + $forum = $generator->create_instance(['course' => $course->id, 'assessed' => 1, 'scale' => 100]); + $gitem = $DB->get_record( + 'grade_items', + ['courseid' => $course->id, 'itemtype' => 'mod', 'itemmodule' => 'forum', 'iteminstance' => $forum->id] + ); $this->assertNotEmpty($gitem); $this->assertEquals(100, $gitem->grademax); $this->assertEquals(0, $gitem->grademin); @@ -77,7 +80,7 @@ class generator_test extends \advanced_testcase { /** * Test create_discussion. */ - public function test_create_discussion() { + public function test_create_discussion(): void { global $DB; $this->resetAfterTest(true); @@ -94,7 +97,7 @@ class generator_test extends \advanced_testcase { $forum = self::getDataGenerator()->create_module('forum', $record); // Add a few discussions. - $record = array(); + $record = []; $record['course'] = $course->id; $record['forum'] = $forum->id; $record['userid'] = $user->id; @@ -105,24 +108,29 @@ class generator_test extends \advanced_testcase { self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); // Check the discussions were correctly created. - $this->assertEquals(3, $DB->count_records_select('forum_discussions', 'forum = :forum', - array('forum' => $forum->id))); + $this->assertEquals(3, $DB->count_records_select( + 'forum_discussions', + 'forum = :forum', + ['forum' => $forum->id] + )); - $record['tags'] = array('Cats', 'mice'); + $record['tags'] = ['Cats', 'mice']; $record = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record); - $this->assertEquals(array('Cats', 'mice'), - array_values(\core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $record->firstpost))); + $this->assertEquals( + ['Cats', 'mice'], + array_values(\core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $record->firstpost)) + ); } /** * Test create_post. */ - public function test_create_post() { + public function test_create_post(): void { global $DB; $this->resetAfterTest(true); - // Create a bunch of users + // Create a bunch of users. $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); $user3 = self::getDataGenerator()->create_user(); @@ -153,21 +161,26 @@ class generator_test extends \advanced_testcase { // Check the posts were correctly created, remember, when creating a discussion a post // is generated as well, so we should have 4 posts, not 3. - $this->assertEquals(4, $DB->count_records_select('forum_posts', 'discussion = :discussion', - array('discussion' => $discussion->id))); + $this->assertEquals(4, $DB->count_records_select( + 'forum_posts', + 'discussion = :discussion', + ['discussion' => $discussion->id] + )); - $record->tags = array('Cats', 'mice'); + $record->tags = ['Cats', 'mice']; $record = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record); - $this->assertEquals(array('Cats', 'mice'), - array_values(\core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $record->id))); + $this->assertEquals( + ['Cats', 'mice'], + array_values(\core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $record->id)) + ); } - public function test_create_content() { + public function test_create_content(): void { global $DB; $this->resetAfterTest(true); - // Create a bunch of users + // Create a bunch of users. $user1 = self::getDataGenerator()->create_user(); $user2 = self::getDataGenerator()->create_user(); $user3 = self::getDataGenerator()->create_user(); @@ -177,22 +190,22 @@ class generator_test extends \advanced_testcase { // Create course and forum. $course = self::getDataGenerator()->create_course(); - $forum = self::getDataGenerator()->create_module('forum', array('course' => $course)); + $forum = self::getDataGenerator()->create_module('forum', ['course' => $course]); $generator = self::getDataGenerator()->get_plugin_generator('mod_forum'); // This should create discussion. $post1 = $generator->create_content($forum); // This should create posts in the discussion. - $post2 = $generator->create_content($forum, array('parent' => $post1->id)); - $post3 = $generator->create_content($forum, array('discussion' => $post1->discussion)); + $post2 = $generator->create_content($forum, ['parent' => $post1->id]); + $post3 = $generator->create_content($forum, ['discussion' => $post1->discussion]); // This should create posts answering another post. - $post4 = $generator->create_content($forum, array('parent' => $post2->id)); + $post4 = $generator->create_content($forum, ['parent' => $post2->id]); // This should create post with tags. - $post5 = $generator->create_content($forum, array('parent' => $post2->id, 'tags' => array('Cats', 'mice'))); + $post5 = $generator->create_content($forum, ['parent' => $post2->id, 'tags' => ['Cats', 'mice']]); - $discussionrecords = $DB->get_records('forum_discussions', array('forum' => $forum->id)); + $discussionrecords = $DB->get_records('forum_discussions', ['forum' => $forum->id]); $postrecords = $DB->get_records('forum_posts'); - $postrecords2 = $DB->get_records('forum_posts', array('discussion' => $post1->discussion)); + $postrecords2 = $DB->get_records('forum_posts', ['discussion' => $post1->discussion]); $this->assertEquals(1, count($discussionrecords)); $this->assertEquals(5, count($postrecords)); $this->assertEquals(5, count($postrecords2)); @@ -201,7 +214,72 @@ class generator_test extends \advanced_testcase { $this->assertEquals($post1->id, $postrecords[$post3->id]->parent); $this->assertEquals($post2->id, $postrecords[$post4->id]->parent); - $this->assertEquals(array('Cats', 'mice'), - array_values(\core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $post5->id))); + $this->assertEquals( + ['Cats', 'mice'], + array_values(\core_tag_tag::get_item_tags_array('mod_forum', 'forum_posts', $post5->id)) + ); + } + + public function test_create_post_time_system(): void { + $this->resetAfterTest(true); + + $user = self::getDataGenerator()->create_user(); + $course = $this->getDataGenerator()->create_course(); + $forum = self::getDataGenerator()->create_module('forum', (object) [ + 'course' => $course->id, + ]); + + $starttime = time(); + + // Add a discussion. + $discussion = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion((object) [ + 'course' => $course->id, + 'forum' => $forum->id, + 'userid' => $user->id, + ]); + + // Add a post. + $post = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post((object) [ + 'discussion' => $discussion->id, + 'userid' => $user->id, + ]); + + $this->assertGreaterThanOrEqual($starttime, $discussion->timemodified); + $this->assertGreaterThanOrEqual($starttime, $post->created); + + // The fallback behavior is to add the number of created posts to the current time to avoid duplicates. + $this->assertLessThanOrEqual(time() + 1, $discussion->timemodified); + $this->assertLessThanOrEqual(time() + 2, $post->created); + } + + public function test_create_post_time_frozen(): void { + $this->resetAfterTest(true); + + $clock = $this->mock_clock_with_frozen(100); + + $user = self::getDataGenerator()->create_user(); + $course = $this->getDataGenerator()->create_course(); + $forum = self::getDataGenerator()->create_module('forum', (object) [ + 'course' => $course->id, + ]); + + $starttime = time(); + + // Add a discussion. + $discussion = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion((object) [ + 'course' => $course->id, + 'forum' => $forum->id, + 'userid' => $user->id, + ]); + $this->assertEquals(100, $discussion->timemodified); + + // Add a post. + $clock->set_to(200); + $post = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post((object) [ + 'discussion' => $discussion->id, + 'userid' => $user->id, + ]); + + $this->assertEquals(200, $post->created); } }