CTP-4793 moderated marking behat tests #181

opitz · 2025-12-12T16:23:19Z

Added Behat tests for moderated marking according to https://liveuclac-my.sharepoint.com/:x:/g/personal/dtnvgal_ucl_ac_uk/ESRtBFnS-xVPmFJl-YynDLsBRKCXh-zHVUgfq0ydIGWSkw?e=FrVGTS&wdOrigin=TEAMS-MAGLEV.p2p_ns.rwc&wdExp=TEAMS-TREATMENT&wdhostclicktime=1763465662042&web=1

Tests involved Turnitin are dealt with in a separate story.

Copilot

Pull request overview

This PR adds comprehensive Behat test coverage for the double marking with blind marking workflow in the coursework module. The tests verify the complete marking cycle from setup through grade release, ensuring blind marking functionality works correctly with double markers and optional moderation.

Key Changes:

Added 897 lines of Behat test scenarios covering double-blind marking workflows
Implemented 27 new Behat step definitions to support the test scenarios
Added helper methods for managing coursework entities (allocations, submissions, feedbacks, moderations)

Reviewed changes

Copilot reviewed 2 out of 6 changed files in this pull request and generated 5 comments.

File	Description
tests/behat/double_marking_blind.feature	Comprehensive test scenarios for double-blind marking workflow including setup, marker allocation, submission, marking, moderation, and grade release
tests/behat/behat_mod_coursework.php	New step definitions and helper methods to support double-blind marking test scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/behat/behat_mod_coursework.php

andrewhancox

I'm not sure how other people feel but I really don't think we should be creating any more custom behat steps unless we can possibly avoid it - if anything we should be going the other way.

leonstr · 2025-12-15T09:11:29Z

I'm not sure how other people feel but I really don't think we should be creating any more custom behat steps unless we can possibly avoid it - if anything we should be going the other way.

Please could you explain further? The dev docs say Sometimes, you will need to set up data that is specific to your plugin, or perform steps that are specific to your plugin's UI. In this case it may be necessary to write new step definitions – no suggestion these should be avoided.

If we're adding custom steps where there are already steps in core that do the job then I agree we shouldn't do that.

But for setting up some specific state that's not part of the thing being tested, I prefer custom steps to painfully walking through manual clicks. They're quicker to run and quicker to read.

andrewhancox · 2025-12-15T09:23:45Z

I'm not sure how other people feel but I really don't think we should be creating any more custom behat steps unless we can possibly avoid it - if anything we should be going the other way.

Please could you explain further? The dev docs say Sometimes, you will need to set up data that is specific to your plugin, or perform steps that are specific to your plugin's UI. In this case it may be necessary to write new step definitions – no suggestion these should be avoided.

Generators are the appropriate way to do setup and add far less code, most set up done by this plugin should be happening this way.
Custom behat steps that know about the DOM are very rarely necessary and we've got way too many of them already.
I would also say that if it's hard to locate an element to fire a click event, check a value etc. (e.g. the second node of this type with this ID contained in a class on the parent but only if submitted on a wednesday...) then we should just add a data attribute that can be directly targeted (@stuartlamour likes these prefixed with behat_ for clarity).

andrewhancox · 2025-12-15T09:26:16Z

Taking mod_assign as an example, there are some generators here:
mod/assign/tests/generator/behat_mod_assign_generator.php
And one custom step here:
mod/assign/tests/behat/behat_mod_assign.php

stuartlamour · 2025-12-15T13:26:59Z

Yep - regex to get dom elements is fragile as hell, constantly breaks and needs updating.
We own the templates/html so no reason not to just add code for behat.
As @andrewhancox said best practice is to use a naming convention for this - so prefix any ids or data attributes with behat.

e.g.
id="behat-mark"
data-behat-markstage="{{markingstage}}"
data-behat-mark="finalmark"
etc ...

depending on what is appropriate.

That should make writing tests a lot simpler.

Copilot

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/behat/behat_mod_coursework.php

opitz · 2025-12-17T11:20:51Z

@andrewhancox @leonstr
I have created a separate story to refactor Behat steps into generator methods here: https://ucldata.atlassian.net/browse/CTP-5564
Given the fact that this story is on my plate for too long now I like to have it approved as is for now, please.

