Fix CI complains #700

Author: dustinhuynh

classes/local/store/s3/admin_settings_aws_region.php

@@ -63,21 +63,21 @@ public function output_html($data, $query='') {
             }
         }
 
-        $inputparams = array(
+        $inputparams = [
             'type' => 'text',
             'list' => $this->get_full_name(),
             'name' => $this->get_full_name(),
             'value' => $data,
             'size' => $this->size,
             'id' => $this->get_id(),
             'class' => 'form-control text-ltr',
-        );
+        ];
 
-        $element = \html_writer::start_tag('div', array('class' => 'form-text defaultsnext'));
+        $element = \html_writer::start_tag('div', ['class' => 'form-text defaultsnext']);
         $element .= \html_writer::empty_tag('input', $inputparams);
-        $element .= \html_writer::start_tag('datalist', array('id' => $this->get_full_name()));
+        $element .= \html_writer::start_tag('datalist', ['id' => $this->get_full_name()]);
         foreach ($options as $option) {
-            $element .= \html_writer::tag('option', $option['label'], array('value' => $option['value']));
+            $element .= \html_writer::tag('option', $option['label'], ['value' => $option['value']]);
         }
         $element .= \html_writer::end_tag('datalist');
         $element .= \html_writer::end_tag('div');

lang/en/tool_objectfs.php

@@ -31,11 +31,11 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  * @covers    \tool_objectfs\check\tagging_migration_status
  */
