Integrate 1920 Source Format and Grader (#363)

* Modified autograding_errors to autograding_summary

* integrate autograder and new xml format

* updated schema

* small refactor

* Renamed template to solutionTemplate for viewer

* Removed redundant comments

* Added :score as a requirement for :testcase

* Removed redundant test and comment

* Removed overriding of status in lambda_worker

* Added more assertions to testcase test

* Small refactor

* Update lib/cadet/jobs/autograder/lambda_worker.ex

Guard parse_response

Co-Authored-By: Julius Putra Tanu Setiaji <indocomsoft@gmail.com>

* Removed redundant pipeline

* Added test for when lambda crashes

* Shifted comments

* Set parse_response to private

* Added comment for clarity

* Refactored functions

Author: geshuming

lib/cadet/assessments/answer.ex

@@ -17,7 +17,7 @@ defmodule Cadet.Assessments.Answer do
     field(:xp, :integer, default: 0)
     field(:xp_adjustment, :integer, default: 0)
     field(:autograding_status, AutogradingStatus, default: :none)
-    field(:autograding_errors, {:array, :map}, default: [])
+    field(:autograding_results, {:array, :map}, default: [])
     field(:answer, :map)
     field(:type, QuestionType, virtual: true)
     field(:comment, :string)
@@ -56,7 +56,7 @@ defmodule Cadet.Assessments.Answer do
   @spec autograding_changeset(%__MODULE__{} | Ecto.Changeset.t(), map()) :: Ecto.Changeset.t()
   def autograding_changeset(answer, params) do
     answer
-    |> cast(params, ~w(grade adjustment xp autograding_status autograding_errors)a)
+    |> cast(params, ~w(grade adjustment xp autograding_status autograding_results)a)
     |> validate_xp_grade_adjustment_total()
   end
 

lib/cadet/assessments/autograding_status.ex

@@ -4,5 +4,6 @@ defenum(Cadet.Assessments.Answer.AutogradingStatus, :autograding_status, [
   :none,
   :processing,
   :success,
+  # note that :failed refers to the autograder failing due to system errors
   :failed
 ])

lib/cadet/assessments/question_types/programming_question.ex

@@ -4,20 +4,27 @@ defmodule Cadet.Assessments.QuestionTypes.ProgrammingQuestion do
   """
   use Cadet, :model
 
+  alias Cadet.Assessments.QuestionTypes.Testcase
+
   @primary_key false
   embedded_schema do
     field(:content, :string)
-    field(:solution_template, :string)
+    field(:prepend, :string, default: "")
+    field(:template, :string)
+    field(:postpend, :string, default: "")
     field(:solution, :string)
-    field(:autograder, {:array, :string})
+    embeds_many(:public, Testcase)
+    embeds_many(:private, Testcase)
   end
 
-  @required_fields ~w(content solution_template)a
-  @optional_fields ~w(solution autograder)a
+  @required_fields ~w(content template)a
+  @optional_fields ~w(solution prepend postpend)a
 
   def changeset(question, params \\ %{}) do
     question
     |> cast(params, @required_fields ++ @optional_fields)
+    |> cast_embed(:public, with: &Testcase.changeset/2)
+    |> cast_embed(:private, with: &Testcase.changeset/2)
     |> validate_required(@required_fields)
   end
 end

lib/cadet/assessments/question_types/programming_question_testcases.ex

@@ -0,0 +1,22 @@
+defmodule Cadet.Assessments.QuestionTypes.Testcase do
+  @moduledoc """
+  The Assessments.QuestionTypes.Testcase entity represents a public/private testcase.
+  """
+  use Cadet, :model
+
+  @primary_key false
+  embedded_schema do
+    field(:program, :string)
+    field(:answer, :string)
+    field(:score, :integer)
+  end
+
+  @required_fields ~w(program answer score)a
+
+  def changeset(question, params \\ %{}) do
+    question
+    |> cast(params, @required_fields)
+    |> validate_required(@required_fields)
+    |> validate_number(:score, greater_than_or_equal_to: 0)
+  end
+end

lib/cadet/jobs/autograder/lambda_worker.ex

@@ -25,18 +25,9 @@ defmodule Cadet.Autograder.LambdaWorker do
       |> ExAws.Lambda.invoke(lambda_params, %{})
       |> ExAws.request!()
 
-    # If the lambda crashes, results are in the format of:
-    # %{"errorMessage" => "${message}"}
-    if is_map(response) do
-      raise inspect(response)
-    else
-      result =
-        response
-        |> parse_response(lambda_params)
-        |> Map.put(:status, :success)
+    result = parse_response(response)
 
-      Que.add(ResultStoreWorker, %{answer_id: answer.id, result: result})
-    end
+    Que.add(ResultStoreWorker, %{answer_id: answer.id, result: result})
   end
 
   def on_failure(%{answer: answer = %Answer{}, question: %Question{}}, error) do
@@ -55,8 +46,17 @@ defmodule Cadet.Autograder.LambdaWorker do
         result: %{
           grade: 0,
           status: :failed,
-          errors: [
-            %{"systemError" => "Autograder runtime error. Please contact a system administrator"}
+          result: [
+            %{
+              "resultType" => "error",
+              "errors" => [
+                %{
+                  "errorType" => "systemError",
+                  "errorMessage" =>
+                    "Autograder runtime error. Please contact a system administrator"
+                }
+              ]
+            }
           ]
         }
       }
@@ -75,8 +75,11 @@ defmodule Cadet.Autograder.LambdaWorker do
       )
 
     %{
-      graderPrograms: question_content["autograder"],
-      studentProgram: answer.answer["code"],
+      prependProgram: Map.get(question_content, "prepend", ""),
+      studentProgram: Map.get(answer.answer, "code"),
+      postpendProgram: Map.get(question_content, "postpend", ""),
+      testcases:
+        Map.get(question_content, "public", []) ++ Map.get(question_content, "private", []),
       library: %{
         chapter: question.grading_library.chapter,
         external: upcased_name_external,
@@ -85,23 +88,24 @@ defmodule Cadet.Autograder.LambdaWorker do
     }
   end
 
-  def parse_response(response, %{graderPrograms: grader_programs}) do
-    response
-    |> Enum.zip(grader_programs)
-    |> Enum.reduce(
-      %{grade: 0, errors: []},
-      fn {result, grader_program}, %{grade: grade, errors: errors} ->
-        if result["resultType"] == "pass" do
-          %{grade: grade + result["grade"], errors: errors}
-        else
-          error_result = %{
-            grader_program: grader_program,
-            errors: result["errors"]
+  defp parse_response(response) when is_map(response) do
+    # If the lambda crashes, results are in the format of:
+    # %{"errorMessage" => "${message}"}
+    if Map.has_key?(response, "errorMessage") do
+      %{
+        grade: 0,
+        status: :failed,
+        result: [
+          %{
+            "resultType" => "error",
+            "errors" => [
+              %{"errorType" => "systemError", "errorMessage" => response["errorMessage"]}
+            ]
           }
-
-          %{grade: grade, errors: errors ++ [error_result]}
-        end
-      end
-    )
+        ]
+      }
+    else
+      %{grade: response["totalScore"], result: response["results"], status: :success}
+    end
   end
 end

lib/cadet/jobs/autograder/result_store_worker.ex

@@ -61,7 +61,7 @@ defmodule Cadet.Autograder.ResultStoreWorker do
       grade: result.grade,
       xp: xp,
       autograding_status: status,
-      autograding_errors: result.errors
+      autograding_results: result.result
     }
 
     answer

lib/cadet/jobs/xml_parser.ex

@@ -247,15 +247,31 @@ defmodule Cadet.Updater.XMLParser do
   end
 
   defp process_question_entity_by_type(entity, "programming") do
-    entity
-    |> xpath(
-      ~x"."e,
-      content: ~x"./TEXT/text()" |> transform_by(&process_charlist/1),
-      solution_template: ~x"./SNIPPET/TEMPLATE/text()" |> transform_by(&process_charlist/1),
-      solution: ~x"./SNIPPET/SOLUTION/text()" |> transform_by(&process_charlist/1),
-      autograder:
-        ~x"./SNIPPET/GRADER/text()"l
-        |> transform_by(&Enum.map(&1, fn charlist -> process_charlist(charlist) end))
+    Map.merge(
+      entity
+      |> xpath(
+        ~x"."e,
+        content: ~x"./TEXT/text()" |> transform_by(&process_charlist/1),
+        prepend: ~x"./SNIPPET/PREPEND/text()" |> transform_by(&process_charlist/1),
+        template: ~x"./SNIPPET/TEMPLATE/text()" |> transform_by(&process_charlist/1),
+        postpend: ~x"./SNIPPET/POSTPEND/text()" |> transform_by(&process_charlist/1),
+        solution: ~x"./SNIPPET/SOLUTION/text()" |> transform_by(&process_charlist/1)
+      ),
+      entity
+      |> xmap(
+        public: [
+          ~x"./SNIPPET/TESTCASES/PUBLIC"l,
+          score: ~x"./@score"oi,
+          answer: ~x"./@answer" |> transform_by(&process_charlist/1),
+          program: ~x"./text()" |> transform_by(&process_charlist/1)
+        ],
+        private: [
+          ~x"./SNIPPET/TESTCASES/PRIVATE"l,
+          score: ~x"./@score"oi,
+          answer: ~x"./@answer" |> transform_by(&process_charlist/1),
+          program: ~x"./text()" |> transform_by(&process_charlist/1)
+        ]
+      )
     )
   end
 

lib/cadet_web/controllers/assessments_controller.ex

@@ -193,9 +193,22 @@ defmodule CadetWeb.AssessmentsController do
               "The library used for this question"
             )
 
-            solutionTemplate(:string, "Solution template for programming questions")
+            prepend(:string, "Prepend program for programming questions")
+
+            template(:string, "Solution template for programming questions")
+
+            postpend(:string, "Postpend program for programming questions")
+
+            testcases(
+              Schema.new do
+                type(:array)
+                items(Schema.ref(:Testcase))
+              end,
+              "Testcase programs for programming questions"
+            )
 
             grader(Schema.ref(:GraderInfo))
+
             gradedAt(:string, "Last graded at", format: "date-time", required: false)
 
             xp(:integer, "Final XP given to this question. Only provided for students.")
@@ -212,6 +225,18 @@ defmodule CadetWeb.AssessmentsController do
               "The max xp for this question",
               required: true
             )
+
+            autogradingStatus(
+              :string,
+              "One of none/processing/success/failed"
+            )
+
+            autogradingResults(
+              Schema.new do
+                type(:array)
+                items(Schema.ref(:AutogradingResult))
+              end
+            )
           end
         end,
       MCQChoice:
@@ -261,6 +286,22 @@ defmodule CadetWeb.AssessmentsController do
               "The external library for this question"
             )
           end
+        end,
+      Testcase:
+        swagger_schema do
+          properties do
+            answer(:string)
+            score(:integer)
+            program(:string)
+          end
+        end,
+      AutogradingResult:
+        swagger_schema do
+          properties do
+            resultType(:string, "One of pass/fail/error")
+            expected(:string)
+            actual(:string)
+          end
         end
     }
   end

lib/cadet_web/views/assessments_view.ex

@@ -128,30 +128,77 @@ defmodule CadetWeb.AssessmentsView do
       grader: grader_builder(grader),
       gradedAt: graded_at_builder(grader),
       xp: &((&1.xp || 0) + (&1.xp_adjustment || 0)),
-      grade: &((&1.grade || 0) + (&1.adjustment || 0))
+      grade: &((&1.grade || 0) + (&1.adjustment || 0)),
+      autogradingStatus: :autograding_status,
+      autogradingResults: build_results(%{results: answer.autograding_results})
     })
   end
 
-  def build_choice(%{choice: choice}) do
+  defp build_results(%{results: results}) do
+    case results do
+      nil -> nil
+      _ -> &Enum.map(&1.autograding_results, fn result -> build_result(result) end)
+    end
+  end
+
+  defp build_result(result) do
+    transform_map_for_view(result, %{
+      resultType: "resultType",
+      expected: "expected",
+      actual: "actual",
+      errorType: "errorType",
+      errors: build_errors(result["errors"])
+    })
+  end
+
+  defp build_errors(errors) do
+    case errors do
+      nil -> nil
+      _ -> &Enum.map(&1["errors"], fn error -> build_error(error) end)
+    end
+  end
+
+  defp build_error(error) do
+    transform_map_for_view(error, %{
+      errorType: "errorType",
+      line: "line",
+      location: "location",
+      errorLine: "errorLine",
+      errorExplanation: "errorExplanation"
+    })
+  end
+
+  defp build_choice(choice) do
     transform_map_for_view(choice, %{
       id: "choice_id",
       content: "content",
       hint: "hint"
     })
   end
 
+  defp build_testcase(testcase) do
+    transform_map_for_view(testcase, %{
+      answer: "answer",
+      score: "score",
+      program: "program"
+    })
+  end
+
   defp build_question_content_by_type(%{question: %{question: question, type: question_type}}) do
     case question_type do
       :programming ->
         transform_map_for_view(question, %{
           content: "content",
-          solutionTemplate: "solution_template"
+          prepend: "prepend",
+          solutionTemplate: "template",
+          postpend: "postpend",
+          testcases: &Enum.map(&1["public"], fn testcase -> build_testcase(testcase) end)
         })
 
       :mcq ->
         transform_map_for_view(question, %{
           content: "content",
-          choices: &Enum.map(&1["choices"], fn choice -> build_choice(%{choice: choice}) end)
+          choices: &Enum.map(&1["choices"], fn choice -> build_choice(choice) end)
         })
     end
   end

priv/repo/migrations/20190318070229_alter_answers_table_autograding_errors.exs

@@ -0,0 +1,7 @@
+defmodule Cadet.Repo.Migrations.AlterAnswersTableAutogradingErrors do
+  use Ecto.Migration
+
+  def change do
+    rename(table(:answers), :autograding_errors, to: :autograding_results)
+  end
+end

schema.png

@@ -32,7 +32,9 @@ defmodule Cadet.AssessmentsTest do
           library: build(:library),
           question: %{
             content: Faker.Pokemon.name(),
-            solution_template: Faker.Lorem.Shakespeare.as_you_like_it(),
+            prepend: "",
+            template: Faker.Lorem.Shakespeare.as_you_like_it(),
+            postpend: "",
             solution: Faker.Lorem.Shakespeare.hamlet()
           }
         },