cloudcoderdotorg / CloudCoder

A web-based programming exercise system.
GNU Affero General Public License v3.0
71 stars 60 forks source link

Actual output not shown in CPLUSPLUS_FUNCTION test results #89

Open katieirenec opened 9 years ago

katieirenec commented 9 years ago

The "Actual" column in CPLUSPLUS_FUNCTION test output is unpopulated (see picture). Actual output is important feedback for students to improve their code.

Missing Actual column

If you can give me some pointers I can try to look into this, if no developers are available.

kgashok commented 9 years ago

Yes, this would be a good fix to have.

sidstamm commented 6 years ago

I also want this feature, so I started digging into the codebase. @katieirenec, I'm curious what you think... these are fairly unstructured thoughts from someone who has not used GWT or looked at this codebase before this week.

It appears the return value of the function is captured and eaten by the C "scaffolding" constructed around the stand-alone function: https://github.com/cloudcoderdotorg/CloudCoder/blob/master/CloudCoderBuilder2/src/org/cloudcoder/builder2/cfunction/AddCFunctionScaffoldingBuildStep.java#L95

Then the success/failure of each test is sent back as a return code constant. For example, this is the code generated for a sample addIntegers() problem and you can see the student submission at the top before #undef eq:

#include <string.h>
#include <stdlib.h>
#include <stdio.h>
int addIntegers(int a, int b) {
    //TODO - add your code here
    return 566;
}

#undef eq
#define eq(a,b) ((a) == (b))
int main(int argc, char ** argv) {
  int rcIfEqual = atoi(argv[2]);
  int rcIfNotEqual = atoi(argv[3]);
  argv[2] = 0;
  argv[3] = 0;
  if (strcmp(argv[1], "t0")==0) {
    return eq(addIntegers(2, 3), (5)) ? rcIfEqual : rcIfNotEqual;
  }
  if (strcmp(argv[1], "t1")==0) {
    return eq(addIntegers(-3, -11), (-14)) ? rcIfEqual : rcIfNotEqual;
  }
  if (strcmp(argv[1], "t2")==0) {
    return eq(addIntegers(0, 7), (7)) ? rcIfEqual : rcIfNotEqual;
  }
  return 99;
}

To kind of hack together a solution we could return the actual output in some sort of side-channel (probably using stdout or something). Then, we'd have to recover the returned value and put it into the TestResult structure during the CheckCFunctionCommandResultsBuildStep: https://github.com/cloudcoderdotorg/CloudCoder/blob/master/CloudCoderBuilder2/src/org/cloudcoder/builder2/cfunction/CheckCFunctionCommandResultsBuildStep.java#L67

But what would probably be better would be to redesign the test assembly structure and model it more closely after how the Python function tests are run. Unfortunately this might not work for C since the python tests return a tuple from the python program, so this would require a bit more deep thought and understanding of the system than I currently have.

Another idea would be to encode the "output" section to contain the function results, then actual stdout separated by some delimiter or size window. I'm not sure what a good solution is.

sidstamm commented 6 years ago

Hmm... and the scaffolding code doesn't know anything about the return type of the function being tested, so it might be helpful to solicit the type in the test cases.

sidstamm commented 6 years ago

This patch seems to do the job, but it's hacky and is limited to "int" return types from C functions: https://github.com/cloudcoderdotorg/CloudCoder/commit/7634fd2475c03afa52dc71a2268c6169bea5ea48

In that changeset, I'm smuggling the return value from the scaffolding C code back into the java app via known location in stderr.

I'm not sure how to best extend it to support other return types, but we could solicit it from the test writer, I suppose.

sidstamm commented 6 years ago

Updated patch: https://github.com/sidstamm/CloudCoder/compare/master...sidstamm:cfcn_results

This set of changes adds a new field to the test case editor so the author can specify what return type to expect, and then it's used by the C Function tests to put the type into the test. It's ignored for everything except the C functions.

Floating point round errors may make it hard to craft good float or double function tests, but it does work with proper test and expected results.

Things left to do:

I'd appreciate feedback from anyone who wants this feature. When I'm happy with it, I'll submit a PR.