From ecf36d49a0ec68a90185f9a01136e42b9f6055dd Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 17 May 2023 16:10:39 +0200 Subject: [PATCH 1/3] Revert "Merge pull request #4639 from dodona-edu/enhance/speedup-validation" This reverts commit bdcbd909fc8d38fc2f9c95cd57ecfcb1d9a31d8b, reversing changes made to 4b06d5f66a3b8af7f011ce130e4ab2faf0bb9dd5. --- app/assets/javascripts/i18n/translations.json | 32 ++++----- app/runners/result_constructor.rb | 68 +++++-------------- test/runners/result_constructor_test.rb | 49 +------------ 3 files changed, 33 insertions(+), 116 deletions(-) diff --git a/app/assets/javascripts/i18n/translations.json b/app/assets/javascripts/i18n/translations.json index d221e55d0b..e676e7720f 100644 --- a/app/assets/javascripts/i18n/translations.json +++ b/app/assets/javascripts/i18n/translations.json @@ -66,19 +66,19 @@ "datetime": { "distance_in_words": { "about_x_hours": { - "one": "about %{count} hour", + "one": "about 1 hour", "other": "about %{count} hours" }, "about_x_months": { - "one": "about %{count} month", + "one": "about 1 month", "other": "about %{count} months" }, "about_x_years": { - "one": "about %{count} year", + "one": "about 1 year", "other": "about %{count} years" }, "almost_x_years": { - "one": "almost %{count} year", + "one": "almost 1 year", "other": "almost %{count} years" }, "half_a_minute": "half a minute", @@ -87,31 +87,31 @@ "other": "less than %{count} minutes" }, "less_than_x_seconds": { - "one": "less than %{count} second", + "one": "less than 1 second", "other": "less than %{count} seconds" }, "over_x_years": { - "one": "over %{count} year", + "one": "over 1 year", "other": "over %{count} years" }, "x_days": { - "one": "%{count} day", + "one": "1 day", "other": "%{count} days" }, "x_minutes": { - "one": "%{count} minute", + "one": "1 minute", "other": "%{count} minutes" }, "x_months": { - "one": "%{count} month", + "one": "1 month", "other": "%{count} months" }, "x_seconds": { - "one": "%{count} second", + "one": "1 second", "other": "%{count} seconds" }, "x_years": { - "one": "%{count} year", + "one": "1 year", "other": "%{count} years" } }, @@ -605,23 +605,23 @@ "other": "meer dan %{count} jaar" }, "x_days": { - "one": "%{count} dag", + "one": "1 dag", "other": "%{count} dagen" }, "x_minutes": { - "one": "%{count} minuut", + "one": "1 minuut", "other": "%{count} minuten" }, "x_months": { - "one": "%{count} maand", + "one": "1 maand", "other": "%{count} maanden" }, "x_seconds": { - "one": "%{count} seconde", + "one": "1 seconde", "other": "%{count} seconden" }, "x_years": { - "one": "%{count} jaar", + "one": "1 jaar", "other": "%{count} jaar" } }, diff --git a/app/runners/result_constructor.rb b/app/runners/result_constructor.rb index 2234f684eb..bf470e93f7 100644 --- a/app/runners/result_constructor.rb +++ b/app/runners/result_constructor.rb @@ -28,34 +28,19 @@ def initialize(locale) def feed(judge_output) raise ResultConstructorError, 'No judge output' if judge_output.empty? - # save the judge output for later validation should the final result be invalid - @judge_output = judge_output - split_jsons(judge_output).each do |json| - if json.key?(:command) - begin - # Clone the object to have better errors, since the update method - # may modify the json hash. - update(json.clone) - rescue StandardError - # We rescue all errors, since the json may have invalid data, resulting - # in stuff like TypeErrors, NoMethodErrors or KeyErrors - - # Try to establish if the error was cause by the judge or by dodona - if PART_SCHEMER.valid?(json.deep_stringify_keys) - raise ResultConstructorError.new( - 'Dodona encountered an error while processing this valid judge output', - json.to_s - ) - else - raise ResultConstructorError.new( - 'Judge output is not a valid json', - json.to_s - ) - end - end - else + # Required by the gem. See issue below for context. + # https://github.com/davishmcclurg/json_schemer/issues/123 + json_with_string_keys = json.deep_stringify_keys + if PART_SCHEMER.valid?(json_with_string_keys) + update(json) + elsif FULL_SCHEMER.valid?(json_with_string_keys) @result = json + else + raise ResultConstructorError.new( + 'Judge output is not a valid json', + json.to_s + ) end end end @@ -74,24 +59,6 @@ def result(timeout) close_tab(badgeCount: @tab[:badgeCount] || 1) if @level == :tab close_judgement(accepted: false, status: status) if @level == :judgement - # Before finalizing the result, check if it is valid. - unless FULL_SCHEMER.valid?(@result.deep_stringify_keys) - # If it is not valid, check if a partial result is the problem and try to give a better error message. - split_jsons(@judge_output).each do |json| - next if PART_SCHEMER.valid?(json.deep_stringify_keys) || FULL_SCHEMER.valid?(json.deep_stringify_keys) - - raise ResultConstructorError.new( - 'Judge output is not a valid json', - json.to_s - ) - end - - raise ResultConstructorError.new( - 'Constructed result based on judge output is not a valid json', - @result.to_s - ) - end - @result end @@ -149,18 +116,15 @@ def append_message(message: nil) end def annotate_code(values) - annotation = { + (@judgement[:annotations] ||= []) << { text: values[:text] || '', type: values[:type] || 'info', row: values[:row] || 0, - rows: values[:rows] || 1 + rows: values[:rows] || 1, + column: values[:column] || nil, + columns: values[:columns] || nil, + externalUrl: values[:externalUrl] || nil } - - annotation[:column] = values[:column] unless values[:column].nil? - annotation[:columns] = values[:columns] unless values[:columns].nil? - annotation[:externalUrl] = values[:externalUrl] unless values[:externalUrl].nil? - - (@judgement[:annotations] ||= []) << annotation end def escalate_status(status: nil) diff --git a/test/runners/result_constructor_test.rb b/test/runners/result_constructor_test.rb index 3daee0c36e..618146ea7b 100644 --- a/test/runners/result_constructor_test.rb +++ b/test/runners/result_constructor_test.rb @@ -189,7 +189,7 @@ class ResultConstructorTest < ActiveSupport::TestCase status: 'correct', description: 'Correct', annotations: [ - { row: 0, rows: 1, text: 'asdf', type: 'info' } + { row: 0, column: nil, rows: 1, columns: nil, text: 'asdf', type: 'info', externalUrl: nil } ] }, construct_result([ '{ "command": "start-judgement" }', @@ -339,53 +339,6 @@ class ResultConstructorTest < ActiveSupport::TestCase end end - test 'intentionally annoyingly wrong json should fail' do - error = assert_raises ResultConstructorError do - construct_result([ - # This command is invalid - '{ "command": "start-nothing" }', - '{ "title": "Test", "command": "start-tab" }', - '{ "description": { "format": "code", "description": "..." }, "command": "start-context" }', - '{ "description": { "format": "plain", "description": "" }, "command": "start-testcase" }', - '{ "expected": "70", "command": "start-test" }' - ]) - end - assert_equal 'Judge output is not a valid json', error.title - - error = assert_raises ResultConstructorError do - construct_result([ - # There is a typo in "command" - '{ "commund": "start-judgement" }', - '{ "title": "Test", "command": "start-tab" }', - '{ "description": { "format": "code", "description": "..." }, "command": "start-context" }', - '{ "description": { "format": "plain", "description": "" }, "command": "start-testcase" }', - '{ "expected": "70", "command": "start-test" }' - ]) - end - assert_equal 'Dodona encountered an error while processing this valid judge output', error.title - - error = assert_raises ResultConstructorError do - construct_result([ - '{ "command": "start-judgement" }', - '{ "title": "Test", "command": "start-tab" }', - # A nested object is invalid: a description is a list - '{ "description": ["yes"], "command": "start-context" }', - '{ "description": { "format": "plain", "description": "" }, "command": "start-testcase" }', - '{ "expected": "70", "command": "start-test" }' - ]) - end - assert_equal 'Constructed result based on judge output is not a valid json', error.title - end - - test 'invalid code in the result constructor should fail' do - rc = ResultConstructor.new('en') - rc.stubs('start_judgement').raises('error') - error = assert_raises(ResultConstructorError) do - rc.feed('{ "command": "start-judgement" }') - end - assert_equal 'Dodona encountered an error while processing this valid judge output', error.title - end - private def construct_result(food, locale: 'en', timeout: false) From 224493f7905836e9b642ed41ca8b363ade400046 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 17 May 2023 16:11:12 +0200 Subject: [PATCH 2/3] Revert "Merge pull request #4646 from dodona-edu/fix/result-constructor" This reverts commit 4055a50f93282562daaa5c4222874b50effb19c8, reversing changes made to 47b06f82f58a2ff956fc3f70d3c8798c86591231. --- app/runners/result_constructor.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/runners/result_constructor.rb b/app/runners/result_constructor.rb index bf470e93f7..4d7761d01f 100644 --- a/app/runners/result_constructor.rb +++ b/app/runners/result_constructor.rb @@ -77,7 +77,7 @@ def start_judgement def start_tab(title: nil, hidden: nil, permission: nil) check_level(:judgement, 'tab started') @tab = {} - @tab[:description] = title unless title.nil? + @tab[:description] = title @tab[:badgeCount] = 0 @tab[:permission] = permission unless permission.nil? @hiddentab = hidden || false @@ -95,7 +95,7 @@ def start_context(description: nil) def start_testcase(description: nil) check_level(:context, 'testcase started') @testcase = {} - @testcase[:description] = description unless description.nil? + @testcase[:description] = description @testcase[:accepted] = true @level = :testcase end @@ -104,7 +104,7 @@ def start_test(description: nil, expected: nil, channel: nil, format: nil) check_level(:testcase, 'test started') @test = {} @test[:description] = description unless description.nil? - @test[:expected] = expected unless expected.nil? + @test[:expected] = expected @test[:channel] = channel unless channel.nil? @test[:format] = format unless format.nil? @level = :test @@ -137,7 +137,7 @@ def escalate_status(status: nil) def close_test(generated: nil, accepted: nil, status: nil) check_level(:test, 'test closed') - @test[:generated] = generated unless generated.nil? + @test[:generated] = generated escalate_status(status: status) @test[:accepted] = if accepted.nil? then status[:enum] == 'correct' From 1f8fea145e434c141dab55a1a9507c99f08148c8 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 17 May 2023 16:17:42 +0200 Subject: [PATCH 3/3] Bump version --- config/initializers/00_version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/00_version.rb b/config/initializers/00_version.rb index da7d0b150f..60bfc7f7e6 100644 --- a/config/initializers/00_version.rb +++ b/config/initializers/00_version.rb @@ -3,7 +3,7 @@ class Application module Version MAJOR = 6 MINOR = 8 - PATCH = 1 + PATCH = 2 STRING = [MAJOR, MINOR, PATCH].compact.join('.') end