-class tagging_migration_status_test extends testcase {
+final class tagging_migration_status_test extends testcase {
     /**
      * Tests scenario that returns N/A
      */
-    public function test_get_result_na() {
+    public function test_get_result_na(): void {
         // Regardless if this is disabled, the check should still return a non n/a status.
         $this->enable_filesystem_and_set_tagging(false);
         $check = new tagging_migration_status();
@@ -45,7 +45,7 @@ public function test_get_result_na() {
     /*
      * Test scenario that returns WARNING
      */
-    public function test_get_result_warning() {
+    public function test_get_result_warning(): void {
         // Regardless if this is disabled, the check should still return a non n/a status.
         $this->enable_filesystem_and_set_tagging(false);
 
@@ -60,7 +60,7 @@ public function test_get_result_warning() {
     /*
      * Test scenario that returns OK
      */
-    public function test_get_result_ok() {
+    public function test_get_result_ok(): void {
         // Regardless if this is disabled, the check should still return a non n/a status.
         $this->enable_filesystem_and_set_tagging(false);
 

tests/check/tagging_migration_status_test.php

@@ -30,11 +30,11 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  * @covers    \tool_objectfs\check\tagging_sync_status
  */
-class tagging_sync_status_test extends testcase {
+final class tagging_sync_status_test extends testcase {
     /**
      * Tests scenario that returns N/A
      */
-    public function test_get_result_na() {
+    public function test_get_result_na(): void {
         // Not enabled by default, should return N/A.
         $check = new tagging_sync_status();
         $this->assertEquals(result::NA, $check->get_result()->get_status());
@@ -43,7 +43,7 @@ public function test_get_result_na() {
     /**
      * Test scenario that returns OK
      */
-    public function test_get_result_ok() {
+    public function test_get_result_ok(): void {
         $this->enable_filesystem_and_set_tagging(true);
         $object = $this->create_remote_object();
         tag_manager::mark_object_tag_sync_status($object->contenthash, tag_manager::SYNC_STATUS_COMPLETE);
@@ -56,7 +56,7 @@ public function test_get_result_ok() {
     /**
      * Tests scenario that returns WARNING
      */
-    public function test_get_result_warning() {
+    public function test_get_result_warning(): void {
         $this->enable_filesystem_and_set_tagging(true);
         $object = $this->create_remote_object();
         tag_manager::mark_object_tag_sync_status($object->contenthash, tag_manager::SYNC_STATUS_ERROR);

tests/check/tagging_sync_status_test.php

@@ -31,7 +31,7 @@
  *
  * @covers \tool_objectfs\local\manager
  */
-class manager_test extends \tool_objectfs\tests\testcase {
+final class manager_test extends \tool_objectfs\tests\testcase {
 
     /**
      * Data provider for test_all_extensions_whitelisted().
@@ -57,7 +57,7 @@ public static function all_extensions_whitelisted_provider(): array {
      * @param  bool   $result            Expected result
      * @throws \dml_exception
      */
-    public function test_all_extensions_whitelisted($signingwhitelist, $result) {
+    public function test_all_extensions_whitelisted($signingwhitelist, $result): void {
         if (isset($signingwhitelist)) {
             set_config('signingwhitelist', $signingwhitelist, 'tool_objectfs');
         }
@@ -91,7 +91,7 @@ public static function is_extension_whitelisted_provider(): array {
      * @param  bool    $result            Expected result
      * @throws \dml_exception
      */
-    public function test_is_extension_whitelisted($signingwhitelist, $filename, $result) {
+    public function test_is_extension_whitelisted($signingwhitelist, $filename, $result): void {
         if (isset($signingwhitelist)) {
             set_config('signingwhitelist', $signingwhitelist, 'tool_objectfs');
         }
@@ -129,7 +129,7 @@ public static function get_header_provider(): array {
      * @param  string  $search      What we are searching for
      * @param  bool    $expected    Expected result
      */
-    public function test_get_header($headers, $search, $expected) {
+    public function test_get_header($headers, $search, $expected): void {
         $actual = manager::get_header($headers, $search);
         $this->assertEquals($expected, $actual);
     }

tests/local/manager_test.php

@@ -26,7 +26,7 @@
  * @copyright Catalyst IT
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class checker_test extends \tool_objectfs\tests\testcase {
+final class checker_test extends \tool_objectfs\tests\testcase {
 
     /** @var string $manipulator */
     protected $manipulator = checker::class;
@@ -45,33 +45,34 @@ protected function setUp(): void {
 
     protected function tearDown(): void {
         ob_end_clean();
+        parent::tearDown();
     }
 
-    public function test_checker_get_location_local_if_object_is_local() {
+    public function test_checker_get_location_local_if_object_is_local(): void {
         global $DB;
         $file = $this->create_local_object();
         $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $file->contenthash]);
         $this->assertEquals('string', gettype($location));
         $this->assertEquals(OBJECT_LOCATION_LOCAL, $location);
     }
 
-    public function test_checker_get_location_duplicated_if_object_is_duplicated() {
+    public function test_checker_get_location_duplicated_if_object_is_duplicated(): void {
         global $DB;
         $file = $this->create_duplicated_object();
         $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $file->contenthash]);
         $this->assertEquals('string', gettype($location));
         $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $location);
     }
 
-    public function test_checker_get_location_external_if_object_is_external() {
+    public function test_checker_get_location_external_if_object_is_external(): void {
         global $DB;
         $file = $this->create_remote_object();
         $location = $DB->get_field('tool_objectfs_objects', 'location', ['contenthash' => $file->contenthash]);
         $this->assertEquals('string', gettype($location));
         $this->assertEquals(OBJECT_LOCATION_EXTERNAL, $location);
     }
 
-    public function test_checker_get_candidate_objects_will_not_get_objects() {
+    public function test_checker_get_candidate_objects_will_not_get_objects(): void {
         $localobject = $this->create_local_object('test_checker_get_candidate_objects_will_not_get_objects_local');
         $remoteobject = $this->create_remote_object('test_checker_get_candidate_objects_will_not_get_objects_remote');
         $duplicatedbject = $this->create_duplicated_object('test_checker_get_candidate_objects_will_not_get_objects_duplicated');
@@ -81,15 +82,15 @@ public function test_checker_get_candidate_objects_will_not_get_objects() {
         self::assertFalse($this->objects_contain_hash($duplicatedbject->contenthash));
     }
 
-    public function test_checker_get_candidate_objects_will_get_object() {
+    public function test_checker_get_candidate_objects_will_get_object(): void {
         global $DB;
         $localobject = $this->create_local_object('test_checker_get_candidate_objects_will_get_object');
         $DB->delete_records('tool_objectfs_objects', ['contenthash' => $localobject->contenthash]);
 
         self::assertTrue($this->objects_contain_hash($localobject->contenthash));
     }
 
-    public function test_checker_can_update_object() {
+    public function test_checker_can_update_object(): void {
         global $DB;
         $localobject = $this->create_local_object('test_checker_can_update_object');
         $localobject->id = null;
@@ -102,28 +103,28 @@ public function test_checker_can_update_object() {
         self::assertFalse($this->objects_contain_hash($localobject->contenthash));
     }
 
-    public function test_checker_manipulate_object_method_will_get_correct_location_if_file_is_local() {
+    public function test_checker_manipulate_object_method_will_get_correct_location_if_file_is_local(): void {
         $file = $this->create_local_object();
         $reflection = new \ReflectionMethod(checker::class, "manipulate_object");
         $reflection->setAccessible(true);
         $this->assertEquals(OBJECT_LOCATION_LOCAL, $reflection->invokeArgs($this->checker, [$file]));
     }
 
-    public function test_checker_manipulate_object_method_will_get_correct_location_if_file_is_duplicated() {
+    public function test_checker_manipulate_object_method_will_get_correct_location_if_file_is_duplicated(): void {
         $file = $this->create_duplicated_object();
         $reflection = new \ReflectionMethod(checker::class, "manipulate_object");
         $reflection->setAccessible(true);
         $this->assertEquals(OBJECT_LOCATION_DUPLICATED, $reflection->invokeArgs($this->checker, [$file]));
     }
 
-    public function test_checker_manipulate_object_method_will_get_correct_location_if_file_is_external() {
+    public function test_checker_manipulate_object_method_will_get_correct_location_if_file_is_external(): void {
         $file = $this->create_remote_object();
         $reflection = new \ReflectionMethod(checker::class, "manipulate_object");
         $reflection->setAccessible(true);
         $this->assertEquals(OBJECT_LOCATION_EXTERNAL, $reflection->invokeArgs($this->checker, [$file]));
     }
 
-    public function test_checker_manipulate_object_method_will_get_error_location_on_error_file() {
+    public function test_checker_manipulate_object_method_will_get_error_location_on_error_file(): void {
         $file = $this->create_error_object();
         $reflection = new \ReflectionMethod(checker::class, "manipulate_object");
         $reflection->setAccessible(true);

tests/local/object_manipulator/checker_test.php

@@ -26,7 +26,7 @@
  * @copyright Catalyst IT
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class deleter_test extends \tool_objectfs\tests\testcase {
+final class deleter_test extends \tool_objectfs\tests\testcase {
 
     /** @var string $manipulator */
     protected $manipulator = deleter::class;
@@ -48,6 +48,7 @@ protected function setUp(): void {
 
     protected function tearDown(): void {
         ob_end_clean();
+        parent::tearDown();
     }
 
     /**
@@ -64,28 +65,28 @@ protected function set_deleter_config($key, $value) {
         $this->deleter = new deleter($this->filesystem, $config, $this->logger);
     }
 
-    public function test_deleter_get_candidate_objects_will_get_duplicated_objects() {
+    public function test_deleter_get_candidate_objects_will_get_duplicated_objects(): void {
         $duplicatedbject = $this->create_duplicated_object();
 
         self::assertTrue($this->objects_contain_hash($duplicatedbject->contenthash));
     }
 
-    public function test_deleter_get_candidate_objects_will_not_get_local_or_remote_objects() {
+    public function test_deleter_get_candidate_objects_will_not_get_local_or_remote_objects(): void {
         $localobject = $this->create_local_object();
         $remoteobject = $this->create_remote_object();
 
         self::assertFalse($this->objects_contain_hash($localobject->contenthash));
         self::assertFalse($this->objects_contain_hash($remoteobject->contenthash));
     }
 
-    public function test_deleter_get_candidate_objects_will_not_get_objects_which_havent_been_duplicated_for_consistancy_delay() {
+    public function test_deleter_get_candidate_objects_will_not_get_objects_which_havent_been_duplicated_for_consistancy_delay(): void {
         $duplicatedbject = $this->create_duplicated_object();
         $this->set_deleter_config('consistencydelay', 100);
 
         self::assertFalse($this->objects_contain_hash($duplicatedbject->contenthash));
     }
 
-    public function test_deleter_can_delete_object() {
+    public function test_deleter_can_delete_object(): void {
         global $DB;
         $object = $this->create_duplicated_object();
 
@@ -97,7 +98,7 @@ public function test_deleter_can_delete_object() {
         $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash));
     }
 
-    public function test_deleter_can_handle_local_object() {
+    public function test_deleter_can_handle_local_object(): void {
         global $DB;
         $object = $this->create_local_object();
 
@@ -109,7 +110,7 @@ public function test_deleter_can_handle_local_object() {
         $this->assertFalse($this->is_externally_readable_by_hash($object->contenthash));
     }
 
-    public function test_deleter_can_handle_remote_object() {
+    public function test_deleter_can_handle_remote_object(): void {
         global $DB;
         $object = $this->create_remote_object();
 
@@ -121,7 +122,7 @@ public function test_deleter_can_handle_remote_object() {
         $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash));
     }
 
-    public function test_deleter_will_delete_no_objects_if_deletelocal_disabled() {
+    public function test_deleter_will_delete_no_objects_if_deletelocal_disabled(): void {
         global $DB;
         $object = $this->create_duplicated_object();
         $this->set_deleter_config('deletelocal', 0);
@@ -134,7 +135,7 @@ public function test_deleter_will_delete_no_objects_if_deletelocal_disabled() {
         $this->assertTrue($this->is_externally_readable_by_hash($object->contenthash));
     }
 
-    public function test_deleter_can_delete_multiple_objects() {
+    public function test_deleter_can_delete_multiple_objects(): void {
         global $DB;
         $objects = [];
         for ($i = 0; $i < 5; $i++) {

tests/local/object_manipulator/deleter_test.php

@@ -27,7 +27,7 @@
  * @copyright Catalyst IT
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class orphaner_test extends \tool_objectfs\tests\testcase {
+final class orphaner_test extends \tool_objectfs\tests\testcase {
 
     /** @var string $manipulator */
     protected $manipulator = orphaner::class;
@@ -48,6 +48,7 @@ protected function setUp(): void {
 
     protected function tearDown(): void {
         ob_end_clean();
+        parent::tearDown();
     }
 
     /**
@@ -64,7 +65,7 @@ protected function set_orphaner_config($key, $value) {
         $this->orphaner = new orphaner($this->filesystem, $config, $this->logger);
     }
 
-    public function test_orphaner_can_orphan_files() {
+    public function test_orphaner_can_orphan_files(): void {
         global $DB;
         $this->orphaner->execute([
             $object1 = $this->create_local_object(),
@@ -79,7 +80,7 @@ public function test_orphaner_can_orphan_files() {
         $this->assertEquals(OBJECT_LOCATION_ORPHANED, $location3);
     }
 
-    public function test_orphaner_finds_correct_candidates() {
+    public function test_orphaner_finds_correct_candidates(): void {
         global $DB;
 
         // Initialise the candidate finder.
@@ -110,7 +111,7 @@ public function test_orphaner_finds_correct_candidates() {
         $this->assertCount(0, $objects); // No candidates - only candidate has been orphaned.
     }
 
-    public function test_orphaner_correctly_orphans_provided_files() {
+    public function test_orphaner_correctly_orphans_provided_files(): void {
         global $DB;
         $this->orphaner->execute([
             $object1 = $this->create_local_object(),