Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

auth_oidc: fix setcreds function #1361

Closed
wants to merge 1 commit into from

Conversation

alexmorrisnz
Copy link
Contributor

microsoft/moodle-auth_oidc#43 added the $scope parameter which wasn't optional, this breaks unit tests as seen below:

Moodle 3.9.1+ (Build: 20200814), b21f40b26d12dcfd13017c21827b5353370588a5
Php: 7.2.31.1.18.04.1.1, pgsql: 10.14 (Ubuntu 10.14-0ubuntu0.18.04.1), OS: Linux 5.4.0-45-generic x86_64
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

...........E...........                                           23 / 23 (100%)

Time: 8.87 seconds, Memory: 565.00 MB

There was 1 error:

1) auth_oidc_oidcclient_testcase::test_creds_getters_and_setters
ArgumentCountError: Too few arguments to function auth_oidc\oidcclient::setcreds(), 4 passed in /<REMOVED>/auth/oidc/tests/oidcclient_test.php on line 58 and exactly 5 expected

/<REMOVED>/auth/oidc/classes/oidcclient.php:62
/<REMOVED>/auth/oidc/tests/oidcclient_test.php:58
/<REMOVED>/lib/phpunit/classes/advanced_testcase.php:80

To re-run:
 vendor/bin/phpunit "auth_oidc_oidcclient_testcase" auth/oidc/tests/oidcclient_test.php

ERRORS!
Tests: 23, Assertions: 65, Errors: 1.

On these lines https://github.com/microsoft/moodle-auth_oidc/blob/master/classes/oidcclient.php#L66,L71 $resource and $scope are optional because of the empty checks, so I made them optional in the function definition. Unit tests now pass with this patch.

@weilai-irl weilai-irl self-assigned this Nov 11, 2021
@weilai-irl weilai-irl added this to the Release 2021-12 milestone Nov 11, 2021
@weilai-irl
Copy link
Collaborator

Hi @alexmorrisnz,

The pull request has been included in the latest release (3.9.8, 3.10.5 and 3.11.2).

Thank you for your contribution.

Regards,
Lai

@weilai-irl weilai-irl closed this Dec 7, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants