SERG-Delft / andy

Andy assesses student's test code. It's used in CSE1110, TU Delft.
MIT License
78 stars 23 forks source link

exception handling Meta Tests #260

Open AtillaColak opened 1 year ago

AtillaColak commented 1 year ago

I initially tried to work with compilation exceptions in the LibraryMetaTest but as there was already exception handling in RunMetaTestsStep I made the changes there. I was not sure about how to alert the teacher if there was a compilation exception so I worked with console logging. Please Let me know if something else was intended with "alerting the teacher".

Another thing to note is that when I made the changes in LibraryMetaTest as errors would already be handled here, in the RunMetaTestsStep we would get failing tests that were initially meant to be there for error handling. So this was another reason for why I worked with RunMetaTestsStep folder instead.

I did not delete the other changes, just in case, only commented them out. So when we reach a final decision on where to handle errors, I will delete those commented lines.

Closes #227

mauricioaniche commented 1 year ago

Hi, @AtillaColak, this is a hard MR to review, as I can't compare the previous code with the new one. I think it's easier if you don't have all the comments there, so that git diff can do its job.

What exactly are you trying to achieve? Which issue is this about?

AtillaColak commented 1 year ago

Hi, @AtillaColak, this is a hard MR to review, as I can't compare the previous code with the new one. I think it's easier if you don't have all the comments there, so that git diff can do its job.

What exactly are you trying to achieve? Which issue is this about?

This is an MR for the issue number #227. I've removed the unnecessary comments so it should be easier to review now.

AtillaColak commented 1 year ago

sure I'll get to it tonight

AtillaColak commented 1 year ago

@martinmladenov Hey Martin could you take a look at this? For some reason, the test I wrote for the exam scenario fails and returns false for the "there is genericfailure" check. I would not want to disturb you if I could have solved it myself but I tried for some hours and was still at a dead end. Maybe you have an idea of what I could be obviously missing, so that I can fix it.

martinmladenov commented 1 year ago

@AtillaColak Are you sure that your changes work correctly? If I'm not mistaken, in the current production version, if there's a compilation error due to a badly written meta test, Andy considers that meta test to fail (i.e. there's no generic failure raised). Your changes add an extra condition for exam mode in the part that raises a generic failure, but I don't think this part of the code gets triggered when a meta test encounters a compilation error.