Copilot

Pull request overview

Copilot reviewed 3 out of 7 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/generator/behat_mod_coursework_generator.php

tests/behat/behat_mod_coursework.php

Copilot

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/behat/double_marking_blind.feature

tests/behat/behat_mod_coursework.php

leonstr

CI checks for this branch currently fail so this at least needs resolving. If any of the current Behat failures are due to bugs in Coursework then those failing scenarios should be moved to pull requests fixing these bugs.

tests/behat/behat_mod_coursework.php

tests/behat/double_marking_blind.feature

andrewhancox · 2026-01-07T09:42:05Z

Am ok if I'm losing the popular vote here but I'm still really uncomfortable with this direction - we're doubling down on weird/bad practice.
We should not be adding more weird functions here. We should be using proper behat generators and using core functions for clicks/form completion - adding behat data attributes to Dom elements if needed.
We're accumulating more technical debt here...

If it's what we need to do in the short/medium term then fair enough but I wanted to reiterate my concerns.

Copilot

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/behat/behat_mod_coursework.php

Copilot

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/behat/behat_mod_coursework.php

Copilot

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/behat/behat_mod_coursework.php

Copilot

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/behat/behat_mod_coursework.php

reamed i_allocate_the_following_markers() to the_following_markers_are_allocated()

…n_marking.feature using existing behat steps to replace there_is_a_blind_marking_coursework()

Copilot

Pull request overview

Copilot reviewed 3 out of 7 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/behat/moderation_marking.feature

opitz · 2026-01-28T10:00:58Z

@leonstr @andrewhancox any chance this can be finalised?

leonstr

Some nitpicks.

leonstr · 2026-01-28T12:42:15Z

tests/behat/behat_mod_coursework.php

+            $userid = $DB->get_field_sql(
+                "SELECT id FROM {user} WHERE firstname = ? AND lastname = ?",
+                [$firstname, $lastname]
+            );


Maybe:

$userid = $DB->get_field('user', 'id', ['firstname' => $firsname, 'lastname' => $lastname]);

leonstr · 2026-01-28T12:54:35Z

tests/behat/double_marking_blind.feature

  I want to perform the full coursework workflow with blind marking.

  Background:
    And the following "roles" exist:


Background should start Given followed by Ands, i.e.:

8 Given the following "roles" exist: ⋮ 83 And there is a course ⋮ 96 And there is a coursework

leonstr · 2026-01-28T12:58:08Z

tests/behat/double_marking_blind.feature

+    # Student has extension so late submission is in time
+    When I am on the "Course 1" "course" page logged in as "student1"
+    And I follow "Coursework 1"
+    When I visit the coursework page


Should be And not When?

leonstr · 2026-01-28T13:00:25Z

tests/behat/double_marking_blind.feature

+    And I follow "Coursework 1"
+    Then I should see "Submission"
+    And I should see "In marking"
+    And I should not see "Edit your submission"


Could use But not And.

leonstr · 2026-01-28T13:03:12Z

tests/behat/moderation_marking.feature

+  I want to perform the full coursework workflow with blind marking.
+
+  Background:
+    And the following "roles" exist:


Background should start Given followed by Ands, i.e.:

7 Background: 8 Given the following "roles" exist: ⋮ 83 And there is a course ⋮ 96 And there is a coursework

leonstr · 2026-01-28T13:04:30Z

tests/behat/moderation_marking.feature

+    Then I should see "Disagreed" in row "2"
+    When I follow "Disagreed"
+    And I wait until the page is ready
+    Then I should see "I don't like it!"


Should be And not Then.

leonstr · 2026-01-28T13:05:34Z

tests/behat/moderation_marking.feature

+      | Comment   | I don't like it at all! |
+
+    And I am on the "Course 1" "course" page logged in as "marker1"
+    And I follow "Coursework 1"


leonstr · 2026-01-28T13:07:52Z

tests/behat/moderation_marking.feature

+    And I follow "Agreed" in row "1"
+    Then I should see "Moderation for "
+    And I should see "Moderator 1"
+    And "Save changes" "button" should not exist


