Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Core unit tests are failing if one of the supported qtypes is not in the code base #35

Open
dmitriim opened this issue Feb 27, 2023 · 10 comments

Comments

@dmitriim
Copy link

dmitriim commented Feb 27, 2023

Running core unit tests in Moodle 4.1 while qtype_combined is not in a code base fails with following error.

PHP Warning:  Uncaught require_once(/var/www/moodle41/question/type/combined/renderer.php): failed to open stream: No such file or directory

/var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php:39
/var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php:39
/var/www/moodle41/lib/classes/component.php:137
/var/www/moodle41/lib/classes/component.php:909
/var/www/moodle41/lib/tests/component_test.php:533
/var/www/moodle41/lib/phpunit/classes/advanced_testcase.php:80
phpvfscomposer:///var/www/moodle41/vendor/phpunit/phpunit/phpunit:97

  thrown in /var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php on line 39
PHP Fatal error:  core_component::main(): Failed opening required '/var/www/moodle41/question/type/combined/renderer.php' (include_path='/var/www/moodle41/lib/pear:.:/usr/share/php') in /var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php on line 39

Warning: Uncaught require_once(/var/www/moodle41/question/type/combined/renderer.php): failed to open stream: No such file or directory

/var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php:39
/var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php:39
/var/www/moodle41/lib/classes/component.php:137
/var/www/moodle41/lib/classes/component.php:909
/var/www/moodle41/lib/tests/component_test.php:533
/var/www/moodle41/lib/phpunit/classes/advanced_testcase.php:80
phpvfscomposer:///var/www/moodle41/vendor/phpunit/phpunit/phpunit:97

  thrown in /var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php on line 39

Fatal error: core_component::main(): Failed opening required '/var/www/moodle41/question/type/combined/renderer.php' (include_path='/var/www/moodle41/lib/pear:.:/usr/share/php') in /var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php on line 39

@dmitriim
Copy link
Author

looks like it fails on other not installed qtypes like oumultiresponse:

Fatal error: core_component::main(): Failed opening required '/var/www/moodle41/question/type/oumultiresponse/combinable/renderer.php' (include_path='/var/www/moodle41/lib/pear:.:/usr/share/php') in /var/www/moodle41/mod/quiz/report/answersheets/classes/output/combined/renderer.php on line 41

@dmitriim dmitriim changed the title Unit tests are failing if qtype_combined is not in the code base Unit tests are failing if one of the supported qtypes is not in the code base Feb 28, 2023
dmitriim added a commit to dmitriim/moodle-quiz_answersheets that referenced this issue Feb 28, 2023
@timhunt
Copy link
Member

timhunt commented Feb 28, 2023

This should work ... and, github actions runs the tests for this plugin without the otptional qtypes installed.

So, why is it failing for you but not github actions?

@dmitriim
Copy link
Author

I guess this is because CI runs tests in isolation without running core tests. Try to install the plugin into vanilla moodle and run vendor/bin/phpunit lib/tests/component_test.php.

I think everything was explained in #36 that was closed.

If you don't want to reorganise the code, then the plugin needs to declare dependencies on all plugins it tires to include files from.

Any other ideas about fixing this issue?

@dmitriim dmitriim changed the title Unit tests are failing if one of the supported qtypes is not in the code base Core unit tests are failing if one of the supported qtypes is not in the code base Feb 28, 2023
@timhunt
Copy link
Member

timhunt commented Feb 28, 2023

#36 did not explain anything. At least not in a way I could understand. It asserted what it was doing, and what problem it was trying to solve, but it did not explain why that action achieves the goal. (I expect it is clear in your head, having just diagnosed the problem, but I am not telepathic. I am, however, busy and tired, so please make it easy for me.)

@dmitriim
Copy link
Author

No worries, I'll provide more details.
test_get_component_classes_in_namespace tries to load all classes for all plugins in output namespace here https://github.com/moodle/moodle/blob/master/lib/tests/component_test.php#L535 This at some point runs class_exists function with $autoload set to default true value. So it tries to load a class and as part of the code also tries to include not existing file which triggers PHP to bark with PHP Fatal error.

@dmitriim
Copy link
Author

So as technically all qtypes renderers could be in any folder as in utils::get_question_renderer method we require a renderer file regardless. I think the easiest fix would be to move file around and make core tests happy.

Hope that explains everything now.

@dmitriim
Copy link
Author

Hi @timhunt
I know that you are busy with stuff related to 4.2 release, but is there any chance you can have a look at this one and revaluate #36 ? Thanks!

@timhunt
Copy link
Member

timhunt commented Mar 23, 2023

This is still not very clear to me, so I can't jsut simply merge this.

The problem with moving stuff around is that we might (probalby do) have chagnes in our local codebase that we have not had time to push to github, and know, i don't have time to synch the two repos right now.

Are we sure this is not a bug in test_get_component_classes_in_namespace that should be fixed, rather than us working around it?

Or, if we need a work-around, would smoething like https://moodledev.io/docs/devupdate#developer-tip---handling-changes-to-base-class-names-while-supporting-multiple-moodle-versions be a less invasive (but more horrible) work-around?

@dmitriim
Copy link
Author

I understand your concerns @timhunt

The main issue here is that auto-loaded files in output namespace are trying to include render files that potentially do not exist. So basically we auto-load a class that extends another class that potentially is not exist.

I don't think that it's a bug in test_get_component_classes_in_namespace as the test calls a core function to return all classes in a given namespace.

We can potentially do something like following so all files will remain where they are now. But I'm not sure if it's a good way also.

if (file_exists($CFG->dirroot . '/question/type/oumultiresponse/renderer.php')) {
    class qtype_oumultiresponse_override_renderer extends \qtype_oumultiresponse_renderer {   
       .... 
    }  
}

TomoTsuyuki added a commit to TomoTsuyuki/moodle-quiz_answersheets that referenced this issue Nov 21, 2023
TomoTsuyuki added a commit to TomoTsuyuki/moodle-quiz_answersheets that referenced this issue Nov 22, 2023
@TomoTsuyuki
Copy link

Hi @timhunt ,

I made PR #37

Could you review and merge if it's ok?

dmitriim added a commit to catalyst/moodle-quiz_answersheets that referenced this issue Feb 21, 2024
Issue moodleou#35: Workaround when the class does not exist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants