Tests fail on PHP7.4

[Peterburnett]

togglelib::get_toggle_state attempts to access an null as an array. This returned null in PHP7.2, in PHP7.4 it causes the test to fail. I will be submitting a PR for the test shortly.

[gjb2048]

@Peterburnett Thanks for the patch, but it is a solution without understanding the problem as 'toggles' should not be null, thus raising the questions:

1. Why is it null?
2. Can the caller cope with a return of null?

Therefore I need to know please:

1. How to replicate the problem.
2. Call stack trace of the problem, i.e. something like:

error_log('toggles');
$e = new \Exception;
error_log($e->getTraceAsString());

at the point of failure.

And the title is 'Tests fail on PHP7.4', what 'tests' please.

[Peterburnett]

E<h2 class="accesshide">Section</h2><ul class="ctopics bsnewgrid"><li id="section-0" class="section main clearfix" role="region" aria-labelledby="sectionid-146000-title" data-sectionid="0" data-sectionreturnid="0"><div class="left side"></div><div class="content"><h3 id="sectionid-146000-title" class="accesshide">General</h3><div class="section_availability"></div><div class="summary"></div><ul class="section img-text"><li class="activity forum modtype_forum " id="module-144000"><div><div class="mod-indent-outer"><div class="mod-indent"></div><div><div class="activityinstance"><a class="aalink" onclick="" href="https://www.example.com/moodle/mod/forum/view.php?id=144000"><img src="https://www.example.com/moodle/theme/image.php/_s/boost/forum/1/icon" class="iconlarge activityicon" alt="" role="presentation" aria-hidden="true" /><span class="instancename">Announcements<span class="accesshide " > Forum</span></span></a></div></div></div></div></li></ul></div><div class="right side"></div></li></ul><ul class="ctopics topics bsnewgrid row">

Time: 38.42 minutes, Memory: 496.50 MB

There was 1 error:

1) format_topcoll_courseformatrenderer_testcase::test_print_multiple_section_page_horizontal
Trying to access array offset on value of type null

/var/www/site/course/format/topcoll/classes/togglelib.php:78
/var/www/site/course/format/topcoll/renderer.php:1042
/var/www/site/course/format/topcoll/tests/courseformatrenderer_test.php:53
/var/www/site/course/format/topcoll/tests/courseformatrenderer_test.php:276
/var/www/site/lib/phpunit/classes/advanced_testcase.php:80

[gjb2048]

@Peterburnett Ah, ok thanks: https://travis-ci.org/github/gjb2048/moodle-format_topcoll/jobs/721644741#L763-L787

[Peterburnett]

Pulled this from our CI. The init function of the tests sets many renderer settings on instantiation, however the renderer constructor doesnt ever call set_toggles, and it is never called during the init, or the test suite. This means that at reaching the L78 of togglelib, this->toggles is a null value. The decode function seems to handle this OK, whether by accident or intentionally, but I have no idea if that is the intended behaviour. The other side to the coin is of course to patch the tests in a way that sets a correct toggle state, however this patch is still useful in itself as it ensures that incorrect language behaviour cannot occur

[gjb2048]

Ah, but the patch has highlighted that there is an error in the conceptual 'sequence diagram' of the correct state of operation that in this case the tests have failed to establish. Thus if the error was to happen again but operationally and not in the PHP Unit tests then it would be a good thing to indicate that the intended operation of the format as a whole has been broken.

[Peterburnett]

Thats a fair assessment, and I understand where you are coming from there, in that case, perhaps there should be an exception at this point if null, and a modification of the test suite to setup the toggles correctly. I can try my hand at a more complex patch tomorrow, if that is the approach you want to take. Let me know your thoughts, I have a bit of time budget to get this sorted.

[gjb2048]

@Peterburnett Ok, despite the other failures (fix when I have time) -> https://travis-ci.org/github/gjb2048/moodle-format_topcoll/jobs/721665933 - then https://github.com/gjb2048/moodle-format_topcoll/commit/77f6f2f46fed966e55e9fc07cdd2c3dfe81fbf66 solves the issue without adding any more code to the operational element of the format. I'm a 'less is more' person, every decision / thought that allows the code to be minimal is good as that means:

Less cpu instructions. Less maintenance.

This all adds up over time.

[Peterburnett]

OK sure, thanks for working with me on this. In this case, I will close my PR, as it will be superseded by more fundamental changes. Thanks again!

[gjb2048]

@Peterburnett NP.