Skip to content

Commit

Permalink
fixup! MDL-xxxxx phpunit: Ensure all data providers are public and st…
Browse files Browse the repository at this point in the history
…atic
  • Loading branch information
stronk7 committed May 17, 2024
1 parent 1e4aa96 commit ae7c3c5
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 159 deletions.
26 changes: 13 additions & 13 deletions analytics/tests/manager_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public function test_validate_models_declaration() {
$this->resetAfterTest();

// This is expected to run without an exception.
$models = $this->load_models_from_fixture_file('no_teaching');
$models = self::load_models_from_fixture_file('no_teaching');
\core_analytics\manager::validate_models_declaration($models);
}

Expand All @@ -217,31 +217,31 @@ public function test_validate_models_declaration_exceptions(array $models, strin
public static function validate_models_declaration_exceptions_provider() {
return [
'missing_target' => [
$this->load_models_from_fixture_file('missing_target'),
self::load_models_from_fixture_file('missing_target'),
'Missing target declaration',
],
'invalid_target' => [
$this->load_models_from_fixture_file('invalid_target'),
self::load_models_from_fixture_file('invalid_target'),
'Invalid target classname',
],
'missing_indicators' => [
$this->load_models_from_fixture_file('missing_indicators'),
self::load_models_from_fixture_file('missing_indicators'),
'Missing indicators declaration',
],
'invalid_indicators' => [
$this->load_models_from_fixture_file('invalid_indicators'),
self::load_models_from_fixture_file('invalid_indicators'),
'Invalid indicator classname',
],
'invalid_time_splitting' => [
$this->load_models_from_fixture_file('invalid_time_splitting'),
self::load_models_from_fixture_file('invalid_time_splitting'),
'Invalid time splitting classname',
],
'invalid_time_splitting_fq' => [
$this->load_models_from_fixture_file('invalid_time_splitting_fq'),
self::load_models_from_fixture_file('invalid_time_splitting_fq'),
'Expecting fully qualified time splitting classname',
],
'invalid_enabled' => [
$this->load_models_from_fixture_file('invalid_enabled'),
self::load_models_from_fixture_file('invalid_enabled'),
'Cannot enable a model without time splitting method specified',
],
];
Expand All @@ -253,7 +253,7 @@ public static function validate_models_declaration_exceptions_provider() {
* @param string $filename
* @return array
*/
protected function load_models_from_fixture_file(string $filename) {
protected static function load_models_from_fixture_file(string $filename) {
global $CFG;

$models = null;
Expand Down Expand Up @@ -430,9 +430,9 @@ public function test_get_time_splitting_methods() {
*/
public function test_model_declaration_identifier() {

$noteaching1 = $this->load_models_from_fixture_file('no_teaching');
$noteaching2 = $this->load_models_from_fixture_file('no_teaching');
$noteaching3 = $this->load_models_from_fixture_file('no_teaching');
$noteaching1 = self::load_models_from_fixture_file('no_teaching');
$noteaching2 = self::load_models_from_fixture_file('no_teaching');
$noteaching3 = self::load_models_from_fixture_file('no_teaching');

// Same model declaration should always lead to same identifier.
$this->assertEquals(
Expand Down Expand Up @@ -474,7 +474,7 @@ public function test_model_declaration_identifier() {
public function test_get_declared_target_and_indicators_instances() {
$this->resetAfterTest();

$definition = $this->load_models_from_fixture_file('no_teaching');
$definition = self::load_models_from_fixture_file('no_teaching');

list($target, $indicators) = \core_analytics\manager::get_declared_target_and_indicators_instances($definition[0]);

Expand Down
116 changes: 62 additions & 54 deletions auth/lti/tests/auth_test.php

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions backup/util/dbops/tests/restore_dbops_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public static function precheck_user_provider() {

foreach ($emailmultiplier as $emailk => $email) {
// Get the related cases.
$cases = $this->precheck_user_cases($email);
$cases = self::precheck_user_cases($email);
// Rename them (keys).
foreach ($cases as $key => $case) {
$providercases[$key . ' - ' . $emailk] = $case;
Expand All @@ -150,7 +150,7 @@ public static function precheck_user_provider() {
*
* @param string $email
*/
private function precheck_user_cases($email) {
private static function precheck_user_cases($email) {
global $CFG;

$baseuserarr = [
Expand Down
2 changes: 1 addition & 1 deletion cache/tests/cache_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -1568,7 +1568,7 @@ public static function ttl_and_static_acceleration_or_not(): array {
*/
public static function ttl_and_simple_data_or_not(): array {
// Same values as for ttl and static acceleration (two booleans).
return $this->ttl_and_static_acceleration_or_not();
return self::ttl_and_static_acceleration_or_not();
}

/**
Expand Down
48 changes: 24 additions & 24 deletions course/format/tests/stateactions_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -270,27 +270,27 @@ public function test_get_state(
*/
public static function get_state_provider(): array {
return array_merge(
$this->course_state_provider('weeks'),
$this->course_state_provider('topics'),
$this->course_state_provider('social'),
$this->section_state_provider('weeks', 'admin'),
$this->section_state_provider('weeks', 'editingteacher'),
$this->section_state_provider('weeks', 'student'),
$this->section_state_provider('topics', 'admin'),
$this->section_state_provider('topics', 'editingteacher'),
$this->section_state_provider('topics', 'student'),
$this->section_state_provider('social', 'admin'),
$this->section_state_provider('social', 'editingteacher'),
$this->section_state_provider('social', 'student'),
$this->cm_state_provider('weeks', 'admin'),
$this->cm_state_provider('weeks', 'editingteacher'),
$this->cm_state_provider('weeks', 'student'),
$this->cm_state_provider('topics', 'admin'),
$this->cm_state_provider('topics', 'editingteacher'),
$this->cm_state_provider('topics', 'student'),
$this->cm_state_provider('social', 'admin'),
$this->cm_state_provider('social', 'editingteacher'),
$this->cm_state_provider('social', 'student'),
self::course_state_provider('weeks'),
self::course_state_provider('topics'),
self::course_state_provider('social'),
self::section_state_provider('weeks', 'admin'),
self::section_state_provider('weeks', 'editingteacher'),
self::section_state_provider('weeks', 'student'),
self::section_state_provider('topics', 'admin'),
self::section_state_provider('topics', 'editingteacher'),
self::section_state_provider('topics', 'student'),
self::section_state_provider('social', 'admin'),
self::section_state_provider('social', 'editingteacher'),
self::section_state_provider('social', 'student'),
self::cm_state_provider('weeks', 'admin'),
self::cm_state_provider('weeks', 'editingteacher'),
self::cm_state_provider('weeks', 'student'),
self::cm_state_provider('topics', 'admin'),
self::cm_state_provider('topics', 'editingteacher'),
self::cm_state_provider('topics', 'student'),
self::cm_state_provider('social', 'admin'),
self::cm_state_provider('social', 'editingteacher'),
self::cm_state_provider('social', 'student'),
);
}

Expand All @@ -300,7 +300,7 @@ public static function get_state_provider(): array {
* @param string $format the course format
* @return array the testing scenarios
*/
public function course_state_provider(string $format): array {
public static function course_state_provider(string $format): array {
$expectedexception = ($format === 'social');
return [
// Tests for course_state.
Expand Down Expand Up @@ -356,7 +356,7 @@ public function course_state_provider(string $format): array {
* @param string $role the user role
* @return array the testing scenarios
*/
public function section_state_provider(string $format, string $role): array {
public static function section_state_provider(string $format, string $role): array {

// Social format will raise an exception and debug messages because it does not
// use sections and it does not provide a renderer.
Expand Down Expand Up @@ -485,7 +485,7 @@ public function section_state_provider(string $format, string $role): array {
* @param string $role the user role
* @return array the testing scenarios
*/
public function cm_state_provider(string $format, string $role): array {
public static function cm_state_provider(string $format, string $role): array {

// All sections and cms that the user can access to.
$usersections = ['section0', 'section1', 'section2', 'section3'];
Expand Down
23 changes: 15 additions & 8 deletions enrol/lti/tests/local/ltiadvantage/entity/migration_claim_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,20 @@ protected function get_stub_legacy_consumer_repo() {
* @param string $clientid string id of the client.
* @param string $exp expiry time.
* @param string $nonce nonce.
* @param legacy_consumer_repository $legacyconsumerrepo legacy consumer repo instance.
* @param string $legacyrepotype use legacy_consumer_repository ('legacy') or stub ('stub') instance.
* @param array $expected array containing expectation data.
* @covers ::__construct
*/
public function test_migration_claim(array $migrationclaimdata, string $deploymentid, string $platform,
string $clientid, string $exp, string $nonce, legacy_consumer_repository $legacyconsumerrepo,
array $expected) {
string $clientid, string $exp, string $nonce, string $legacyrepotype, array $expected) {

if ($legacyrepotype === 'legacy') {
$legacyconsumerrepo = new legacy_consumer_repository();
} else if ($legacyrepotype === 'stub') {
$legacyconsumerrepo = $this->get_stub_legacy_consumer_repo();
} else {
$this->fail("Incorrect type of legacy consumer repo. Use 'legacy' or 'stub'.");
}

if (!empty($expected['exception'])) {
$this->expectException($expected['exception']);
Expand Down Expand Up @@ -98,7 +105,7 @@ public static function migration_claim_provider(): array {
'clientid' => 'a1b2c3d4',
'exp' => '1622612930',
'nonce' => 'j45j2j5nnjn24544',
new legacy_consumer_repository(),
'legacy',
'expected' => [
'exception' => \coding_exception::class,
'exceptionmessage' => "Missing 'oauth_consumer_key' property in lti1p1 migration claim."
Expand All @@ -114,7 +121,7 @@ public static function migration_claim_provider(): array {
'clientid' => 'a1b2c3d4',
'exp' => '1622612930',
'nonce' => 'j45j2j5nnjn24544',
new legacy_consumer_repository(),
'legacy',
'expected' => [
'exception' => \coding_exception::class,
'exceptionmessage' => "Missing 'oauth_consumer_key_sign' property in lti1p1 migration claim."
Expand All @@ -130,7 +137,7 @@ public static function migration_claim_provider(): array {
'clientid' => 'a1b2c3d4',
'exp' => '1622612930',
'nonce' => 'j45j2j5nnjn24544',
new legacy_consumer_repository(),
'legacy',
'expected' => [
'exception' => \coding_exception::class,
'exceptionmessage' => "Invalid 'oauth_consumer_key_sign' signature in lti1p1 claim."
Expand All @@ -152,7 +159,7 @@ public static function migration_claim_provider(): array {
'clientid' => 'a1b2c3d4',
'exp' => '1622612930',
'nonce' => 'j45j2j5nnjn24544',
$this->get_stub_legacy_consumer_repo(),
'stub',
'expected' => [
'user_id' => null,
'context_id' => null,
Expand Down Expand Up @@ -180,7 +187,7 @@ public static function migration_claim_provider(): array {
'clientid' => 'a1b2c3d4',
'exp' => '1622612930',
'nonce' => 'j45j2j5nnjn24544',
$this->get_stub_legacy_consumer_repo(),
'stub',
'expected' => [
'user_id' => '24',
'context_id' => 'd345b',
Expand Down
8 changes: 6 additions & 2 deletions lib/tests/moodlelib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -4624,6 +4624,10 @@ public static function component_class_callback_multiple_params_provider() {
* @param string $expectedname
*/
public function test_get_callable_name($callable, $expectedname) {
// Providers are static, so they cannot use/return $this. Let's force it here.
if (is_array($callable) && $callable[0] === 'TRY_WITH_THIS') {
$callable[0] = $this;
}
$this->assertSame($expectedname, get_callable_name($callable));
}

Expand All @@ -4650,8 +4654,8 @@ public static function callable_names_provider() {
['my_foobar_class', 'my_foobar_method'],
'my_foobar_class::my_foobar_method',
],
'static_method_of_object' => [
[$this, 'my_foobar_method'],
'static_method_of_this' => [
['TRY_WITH_THIS', 'my_foobar_method'],
'core\moodlelib_test::my_foobar_method',
],
'method_of_object' => [
Expand Down
8 changes: 4 additions & 4 deletions lib/tests/output/icon_system_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public function test_instance_singleton_reset(): void {
*
* @return array
*/
public function icon_system_provider(): array {
public static function icon_system_provider(): array {
return [
'icon_system => icon_system_standard' => [
icon_system::class,
Expand Down Expand Up @@ -212,7 +212,7 @@ public function icon_system_provider(): array {
* @return array
*/
public static function is_valid_subsystem_provider(): array {
return $this->icon_system_provider();
return self::icon_system_provider();
}

/**
Expand All @@ -222,7 +222,7 @@ public static function is_valid_subsystem_provider(): array {
*/
public static function invalid_instance_provider(): array {
return array_filter(
$this->icon_system_provider(),
self::icon_system_provider(),
function($data) {
return !$data[2];
},
Expand All @@ -237,7 +237,7 @@ function($data) {
*/
public static function valid_instance_provider(): array {
return array_filter(
$this->icon_system_provider(),
self::icon_system_provider(),
function($data) {
return $data[2];
},
Expand Down
8 changes: 4 additions & 4 deletions lib/tests/output/mustache_template_source_loader_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public static function load_test_cases() {
'test' => '{{! a comment }}The rest of the template'
]
];
$loader = $this->build_loader_from_static_cache($cache);
$loader = self::build_loader_from_static_cache($cache);

return [
'with comments' => [
Expand Down Expand Up @@ -205,7 +205,7 @@ public static function load_with_dependencies_test_cases() {
'bim' => $bim
]
];
$loader = $this->build_loader_from_static_cache($cache);
$loader = self::build_loader_from_static_cache($cache);

return [
'no template includes w comments' => [
Expand Down Expand Up @@ -388,7 +388,7 @@ public static function scan_template_source_for_dependencies_test_cases() {
'multiline5' => $multiline5,
]
];
$loader = $this->build_loader_from_static_cache($cache);
$loader = self::build_loader_from_static_cache($cache);

return [
'single template include' => [
Expand Down Expand Up @@ -510,7 +510,7 @@ public function test_scan_template_source_for_dependencies($loader, $source, $ex
* @param array $cache A cache of templates
* @return mustache_template_source_loader
*/
private function build_loader_from_static_cache(array $cache): mustache_template_source_loader {
private static function build_loader_from_static_cache(array $cache): mustache_template_source_loader {
return new mustache_template_source_loader(function($component, $name, $themename) use ($cache) {
return $cache[$component][$name];
});
Expand Down

0 comments on commit ae7c3c5

Please sign in to comment.