Could use But.

opitz requested a review from leonstr December 12, 2025 16:23

opitz self-assigned this Dec 12, 2025

Copilot AI review requested due to automatic review settings December 12, 2025 16:23

Copilot AI reviewed Dec 12, 2025

View reviewed changes

andrewhancox requested changes Dec 12, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings December 15, 2025 17:03

Copilot AI reviewed Dec 15, 2025

View reviewed changes

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

opitz requested review from andrewhancox and Copilot December 17, 2025 11:21

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings December 18, 2025 12:23

Copilot AI reviewed Dec 18, 2025

View reviewed changes

tests/behat/double_marking_blind.feature Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

leonstr requested changes Dec 22, 2025

View reviewed changes

opitz force-pushed the CTP-4793-behat-tests branch from eb3eff1 to 23ee7dc Compare January 7, 2026 19:11

stuartlamour requested a review from Copilot January 8, 2026 08:13

Copilot AI reviewed Jan 8, 2026

View reviewed changes

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

opitz force-pushed the CTP-4793-behat-tests branch from 23ee7dc to 72dce74 Compare January 12, 2026 10:52

opitz requested review from Copilot and leonstr January 12, 2026 10:53

Copilot AI reviewed Jan 12, 2026

View reviewed changes

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

opitz requested a review from Copilot January 12, 2026 11:28

Copilot AI reviewed Jan 12, 2026

View reviewed changes

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

opitz requested a review from Copilot January 12, 2026 17:07

Copilot AI reviewed Jan 12, 2026

View reviewed changes

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

tests/behat/behat_mod_coursework.php Outdated Show resolved Hide resolved

opitz added 23 commits January 21, 2026 18:33

removed Scenario: Add markers

59691be

removed unused user teacher1

2f058c5

removed custom field

cdfad18

removed unneccessary logins,

59b589c

reamed i_allocate_the_following_markers() to the_following_markers_are_allocated()

added $coursework->assessorallocationstrategy = "none"

9f5fb79

revoking refactoring i_should_see_text_in_row() for now

8a10025

check moderator can edit before marks have been released

ca332dc

using ExpectationException

133eb65

addressed copilot code review issues

e59de35

addressed copilot code review issues 2

6c8d0c6

reintroduced checking int for datestring to avoid error

4c33925

addressed copilot code review issues 3

97c69a7

refactored Scenario: Check moderation

42d2c24

addressed copilot code review issues 4

137a15d

addressed copilot code review issues 5

af894d8

addressed copilot code review issues 6

7eea472

addressed copilot code review issues 7

d3c874f

refactored into two files: double_marking_blind.feature and moderatio…

cdcff4d

…n_marking.feature using existing behat steps to replace there_is_a_blind_marking_coursework()

updated message

b4137bc

removed allow_role_to_assign_role() as no longer needed

e41fff9

replaced i_should_see_text_in_row() with core step and removed it

a8ab47c

replaced i_follow_in_row() with core step and removed it

0610dbe

addressed copilot code review issues 8

eab83a0

opitz force-pushed the CTP-4793-behat-tests branch from c1d88de to eab83a0 Compare January 21, 2026 18:33

using unique feature

3fd615e

Copilot AI review requested due to automatic review settings January 22, 2026 10:08

Copilot AI reviewed Jan 22, 2026

View reviewed changes

tests/behat/moderation_marking.feature Outdated Show resolved Hide resolved

removed trailing spaces

b4958ee

leonstr reviewed Jan 28, 2026

View reviewed changes

CTP-4793 moderated marking behat tests #181

Are you sure you want to change the base?

CTP-4793 moderated marking behat tests #181

Uh oh!

Conversation

opitz commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewhancox left a comment

Choose a reason for hiding this comment

Uh oh!

leonstr commented Dec 15, 2025

Uh oh!

andrewhancox commented Dec 15, 2025

Uh oh!

andrewhancox commented Dec 15, 2025

Uh oh!

stuartlamour commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

opitz commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

leonstr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewhancox commented Jan 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

stuartlamour commented Dec 15, 2025 •

edited

Loading