"When checking his answers, in 2.1 the student should see the correctness/incorrectness of his answers clearly marked (color and icon) as it was in 1.9"

I do agree... In adaptative mode, it's quite useful for students to be able to see quickly and easily what is rigth and what is wrong.
No news about this improvement ?

Is this bug still present in Moodle 2.3+?

Yes it is.
I wouldn't call it a bug, but the improvement would be very welcome...
At the moment, finding the wrong answer(s) is less obvious than it was in Moodle 1.9

Nothing has changed (in Moodle 2.3+) since I reported these bugs almost a year ago.

Some of the problems has been corrected see MDL-35370 which is waiting for integration.

We should note however that the adaptative in 2,3 for shortanswer does not display color on the response input display on check button response.
The close follow the shortanswer display.
On close pop up the feedback is shown and the grade seems OK.
More tests are need to compare all the possibilities...

However the shortanswer handling in 1,9 put color and icon.

I recently added a shortsighted hack to remedy this problem with the multianswer qtype in our Moodle.

https://github.com/martinluther/moodle/commits/multianswerfeedback

I don't agree that once you click on the "Submit all and finish" button the not answered subquestion Q03 should be left white, IMHO as the attempt is finished it should be considered false and be red as it is now.
And also note that in latest master Q02 tooltip correctly show "Incorrect".
So I think the 2 things marked as BUG by Joseph are no more a problem.
This left us with the fact that subquestions are not coloured when you click on "Check".
It seems that all opinions expressed in the forum are in favour of bringing this feature back.

If someone can turn my suggested change https://moodle.org/mod/forum/discuss.php?d=226739#p984687 into a git commit, and write some unit tests, then I am happy to review and submit this for integration. I am afraid I don't have time to work on the code myself.

I agree with Jean-Michel's comments dated 22/Apr/13 2:20 PM.
I would be grateful if Jean-Michel would turn Tim's suggested changes in the Quiz forum into a git commit and write the relevant test units (at which J.-M. is very good ).

Thanks Joseph but don't forget we are both french so I learned "Le corbeau et le renard" when I was a schoolboy
Turning Tim's change in a github branch is quite easy.
Question for Tim:
For the tests, I assume we only want to test that the classes are present in the output, so something like

question_pattern_expectation('~<input[^>]* class="correct" [^>]*/>~')

and

question_pattern_expectation('~<input[^>]* class="incorrect" [^>]*/>~')

would be enough ? or do we want to test that each subquestion has the right class associated ?

If you look at some of the other unit tests, for example https://github.com/maths/moodle-qtype_stack/blob/master/tests/test_base.php#L157, you will see that PHPunit has helpers to assert whether some HTML contains a given tag with given attributes.

I think that if you use that, rather than regular expressions, then you can assert that particular inputs have a particular class. But, if that seems like too much work, the regular expressions you propose would be OK.

Oh yes, I remember now, I already copied your check_output_contains_text_input function when testing formulas !! Should be easy to adapt this to test that inputs have the right class.
I will do that tomorrow, I am too tired now.

Hello Tim,
Sorry to bother you, but before I go further, can you have a look at the way I do my tests because I am not really sure I am heading in the right direction.
Maybe I should try to write a get_contains_xxx and a get_does_not_contain_xxx function but I must admit I don't clearly see the benefit over check_output_contains_ functions ?
stack walkthrough tests seems tu use a lot of check_output_contains_ and check_output_does_not_contain_ but other walkthrough tests don't.

Oops, I realize my wait to use assertTag with a regexp is flawed in some case because incorrect and partiallycorrect both contains correct as a substring

Right, this is not recorded anywhere except in my brain, but the old get_contains_xxx way is deprecated, and the check_output_contains_ way is recommended.

I really must find time to fix all the existing tests in Moodle core, so this is clear.

Splendid so I used the new way of doing things and not the deprecated one
Additionally working on this gave you the occasion to say to somebody else what is the recommended way
If you can tell me if my test are OK, I will continue working on this in the evening. I have some gardening to do for now.

Looking good. Just a few minor points.

I think you should rename check_output_contains_text_input_class -> check_output_contains_text_input_with_class

I would suggest moving that method to question_testcase, since it would also be useful for other qtypes.

I thinkyou should change

$attributes['class'] = 'regexp:/' . $class . '/';

to

$attributes['class'] = 'regexp:/\b' . $class . '\b/';

(\b means start or end of a word.)

I want to add a walkthrough test for a multianswer question in adaptive mode, but as I must both extend qbehaviour_adaptive_walkthrough and include multianswer helper to have some cloze questions defined, am I right in thinking the best solution is to create a new file named adaptive_walkthrough_test.php in question/type/multianswer/tests/ ?

Doh, my question was stupid, I just need to create a new function in question/behaviour/adaptive/tests/walkthrough_test.php I don't need a new file at all, or to extend qbehaviour_adaptive_walkthrough, or to include question/type/multianswer/tests/helper.php !

I don't mind if you put it in behaviour_adaptive or in qtype_multianswer. I guess in the qtype is slightly better.

Hello Tim,
asking for a first peer review. This seems to be working as expected and has unit tests for shortanswer and multianswer.

Oops, I only saw your message "I guess in the qtype is slightly better" after doing it in the behaviour, but as you don't mind ...

I can no longer remember why this issue is sitting here in the 'peer review in progress' state.

Is there anything still to do?

If not, can you rebase and squash down to one commit, and cherry pick to stable branches if appropriate, which I guess it is.

No I don't think there is anything more to do.
I will rebase, squash and create other branchs.

About creating branchs for MOODLE_24_STABLE and MOODLE_23_STABLE
I have 2 solutions:

• backport only the change without the tests
• backport also the tests but that would require to add other functions to question/engine/tests helpers (for instance I need to backport also get_tag_matcher because I use it and it is only currently in master branch)
  What is your preference ?

My general principles are:

1. If the tests back-port easily, then you should back-port them.
2. If not, then back-port without the unit tests.

This seems to be case 2.

I think this is ready, I tested all branchs. I also verified once more that all tests are OK for adaptive behaviour and all question types on master.

Thanks. Submitting for integration.

Integrated (23, 24 & master), thanks!

Tested on the 2.3, 2.4 and master integration branches.
The adaptive mode now shows colour highlighting.
Test passed.

Hello Tim, Eloy and Adrian,
Thanks a lot for this quick integration and testing. I am sure many users will appreciate this can be part of the future releases.

Did you think this day was not going to arrive ever?

Your patience has been rewarded, yay, sent upstream, thanks!

Closing...ciao