From d4288dbbad5400d9315454a271d1d4e5063d3dd9 Mon Sep 17 00:00:00 2001 From: Ben Eater Date: Wed, 13 Nov 2013 13:20:45 -0800 Subject: [PATCH] Fix handling of unsimplified fractions in set & multiple answers All of the new unit tests I added were things that were previously broken. :( Roughly the same bugs exist in Perseus too, so I guess I'll tackle that next... Test Plan: Run unit tests at http://exercises.ka.local/utils/test/answer-types.html and try answering http://exercises.ka.local/exercises/completing_the_square_1.html?debug&problem=original with one part unsimplified and the rest otherwise correct, or with the rest wrong in some way. Auditors: alex --- utils/answer-types.js | 62 +++++++++++++++++++++------- utils/test/answer-types.html | 35 ++++++++-------- utils/test/rational_expressions.html | 2 +- 3 files changed, 66 insertions(+), 33 deletions(-) diff --git a/utils/answer-types.js b/utils/answer-types.js index 24b599dfe..77ca34c0e 100644 --- a/utils/answer-types.js +++ b/utils/answer-types.js @@ -1103,6 +1103,7 @@ Khan.answerTypes = $.extend(Khan.answerTypes, { message: null, guess: guess }; + var blockGradingMessage = null; // If the answer is completely empty, don't grade it if (checkIfAnswerEmpty(guess)) { @@ -1117,27 +1118,36 @@ Khan.answerTypes = $.extend(Khan.answerTypes, { // with the corresponding validator var pass = validators[i](g); - score.empty = score.empty && pass.empty; - score.correct = score.correct && pass.correct; - // TODO(eater): This just forwards one message - if (pass.message) { - score.message = pass.message; + if (pass.message && pass.empty) { // Special case where a validator returns a message // for an "empty" response. This probably means it's // not really empty, but a correct-but-not-simplified // answer. Rather that treating this as actually empty, // possibly leading to the entire multiple being marked - // wrong for being incomplete, bail here and forward on - // the message. - if (pass.empty) { - score.empty = true; - score.correct = false; - return score; - } + // wrong for being incomplete, note the situation but + // continue determining whether the entire answer is + // otherwise correct or not before forwarding on the + // message. + blockGradingMessage = pass.message; + } else { + score.empty = score.empty && pass.empty; + score.correct = score.correct && pass.correct; + // TODO(eater): This just forwards one message + score.message = pass.message; } }); - return score; + if (blockGradingMessage == null || !score.correct) { + score.empty = false; + return score; + } else { + return { + empty: true, + correct: false, + message: blockGradingMessage, + guess: guess + }; + } }; } }, @@ -1255,6 +1265,8 @@ Khan.answerTypes = $.extend(Khan.answerTypes, { message: null, guess: guess }; + var blockGradingMessage = null; + // Store a copy of each of the validators. If one correctly // identifies a guess, remove it from this array, so duplicate // answers aren't marked correct twice @@ -1269,6 +1281,19 @@ Khan.answerTypes = $.extend(Khan.answerTypes, { $.each(unusedValidators, function(i, validator) { var pass = validator(g); + // If this validator is trying to block grading + if (pass.empty && pass.message) { + // remove the working validator + unusedValidators.splice(i, 1); + // We want to block the entire answer from being + // accepted as correct but continue checking in + // case another part is wrong. + blockGradingMessage = pass.message; + correct = true; + // break + return false; + } + // If this validator completely accepts this answer // or returns a check answer message if (pass.correct || pass.message) { @@ -1322,7 +1347,16 @@ Khan.answerTypes = $.extend(Khan.answerTypes, { score.correct = false; } - return score; + if (blockGradingMessage == null || !score.correct) { + return score; + } else { + return { + empty: true, + correct: false, + message: blockGradingMessage, + guess: guess + }; + } }; } }, diff --git a/utils/test/answer-types.html b/utils/test/answer-types.html index db5336417..d7f0c449c 100644 --- a/utils/test/answer-types.html +++ b/utils/test/answer-types.html @@ -43,7 +43,7 @@ }; - + @@ -396,21 +396,6 @@

start(); }); - asyncTest("number decimal percent", 9, function() { - var $problem = jQuery("#qunit-fixture .problem").append( - "

0.42<\/p>" - ); - - var answerData = Khan.answerTypes.number.setup($("#solutionarea"), - $problem.children(".solution")); - - testAnswer(answerData, "0.42", "right", "right answer is right"); - testAnswer(answerData, "42%", "right", "right answer is right"); - testAnswer(answerData, "42", "empty-message", "leaving off percent sign provides a message"); - - start(); - }); - asyncTest("number mixed", 18, function() { var $problem = jQuery("#qunit-fixture .problem").append( "

-1.5<\/p>" @@ -636,7 +621,7 @@

start(); }); - asyncTest("multiple", 21, function() { + asyncTest("multiple", 42, function() { var $problem = jQuery("#qunit-fixture .problem").append( "

" + "7<\/span>" + @@ -654,6 +639,13 @@

testMultipleAnswer(answerData, ["", "3/2"], "wrong", "incomplete answer is wrong"); testMultipleAnswer(answerData, ["", ""], "empty", "empty answer is empty"); testMultipleAnswer(answerData, ["7", "6/4"], "empty-message", "unsimplified right answer provides a message"); + testMultipleAnswer(answerData, ["14/2", "6/4"], "empty-message", "unsimplified right gives message"); + testMultipleAnswer(answerData, ["14/2", "3/2"], "empty-message", "unsimplified right gives message"); + testMultipleAnswer(answerData, ["7", "6/4"], "empty-message", "unsimplified right gives message"); + testMultipleAnswer(answerData, ["14/2", "7"], "wrong", "unsimplified right and wrong is wrong"); + testMultipleAnswer(answerData, ["3", "6/4"], "wrong", "unsimplified right and wrong is wrong"); + testMultipleAnswer(answerData, ["4/2", "4/2"], "wrong", "unsimplified wrong is wrong"); + testMultipleAnswer(answerData, ["14/2", ""], "wrong", "unsimplified imcomplete answer is wrong"); start(); }); @@ -862,7 +854,7 @@

start(); }); - asyncTest("set with as many things", 24, function() { + asyncTest("set with as many things", 45, function() { var $problem = jQuery("#qunit-fixture .problem").append( "
" + "12<\/span>" + @@ -885,6 +877,13 @@

testMultipleAnswer(answerData, ["", ""], "empty", "empty answer is empty"); testMultipleAnswer(answerData, ["12", "14"], "wrong", "wrong answer is wrong"); testMultipleAnswer(answerData, ["12", "12"], "wrong", "wrong answer is wrong"); + testMultipleAnswer(answerData, ["24/2", "26/2"], "empty-message", "unsimplified right gives message"); + testMultipleAnswer(answerData, ["24/2", "13"], "empty-message", "unsimplified right gives message"); + testMultipleAnswer(answerData, ["12", "26/2"], "empty-message", "unsimplified right gives message"); + testMultipleAnswer(answerData, ["24/2", "14"], "wrong", "unsimplified right and wrong is wrong"); + testMultipleAnswer(answerData, ["14", "26/2"], "wrong", "unsimplified right and wrong is wrong"); + testMultipleAnswer(answerData, ["4/2", "4/2"], "wrong", "unsimplified wrong is wrong"); + testMultipleAnswer(answerData, ["24/2", ""], "wrong", "unsimplified imcomplete answer is wrong"); start(); }); diff --git a/utils/test/rational_expressions.html b/utils/test/rational_expressions.html index 2b254e9b9..7c2750d2e 100644 --- a/utils/test/rational_expressions.html +++ b/utils/test/rational_expressions.html @@ -38,7 +38,7 @@ }; - +