search

catalyst / moodle-auth_outage

Planned, graduated user and admin friendly moodle outages
https://moodle.org/plugins/auth_outage
17 stars 33 forks source link

WR#334284 fixing unit tests #235

Closed christina-roperto closed 3 months ago

christina-roperto commented 3 years ago

This fixes issue #236

danmarsden commented 3 years ago

Any chance of a public issue in the tracker that covers what this actually fixes? The commit message is a bit light on detail.

christina-roperto commented 3 years ago

Hi @danmarsden ,

Apologies for the light detail, it's related to an internal WR. This fixes the unit tests on newer moodle which requires assertions. It should be backward compatible (only added assertions) but without CI, I can't say for sure.

The next step for me is to fix the CI but I've been allocated to another project so I won't be working on this right now. What do you think?

danmarsden commented 3 years ago

Even of you could drop a new public issue here in the tracker with a copy of the unit test failures and then add a link to that in your commit it would be better (and quick to do) Thanks!

christina-roperto commented 3 years ago

@danmarsden ,

You are right. I'm very new to this. I've created issue #236 as you suggested. Let me know if I can improve this PR :)

danmarsden commented 3 years ago

awesome - thanks for that - helps with folks using Google that might be looking for a fix.

Haven't had a chance to review the patch - hopefully someone in the AU team is able to follow this through with you?

christina-roperto commented 3 years ago

Apologies for the late reply. I will work on this once I finish the current project and will ask someone is AU team to review it. Thank you :)

danmarsden commented 3 years ago

@christina-roperto do you know if this one is still needed? - what versions needed this?