Skip to content
This repository has been archived by the owner on May 11, 2021. It is now read-only.

Commit

Permalink
Fix handling of unsimplified fractions in set & multiple answers
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
Ben Eater committed Nov 13, 2013
1 parent e7a7b6e commit d4288db
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 33 deletions.
62 changes: 48 additions & 14 deletions utils/answer-types.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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
};
}
};
}
},
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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
};
}
};
}
},
Expand Down
35 changes: 17 additions & 18 deletions utils/test/answer-types.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
};
</script>
<script src="../../local-only/underscore.js"></script>
<script src="../../third_party/MathJax/2.1/MathJax.js?config=KAthJax-da9a7f53e588f3837b045a600e1dc439"></script>
<script src="../../third_party/MathJax/2.1/MathJax.js?config=KAthJax-9e2776ffe7d2006f16f36d0d55d9464b"></script>
<script src="../../local-only/katex/katex.js"></script>

<!-- Include QUnit -->
Expand Down Expand Up @@ -396,21 +396,6 @@ <h2 id="qunit-userAgent"></h2>
start();
});

asyncTest("number decimal percent", 9, function() {
var $problem = jQuery("#qunit-fixture .problem").append(
"<p class='solution' data-type='number' data-forms='decimal, percent'>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(
"<p class='solution' data-type='number' data-forms='mixed'>-1.5<\/p>"
Expand Down Expand Up @@ -636,7 +621,7 @@ <h2 id="qunit-userAgent"></h2>
start();
});

asyncTest("multiple", 21, function() {
asyncTest("multiple", 42, function() {
var $problem = jQuery("#qunit-fixture .problem").append(
"<p class='solution' data-type='multiple'>" +
"<span class='sol'>7<\/span>" +
Expand All @@ -654,6 +639,13 @@ <h2 id="qunit-userAgent"></h2>
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();
});
Expand Down Expand Up @@ -862,7 +854,7 @@ <h2 id="qunit-userAgent"></h2>
start();
});

asyncTest("set with as many things", 24, function() {
asyncTest("set with as many things", 45, function() {
var $problem = jQuery("#qunit-fixture .problem").append(
"<div class='solution' data-type='set'>" +
"<span class='set-sol'>12<\/span>" +
Expand All @@ -885,6 +877,13 @@ <h2 id="qunit-userAgent"></h2>
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();
});
Expand Down
2 changes: 1 addition & 1 deletion utils/test/rational_expressions.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
};
</script>
<script src="../../local-only/underscore.js"></script>
<script src="../MathJax/2.1/MathJax.js?config=KAthJax-da9a7f53e588f3837b045a600e1dc439"></script>
<script src="../MathJax/2.1/MathJax.js?config=KAthJax-9e2776ffe7d2006f16f36d0d55d9464b"></script>

<!-- Include QUnit -->
<link rel="stylesheet" href="../../test/qunit/qunit/qunit.css" type="text/css" media="screen">
Expand Down

0 comments on commit d4288db

Please sign in to comment.