-
-
Notifications
You must be signed in to change notification settings - Fork 117
Custom judges #1351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom judges #1351
Conversation
hole/judges.go
Outdated
}) | ||
} | ||
|
||
func getClosestMultiset(anyAnswer, stdout, itemDelimiter string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is just moved over from play.go
, but support for case insensitivity was added.
hole/play.go
Outdated
answers = []Answer{{Args: []string{}, Answer: code}} | ||
// All other holes use the default judge which compares by equality (trimming the line endings) | ||
if holeJudges[hole.ID] == nil { | ||
holeJudges[hole.ID] = defaultJudge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand why these are updated per-request. They won't change for the lifetime of a deployment right? Could we not pre-populate these in an init block somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that makes sense.
Hmm this failure look legit, can you please take a look @MichalMarsalek
|
@JRaspass Oh, sorry I wasn't paying much attention to the e2e tests, because they are flaky. Yes, I broke that expectation - before the changes the quine expected answer was being set at the very beginning, after the changes it is set later and since the TeX-trivial-solution-prevention fails the solution early without running the code (keeps the stdout empty and sets stderr), we never get to the step when the expected answer now gets set. I see 4 solutions:
|
Ah I see, I think it's fine to update the test(s). The main thing is that trivial quines are prevented, the exact observable output doesn't matter hugely. |
Yes, but now that I think about it, it feels kinda weird to have that expected output empty if the code fails to output. I think option 4 is best here. |
I think that ideally, there should be no Runs generator. Instead, there should be a separate input/args generator and a separate checker/judge, to allow more flexibility. I didn't want to refactor all of the holes (plus some are implemented in a way where the output is not generated by solving for the input, but rather in one step). So, this PR keeps the current way of defining holes, but makes it easier to implement holes that require special output validation by introducing Judges. We already had one such judge - the order agnostic judge, this PR generalises that.
A judge takes the hole inputs and the user output and returns a valid solution close to the user's. I consider this to be quite a clever way of supporting different custom validation rules without having to add special UI for each new hole (but of course special UI for a given judge can be added, like I did for the order agnostic one).
To implement a hole one should
.Answer
of each Run in the Run generator, or.Answer
value and can be used to just modify that.This PR also adds a "Css Colors Inverse" hole as a showcase.