Skip to content

M5U-34 Added Turnitin Tool Two plugin#370

Draft
lbailey-ucsf wants to merge 1 commit into
UCSFCLE_501_STABLEfrom
M5U-34/UCSFCLE_501/add_turnitintooltwo
Draft

M5U-34 Added Turnitin Tool Two plugin#370
lbailey-ucsf wants to merge 1 commit into
UCSFCLE_501_STABLEfrom
M5U-34/UCSFCLE_501/add_turnitintooltwo

Conversation

@lbailey-ucsf
Copy link
Copy Markdown
Collaborator

No description provided.

@lbailey-ucsf lbailey-ucsf force-pushed the M5U-34/UCSFCLE_501/add_turnitintooltwo branch from 7dbf9e8 to 0d685a5 Compare April 22, 2026 17:34
@lbailey-ucsf
Copy link
Copy Markdown
Collaborator Author

@ctam @mirleu

Curious if this is a change in 5.1 where CI fails on phpunit warnings?

@lbailey-ucsf
Copy link
Copy Markdown
Collaborator Author

lbailey-ucsf commented Apr 22, 2026

@ctam @mirleu

Curious if this is a change in 5.1 where CI fails on phpunit warnings?

I suspect the tests were always non-functional and emitted warnings but our UCSFCLE_501_STABLE branch now includes failOnWarning="true"

How do you think we need to handle this?

@ctam
Copy link
Copy Markdown
Collaborator

ctam commented Apr 22, 2026

I encountered the same type of warnings in Kaltura, and I created an issue on the Kaltura's repo and a PR for the fix.

If the developer is not very responsive, we will have to mirror their repo and apply our fix there with the creation of our own UCSFLCLE_501_STABLE.

@lbailey-ucsf
Copy link
Copy Markdown
Collaborator Author

lbailey-ucsf commented Apr 22, 2026

The problem here is, they put their tests in a sub-directory, tests/unit. But Moodle requires test to be at the root level tests/.

  • If we fork it, we'd have to move tests out of tests/unit and into test/ and edit all 11 failing tests. Ironically, the plugin claims to be be Moodle 5.1 compatible; I suspect the plugin works in M5.1 even with broken unit test semantics.
  • I don't see any evidence that our CI ever ran the tests for this plugin in our UCSFCLE_405_STABLE branch; we could exclude the testsuite altogether with a push.yml edit in our UCSFCLE_501_STABLE branch.
  • I doubt they will restructure their project between now and when we go live. They seem to have a specific testing procedure.
  • We should also consider the fact that the plugin is being retired and replaced with an LTI integration.

@lbailey-ucsf lbailey-ucsf force-pushed the M5U-34/UCSFCLE_501/add_turnitintooltwo branch from 0d685a5 to dbef91c Compare April 24, 2026 16:11
@ctam
Copy link
Copy Markdown
Collaborator

ctam commented Apr 24, 2026

Just to circulate back from the discussion at the code fest, are we going to push for replacing this plugin with their LTI counterpart, @mirleu? We might need to let the Service Team know ahead and have time to plan for it.

@lbailey-ucsf
Copy link
Copy Markdown
Collaborator Author

The tests fail the same way when I try to run them directly; they're just not compatible with Moodle 5.1.

PHPUnit 9 (used by Moodle 4.x) may have allowed test classes to extend advanced_testcase with any class name and the name didn't have to match the filename. But PHPUnit 11 (Moodle 5.1) enforces that the class name in the file must match what it derives from the filename. This plugin's tests were written for PHPUnit 9 conventions and haven't yet been updated for PHPUnit 11.

We need to decide if we want to skip the testsuite, fork-and-fix, or something else.

@lbailey-ucsf lbailey-ucsf force-pushed the M5U-34/UCSFCLE_501/add_turnitintooltwo branch from 54da18a to bf92409 Compare May 6, 2026 18:03
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

Successfully merging this pull request may close these issues.

2 participants