From ee7729110b432fd0020faeab686ba0fbb4f707a3 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Mon, 26 Oct 2020 21:08:43 +0100 Subject: [PATCH 1/3] Add LinterCheck and LinterCheckRun --- app/models/linter_check.rb | 5 ++++ app/models/linter_check_run.rb | 30 +++++++++++++++++++ .../20201026184633_create_linter_checks.rb | 19 ++++++++++++ db/schema.rb | 22 +++++++++++++- 4 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 app/models/linter_check.rb create mode 100644 app/models/linter_check_run.rb create mode 100644 db/migrate/20201026184633_create_linter_checks.rb diff --git a/app/models/linter_check.rb b/app/models/linter_check.rb new file mode 100644 index 00000000..521035bd --- /dev/null +++ b/app/models/linter_check.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class LinterCheck < ApplicationRecord + has_many :linter_check_runs +end diff --git a/app/models/linter_check_run.rb b/app/models/linter_check_run.rb new file mode 100644 index 00000000..851130e8 --- /dev/null +++ b/app/models/linter_check_run.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class LinterCheckRun < ApplicationRecord + belongs_to :linter_check + belongs_to :testrun + belongs_to :file, class_name: 'CodeOcean::File' + + def self.create_from(testrun, assessment) + assessment[:detailed_linter_results].each do |linter_result| + check = LinterCheck.find_or_create_by!(code: linter_result[:code]) do |new_check| + new_check.name = linter_result[:name] + new_check.severity = linter_result[:severity] + end + + file = testrun.submission.file_by_name(linter_result[:file_name]) + + LinterCheckRun.create!( + linter_check: check, + result: linter_result[:result], + line: linter_result[:line], + scope: linter_result[:scope], + testrun: testrun, + file: file + ) + end + rescue ActiveRecord::RecordInvalid + # Something bad happened. Probably, the RegEx in lib/py_lint_adapter.rb didn't work. + Raven.extra_context(testrun: testrun, linter_result: linter_result) + end +end diff --git a/db/migrate/20201026184633_create_linter_checks.rb b/db/migrate/20201026184633_create_linter_checks.rb new file mode 100644 index 00000000..2e98473b --- /dev/null +++ b/db/migrate/20201026184633_create_linter_checks.rb @@ -0,0 +1,19 @@ +class CreateLinterChecks < ActiveRecord::Migration[5.2] + def change + create_table :linter_checks do |t| + t.string :name, null: false + t.string :code, null: false + t.string :severity + end + + create_table :linter_check_runs do |t| + t.references :linter_check, null: false + t.string :scope + t.integer :line + t.text :result + t.references :testrun, null: false + t.references :file, null: false + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 112687e7..bfc9c8a3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_10_19_090123) do +ActiveRecord::Schema.define(version: 2020_10_26_184633) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -265,6 +265,26 @@ ActiveRecord::Schema.define(version: 2020_10_19_090123) do t.datetime "updated_at" end + create_table "linter_check_runs", force: :cascade do |t| + t.bigint "linter_check_id", null: false + t.string "scope" + t.integer "line" + t.text "result" + t.bigint "testrun_id", null: false + t.bigint "file_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["file_id"], name: "index_linter_check_runs_on_file_id" + t.index ["linter_check_id"], name: "index_linter_check_runs_on_linter_check_id" + t.index ["testrun_id"], name: "index_linter_check_runs_on_testrun_id" + end + + create_table "linter_checks", force: :cascade do |t| + t.string "name", null: false + t.string "code", null: false + t.string "severity" + end + create_table "lti_parameters", id: :serial, force: :cascade do |t| t.integer "external_users_id" t.integer "consumers_id" From e01f2f9ee642173eeb09985189b07090ee7bee7d Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Mon, 26 Oct 2020 21:09:21 +0100 Subject: [PATCH 2/3] Improve PyLintAdapter to match more results --- lib/py_lint_adapter.rb | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/py_lint_adapter.rb b/lib/py_lint_adapter.rb index 828586c8..2447b37a 100644 --- a/lib/py_lint_adapter.rb +++ b/lib/py_lint_adapter.rb @@ -1,6 +1,6 @@ class PyLintAdapter < TestingFrameworkAdapter REGEXP = /Your code has been rated at (-?\d+\.?\d*)\/(\d+\.?\d*)/ - ASSERTION_ERROR_REGEXP = /^(.*?\.py):(\d+):.*?\([^,]*?,\ ([^,]*?),([^,]*?)\)\ (.*?)$/ + ASSERTION_ERROR_REGEXP = /^(.*?\.py):(\d+):(.*?)\(([^,]*?), ([^,]*?),([^,]*?)\) (.*?)$/ def self.framework_name 'PyLint' @@ -21,19 +21,22 @@ class PyLintAdapter < TestingFrameworkAdapter begin assertion_error_matches = Timeout.timeout(2.seconds) do output[:stdout].scan(ASSERTION_ERROR_REGEXP).map do |match| - file_name = match.first.strip - line_no = match.second.strip - test = match.third.strip - # e.g. function name, nil if outside of a function. Not always available - context = match.fourth.strip.presence - description = match.fifth.strip - {test: test, description: description, context: context, line: line_no, file: file_name} + { + file_name: match[0].strip, + line: match[1].to_i, + severity: match[2].strip, + code: match[3].strip, + name: match[4].strip, + # e.g. function name, nil if outside of a function. Not always available + scope: match[5].strip.presence, + result: match[6].strip + } end || [] end rescue Timeout::Error assertion_error_matches = [] end - concatenated_errors = assertion_error_matches.map { |result| "#{result[:test]}: #{result[:description]}" }.flatten + concatenated_errors = assertion_error_matches.map { |result| "#{result[:name]}: #{result[:result]}" }.flatten {count: count, failed: failed, error_messages: concatenated_errors, detailed_linter_results: assertion_error_matches} end end From bad51add3fd45562e76a3e705b28215859052331 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Mon, 26 Oct 2020 21:11:37 +0100 Subject: [PATCH 3/3] Store parsed linter messages --- app/controllers/concerns/submission_scoring.rb | 9 ++++++--- app/models/submission.rb | 5 +++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/controllers/concerns/submission_scoring.rb b/app/controllers/concerns/submission_scoring.rb index c6fd56e7..6c258a5a 100644 --- a/app/controllers/concerns/submission_scoring.rb +++ b/app/controllers/concerns/submission_scoring.rb @@ -12,14 +12,14 @@ module SubmissionScoring output = execute_test_file(file, submission) assessment = assessor.assess(output) passed = ((assessment[:passed] == assessment[:count]) and (assessment[:score]).positive?) - testrun_output = passed ? nil : 'message: ' + output[:message].to_s + "\n stdout: " + output[:stdout].to_s + "\n stderr: " + output[:stderr].to_s + testrun_output = passed ? nil : 'status: ' + output[:status].to_s + "\n stdout: " + output[:stdout].to_s + "\n stderr: " + output[:stderr].to_s unless testrun_output.blank? submission.exercise.execution_environment.error_templates.each do |template| pattern = Regexp.new(template.signature).freeze StructuredError.create_from_template(template, testrun_output, submission) if pattern.match(testrun_output) end end - Testrun.new( + testrun = Testrun.create( submission: submission, cause: 'assess', # Required to differ run and assess for RfC show file: file, # Test file that was executed @@ -27,7 +27,10 @@ module SubmissionScoring output: testrun_output, container_execution_time: output[:container_execution_time], waiting_for_container_time: output[:waiting_for_container_time] - ).save + ) + + LinterCheckRun.create_from(testrun, assessment) if file.teacher_defined_linter? + output.merge!(assessment) output.merge!(filename: file.name_with_extension, message: feedback_message(file, output), weight: file.weight) # end diff --git a/app/models/submission.rb b/app/models/submission.rb index 62d7ad8e..6ff1894a 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -53,6 +53,11 @@ class Submission < ApplicationRecord collect_files.detect(&:main_file?) end + def file_by_name(file_path) + # expects the full file path incl. file extension + collect_files.detect { |file| file.filepath == file_path } + end + def normalized_score ::NewRelic::Agent.add_custom_attributes({unnormalized_score: score}) if !score.nil? && !exercise.maximum_score.nil? && (exercise.maximum_score > 0)