Improve error messages.

cs125-illinois · May 11, 2021 · c9ce785 · c9ce785
1 parent 06b17a2
commit c9ce785
Show file tree

Hide file tree

Showing 10 changed files with 123 additions and 53 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -8,7 +8,7 @@ plugins {
 }
 subprojects {
     group = "com.github.cs125-illinois.questioner"
-    version = "2021.5.3"
+    version = "2021.5.4"
     tasks.withType<KotlinCompile> {
         val javaVersion = JavaVersion.VERSION_1_8.toString()
         sourceCompatibility = javaVersion
@@ -17,16 +17,9 @@ subprojects {
             jvmTarget = javaVersion
         }
     }
-    /*
-    configurations.all {
-        resolutionStrategy {
-            force(
-                "org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.5.0",
-                "org.jetbrains.kotlin:kotlin-script-runtime:1.5.0"
-            )
-        }
+    tasks.withType<Test> {
+        useJUnitPlatform()
     }
-     */
 }
 allprojects {
     repositories {
@@ -40,9 +33,9 @@ allprojects {
 }
 tasks.dependencyUpdates {
     fun String.isNonStable() = !(
-            listOf("RELEASE", "FINAL", "GA", "JRE").any { toUpperCase().contains(it) }
-                    || "^[0-9,.v-]+(-r)?$".toRegex().matches(this)
-            )
+        listOf("RELEASE", "FINAL", "GA", "JRE").any { toUpperCase().contains(it) }
+            || "^[0-9,.v-]+(-r)?$".toRegex().matches(this)
+        )
     rejectVersionIf { candidate.version.isNonStable() }
     gradleReleaseChannel = "current"
 }
diff --git a/lib/src/main/kotlin/Question.kt b/lib/src/main/kotlin/Question.kt
@@ -26,6 +26,7 @@ import edu.illinois.cs.cs125.jeed.core.kompile
 import edu.illinois.cs.cs125.jeed.core.ktLint
 import edu.illinois.cs.cs125.jeed.core.moshi.CompiledSourceResult
 import edu.illinois.cs.cs125.jenisol.core.CapturedResult
+import edu.illinois.cs.cs125.jenisol.core.ParameterGroup
 import edu.illinois.cs.cs125.jenisol.core.Settings
 import edu.illinois.cs.cs125.jenisol.core.SubmissionDesignError
 import edu.illinois.cs.cs125.jenisol.core.TestResult
@@ -52,7 +53,7 @@ data class Question(
     val klass: String,
     val metadata: Metadata,
     val question: FlatFile,
-    val correct: FlatFile,
+    val correct: FlatFileWithComplexity,
     val alternativeSolutions: List<FlatFile>,
     val incorrect: List<IncorrectFile>,
     val common: List<String>?,
@@ -100,6 +101,14 @@ data class Question(
     @JsonClass(generateAdapter = true)
     data class FlatFile(val klass: String, val contents: String, val language: Language)
 
+    @JsonClass(generateAdapter = true)
+    data class FlatFileWithComplexity(
+        val klass: String,
+        val contents: String,
+        val language: Language,
+        val complexity: Int
+    )
+
     @JsonClass(generateAdapter = true)
     data class IncorrectFile(val klass: String, val contents: String, val reason: Reason, val language: Language) {
         enum class Reason { DESIGN, COMPILE, TEST, CHECKSTYLE, TIMEOUT }
@@ -303,10 +312,10 @@ data class Question(
             }
             val klass = it.first()
             if (!(
-                        language == Language.kotlin &&
-                                solution.skipReceiver &&
-                                klass == "${compilationDefinedClass}Kt"
-                        )
+                    language == Language.kotlin &&
+                        solution.skipReceiver &&
+                        klass == "${compilationDefinedClass}Kt"
+                    )
             ) {
                 if (klass != compilationDefinedClass) {
                     message =
@@ -363,6 +372,12 @@ data class Question(
     @Transient
     var slowestFailingContent = ""
 
+    @Transient
+    var slowestFailingFailed = false
+
+    @Transient
+    var slowestFailingInputs: ParameterGroup? = null
+
     var solutionPrinted = 0
 
     @Suppress("ReturnCount", "LongMethod", "ComplexMethod", "LongParameterList")
@@ -547,6 +562,8 @@ data class Question(
                 if (correct == false && results.size > requiredTestCount) {
                     requiredTestCount = results.size
                     slowestFailingContent = compiledSubmission.source.contents
+                    slowestFailingFailed = results.any { it.failed }
+                    slowestFailingInputs = results.find { it.failed }?.parameters
                 }
                 testResults.complete.testing =
                     TestResults.TestingResult(results.map { it.asTestResult(compiledSubmission.source) }, results.size)
@@ -556,18 +573,16 @@ data class Question(
     }
 
     var validated = false
+    var validationSeed: Int? = null
 
     data class ValidationReport(
         val incorrect: Int,
         val mutations: Int,
         val requiredTestCount: Int,
         val kotlin: Boolean,
         val kotlinIncorrect: Int,
-        val slowestFailing: String?,
         val timeout: Long
-    ) {
-        fun summary() = copy(slowestFailing = null).toString()
-    }
+    )
 
     @Suppress("LongMethod", "ComplexMethod")
     suspend fun initialize(addMutations: Boolean = true, seed: Int = Random.nextInt()): ValidationReport {
@@ -615,26 +630,26 @@ data class Question(
         }
 
         val incorrectToTest = (
-                incorrect + if (metadata.mutate && addMutations) {
-                    templateSubmission(
-                        if (getTemplate(Language.java) != null) {
-                            "// TEMPLATE_START\n" + correct.contents + "\n// TEMPLATE_END \n"
-                        } else {
-                            correct.contents
-                        }
-                    ).allMutations(random = Random(seed))
-                        .map { it.contents.kotlinDeTemplate(getTemplate(Language.java)) }
-                        // Templated questions sometimes will mutate the template
-                        .filter { it != correct.contents }
-                        .map { IncorrectFile(klass, it, IncorrectFile.Reason.TEST, Language.java) }
-                } else {
-                    listOf()
-                }.also { incorrect ->
-                    check(incorrect.all { it.contents != correct.contents }) {
-                        "Incorrect solution identical to correct solution"
+            incorrect + if (metadata.mutate && addMutations) {
+                templateSubmission(
+                    if (getTemplate(Language.java) != null) {
+                        "// TEMPLATE_START\n" + correct.contents + "\n// TEMPLATE_END \n"
+                    } else {
+                        correct.contents
                     }
+                ).allMutations(random = Random(seed))
+                    .map { it.contents.kotlinDeTemplate(getTemplate(Language.java)) }
+                    // Templated questions sometimes will mutate the template
+                    .filter { it != correct.contents }
+                    .map { IncorrectFile(klass, it, IncorrectFile.Reason.TEST, Language.java) }
+            } else {
+                listOf()
+            }.also { incorrect ->
+                check(incorrect.all { it.contents != correct.contents }) {
+                    "Incorrect solution identical to correct solution"
                 }
-                ).also {
+            }
+            ).also {
                 check(incorrect.isNotEmpty()) { "No incorrect examples found" }
             }
 
@@ -664,15 +679,37 @@ data class Question(
             requiredTestCount,
             hasKotlin,
             incorrect.count { it.language == Language.kotlin },
-            slowestFailingContent,
             submissionTimeout
         )
         check(requiredTestCount > 0)
+        if (requiredTestCount > metadata.maxTestCount) {
+            if (slowestFailingFailed) {
+                error(
+                    """Found incorrect input $slowestFailingInputs for the following incorrect code,
+                        |but it took too many tests (${requiredTestCount} > ${metadata.maxTestCount}):
+                        |---
+                        |$slowestFailingContent
+                        |---
+                        |Perhaps add this input to @FixedParameters?
+                        """.trimMargin()
+                )
+            } else {
+                error(
+                    """Unable to find a failing input for the following incorrect code:
+                        |---
+                        |$slowestFailingContent
+                        |---
+                        |Perhaps add an input to @FixedParameters, or disable this mutation using // mutate-disable?
+                        """.trimMargin()
+                )
+            }
+        }
         require(requiredTestCount <= metadata.maxTestCount) {
             "Requires too many tests: $report"
         }
 
         validated = true
+        validationSeed = seed
 
         test(
             correct.contents,
@@ -731,7 +768,7 @@ data class Question(
 
     val hasKotlin =
         metadata.kotlinDescription != null && alternativeSolutions.find { it.language == Language.kotlin } != null &&
-                ((javaStarter != null) == (kotlinStarter != null))
+            ((javaStarter != null) == (kotlinStarter != null))
 
     companion object {
         const val DEFAULT_RETURN_TIMEOUT = 1024

diff --git a/lib/src/main/kotlin/Validator.kt b/lib/src/main/kotlin/Validator.kt
@@ -20,12 +20,7 @@ class Validator(questionsFile: File, private val sourceDir: String, private val
             return
         }
         question.initialize(seed = seed).also { report ->
-            val output = if (verbose) {
-                report.toString()
-            } else {
-                report.summary()
-            }
-            println("$name: $output")
+            println("$name: $report")
             question.validationFile(sourceDir)
                 .writeText(moshi.adapter(Question::class.java).indent("  ").toJson(questions[name]))
         }

diff --git a/plugin/build.gradle.kts b/plugin/build.gradle.kts
@@ -38,6 +38,9 @@ tasks.compileKotlin {
 tasks.compileTestKotlin {
     dependsOn(tasks.generateGrammarSource)
 }
+tasks.lintKotlinMain {
+    dependsOn(tasks.generateGrammarSource)
+}
 tasks.generateGrammarSource {
     outputDirectory = File(projectDir, "src/main/java/edu/illinois/cs/cs125/questioner/antlr")
     arguments.addAll(

diff --git a/plugin/src/main/kotlin/CleanQuestions.kt b/plugin/src/main/kotlin/CleanQuestions.kt
@@ -0,0 +1,26 @@
+package edu.illinois.cs.cs125.questioner.plugin
+
+import org.gradle.api.DefaultTask
+import org.gradle.api.file.FileCollection
+import org.gradle.api.plugins.JavaPluginConvention
+import org.gradle.api.tasks.InputFiles
+import org.gradle.api.tasks.TaskAction
+
+@Suppress("unused")
+abstract class CleanQuestions : DefaultTask() {
+    init {
+        group = "Build"
+        description = "Clean question validation results."
+    }
+
+    @InputFiles
+    val inputFiles: FileCollection = project.convention.getPlugin(JavaPluginConvention::class.java)
+        .sourceSets.getByName("main").allSource.filter { it.name == ".validation.json" }
+
+    @TaskAction
+    fun clean() {
+        inputFiles.forEach {
+            it.delete()
+        }
+    }
+}
diff --git a/plugin/src/main/kotlin/QuestionerPlugin.kt b/plugin/src/main/kotlin/QuestionerPlugin.kt
@@ -55,5 +55,6 @@ class QuestionerPlugin : Plugin<Project> {
         project.convention.getPlugin(JavaPluginConvention::class.java)
             .sourceSets.getByName("main").resources { it.srcDirs(File(project.buildDir, "questioner")) }
         project.tasks.register("questionerTesting", TestingTask::class.java)
+        project.tasks.register("cleanQuestions", CleanQuestions::class.java)
     }
 }
diff --git a/plugin/src/main/kotlin/save/ParseJava.kt b/plugin/src/main/kotlin/save/ParseJava.kt
@@ -4,6 +4,8 @@ import com.google.googlejavaformat.java.Formatter
 import edu.illinois.cs.cs125.jeed.core.CheckstyleArguments
 import edu.illinois.cs.cs125.jeed.core.Source
 import edu.illinois.cs.cs125.jeed.core.checkstyle
+import edu.illinois.cs.cs125.jeed.core.complexity
+import edu.illinois.cs.cs125.jeed.core.fromSnippet
 import edu.illinois.cs.cs125.questioner.antlr.JavaLexer
 import edu.illinois.cs.cs125.questioner.antlr.JavaParser
 import edu.illinois.cs.cs125.questioner.lib.AlsoCorrect
@@ -175,7 +177,7 @@ data class ParsedJavaFile(val path: String, val contents: String) {
         }
     }
 
-    fun toCleanSolution(cleanSpec: CleanSpec): Question.FlatFile {
+    fun toCleanSolution(cleanSpec: CleanSpec): Question.FlatFileWithComplexity {
         val solutionContent = clean(cleanSpec).let { content ->
             Source.fromJava(content).checkstyle(CheckstyleArguments(failOnError = false)).let { results ->
                 val removeLines = results.errors.filter { error ->
@@ -184,7 +186,18 @@ data class ParsedJavaFile(val path: String, val contents: String) {
                 content.lines().filterIndexed { index, _ -> !removeLines.contains(index + 1) }.joinToString("\n")
             }
         }
-        return Question.FlatFile(className, solutionContent, Question.Language.java)
+        val complexity = if (cleanSpec.notClass) {
+            Source.fromSnippet(solutionContent)
+        } else {
+            Source(mapOf("$className.java" to solutionContent))
+        }.complexity().let {
+            if (cleanSpec.notClass) {
+                it.lookup("")
+            } else {
+                it.lookup(className, "$className.java")
+            }
+        }.complexity
+        return Question.FlatFileWithComplexity(className, solutionContent, Question.Language.java, complexity)
     }
 
     fun toIncorrectFile(cleanSpec: CleanSpec): Question.IncorrectFile {

diff --git a/plugin/src/main/kotlin/save/SaveQuestions.kt b/plugin/src/main/kotlin/save/SaveQuestions.kt
@@ -408,7 +408,9 @@ data class CleanSpec(
     val hasTemplate: Boolean = false,
     val wrappedClass: String? = null,
     val importNames: List<String> = listOf()
-)
+) {
+    val notClass = hasTemplate || wrappedClass != null
+}
 
 internal fun String.stripPackage(): String {
     val packageLine = lines().indexOfFirst { it.trim().startsWith("package ") }

diff --git a/plugin/src/test/kotlin/TestSaveQuestions.kt b/plugin/src/test/kotlin/TestSaveQuestions.kt
@@ -45,7 +45,7 @@ public class Second {
 
                 parsedFile.correct shouldNotBe null
                 parsedFile.type shouldBe "Correct"
-                parsedFile.wrapWith shouldBe "Question"
+                parsedFile.wrapWith shouldBe "Second"
 
                 parsedFile.correct!!.also {
                     it.name shouldBe "Test"

diff --git a/server/src/main/resources/edu.illinois.cs.cs125.questioner.server.version b/server/src/main/resources/edu.illinois.cs.cs125.questioner.server.version
@@ -1 +1 @@
-version=2021.5.3
+version=2021.5.4