jmcgettrick / MoodleDirectV2

Turnitin Moodle Direct version 2.
24 stars 24 forks source link

Plugin breaks Totara 2.6 Unit tests #480

Closed mattporritt closed 9 years ago

mattporritt commented 9 years ago

Installing version 2015040106 of this plugin in Totara 2.6 causes a unit test failure.

As part of it's install the direct v2 plugin inserts a scale in the mdl_scale table in the database. Totara has a unit test that preloads this table with some scales to test against. When the plugin is installed a primary key collision happens when the Totara unit test does its initial setup.

One solution would be to not create the scale for the direct v2 plugin if the setup is called by phpunit. You could argue that this is a Totara LMS issue, as their unit test makes the assumption about the scale table being empty, but I'm raising it here as well.

The applicable test is: columns_test totara/reportbuilder/tests/column_test.php

danmarsden commented 9 years ago

I would definitely argue this is a TotaraLMS issue - I'm not sure if Turnitin should care too much about unit tests in non-standard Moodle code.

danmarsden commented 9 years ago

I would also be surprised to hear if there were many Totara installations using Turnitin, or has this only occurred on your development machine?

mattporritt commented 9 years ago

Hi Dan, Agreed, the fact that the Totara unit test makes assumptions about an auto-increment id is not great. I've also logged a bug via the partner support channel for Turnitin.

ATM, it's only on my dev instance, we have a client evaluating it.

mattporritt commented 9 years ago

FYI: Feedback from Totara is they won't fix this.

jmcgettrick commented 9 years ago

Hi, Matt. Dan's right about this, I don't think it's a priority for us although it looks to be a straightforward fix. The settings.php page calls get_scales_menu() which if it finds no scales creates a default one. Simply adding a check to see if any scales exist before calling get_scales_menu() would work although that is somewhat duplicating database calls. If you want to make a pull request to develop with that I'll merge it in. Cheers (and thanks as well @danmarsden) John