From 6c8b61b93a626bc3bb699a6dcd7b7739ee737cb8 Mon Sep 17 00:00:00 2001 From: Phil Gengler Date: Sun, 29 Sep 2019 21:36:44 -0400 Subject: [PATCH 1/4] Add 'ember_try_results' field to TestResult for saving raw ember-try result JSON --- app/controllers/test_results_controller.rb | 1 + app/models/test_result.rb | 23 ++++++++++--------- ...1_add_ember_try_results_to_test_results.rb | 5 ++++ db/schema.rb | 1 + .../test_results_controller_test.rb | 9 ++++++++ test/factories/test_results.rb | 23 ++++++++++--------- test/models/test_result_test.rb | 23 ++++++++++--------- 7 files changed, 52 insertions(+), 33 deletions(-) create mode 100644 db/migrate/20190930010621_add_ember_try_results_to_test_results.rb diff --git a/app/controllers/test_results_controller.rb b/app/controllers/test_results_controller.rb index 520b0151..1e37619f 100644 --- a/app/controllers/test_results_controller.rb +++ b/app/controllers/test_results_controller.rb @@ -42,6 +42,7 @@ def create addon_version_id: build.addon_version.id, build_server: build.build_server, canary: build.canary?, + ember_try_results: @results ? JSON.generate(@results) : nil, output: output, output_format: output_format, semver_string: semver_string, diff --git a/app/models/test_result.rb b/app/models/test_result.rb index 4ba33e9d..5e27584a 100644 --- a/app/models/test_result.rb +++ b/app/models/test_result.rb @@ -4,17 +4,18 @@ # # Table name: test_results # -# id :integer not null, primary key -# addon_version_id :integer -# succeeded :boolean -# status_message :string -# created_at :datetime not null -# updated_at :datetime not null -# canary :boolean default(FALSE), not null -# build_server_id :integer -# semver_string :string -# output :text -# output_format :string default("text"), not null +# id :integer not null, primary key +# addon_version_id :integer +# succeeded :boolean +# status_message :string +# created_at :datetime not null +# updated_at :datetime not null +# canary :boolean default(FALSE), not null +# build_server_id :integer +# semver_string :string +# output :text +# output_format :string default("text"), not null +# ember_try_results :text # # Indexes # diff --git a/db/migrate/20190930010621_add_ember_try_results_to_test_results.rb b/db/migrate/20190930010621_add_ember_try_results_to_test_results.rb new file mode 100644 index 00000000..8e85629c --- /dev/null +++ b/db/migrate/20190930010621_add_ember_try_results_to_test_results.rb @@ -0,0 +1,5 @@ +class AddEmberTryResultsToTestResults < ActiveRecord::Migration[5.1] + def change + add_column :test_results, :ember_try_results, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 7335b2f5..660eeca6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -323,6 +323,7 @@ t.string "semver_string" t.text "output" t.string "output_format", default: "text", null: false + t.text "ember_try_results" t.index ["addon_version_id"], name: "index_test_results_on_addon_version_id" t.index ["build_server_id"], name: "index_test_results_on_build_server_id" end diff --git a/test/controllers/test_results_controller_test.rb b/test/controllers/test_results_controller_test.rb index 96d4ffe0..6b4812ee 100644 --- a/test/controllers/test_results_controller_test.rb +++ b/test/controllers/test_results_controller_test.rb @@ -161,6 +161,15 @@ class TestResultsControllerTest < ControllerTest assert_equal 'json', test_result.output_format end + test 'saves provided results' do + output = JSON.generate(foo: 'bar') + ember_try_results = build_test_result_string(1) + authed_post :create, pending_build_id: @pending_build.id, format: 'json', status: 'succeeded', results: ember_try_results, output: output + + test_result = TestResult.find_by(addon_version_id: @pending_build.addon_version_id) + assert_equal ember_try_results, test_result.ember_try_results + end + test "'retry' action responds with HTTP unauthorized if request..." do test_result = create(:test_result) diff --git a/test/factories/test_results.rb b/test/factories/test_results.rb index 8e2a4a6a..e535dd18 100644 --- a/test/factories/test_results.rb +++ b/test/factories/test_results.rb @@ -4,17 +4,18 @@ # # Table name: test_results # -# id :integer not null, primary key -# addon_version_id :integer -# succeeded :boolean -# status_message :string -# created_at :datetime not null -# updated_at :datetime not null -# canary :boolean default(FALSE), not null -# build_server_id :integer -# semver_string :string -# output :text -# output_format :string default("text"), not null +# id :integer not null, primary key +# addon_version_id :integer +# succeeded :boolean +# status_message :string +# created_at :datetime not null +# updated_at :datetime not null +# canary :boolean default(FALSE), not null +# build_server_id :integer +# semver_string :string +# output :text +# output_format :string default("text"), not null +# ember_try_results :text # # Indexes # diff --git a/test/models/test_result_test.rb b/test/models/test_result_test.rb index 2a1798c6..99f96b02 100644 --- a/test/models/test_result_test.rb +++ b/test/models/test_result_test.rb @@ -4,17 +4,18 @@ # # Table name: test_results # -# id :integer not null, primary key -# addon_version_id :integer -# succeeded :boolean -# status_message :string -# created_at :datetime not null -# updated_at :datetime not null -# canary :boolean default(FALSE), not null -# build_server_id :integer -# semver_string :string -# output :text -# output_format :string default("text"), not null +# id :integer not null, primary key +# addon_version_id :integer +# succeeded :boolean +# status_message :string +# created_at :datetime not null +# updated_at :datetime not null +# canary :boolean default(FALSE), not null +# build_server_id :integer +# semver_string :string +# output :text +# output_format :string default("text"), not null +# ember_try_results :text # # Indexes # From 1f9f2f054c295cba8f0c31c03033b187aead160c Mon Sep 17 00:00:00 2001 From: Phil Gengler Date: Sun, 29 Sep 2019 22:56:06 -0400 Subject: [PATCH 2/4] Add 'ember_try_results' to TestResult serializer --- app/resources/api/v2/test_result_resource.rb | 2 +- test/integration/api/v2/test_result_test.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/resources/api/v2/test_result_resource.rb b/app/resources/api/v2/test_result_resource.rb index ff65eed6..649796ef 100644 --- a/app/resources/api/v2/test_result_resource.rb +++ b/app/resources/api/v2/test_result_resource.rb @@ -2,7 +2,7 @@ class API::V2::TestResultResource < JSONAPI::Resource immutable - attributes :succeeded, :status_message, :created_at, :semver_string, :canary, :output, :output_format + attributes :ember_try_results, :succeeded, :status_message, :created_at, :semver_string, :canary, :output, :output_format has_one :version, class_name: 'Version', relation_name: 'addon_version', foreign_key: 'addon_version_id' has_many :ember_version_compatibilities diff --git a/test/integration/api/v2/test_result_test.rb b/test/integration/api/v2/test_result_test.rb index 0a22a252..00aece1e 100644 --- a/test/integration/api/v2/test_result_test.rb +++ b/test/integration/api/v2/test_result_test.rb @@ -4,6 +4,7 @@ class API::V2::ReviewTest < IntegrationTest TEST_RESULT_ATTRIBUTES = %w[ + ember-try-results succeeded status-message created-at From 6e962eb883844fa56f17c9bbc059e4b50601a830 Mon Sep 17 00:00:00 2001 From: Phil Gengler Date: Sat, 19 Oct 2019 06:46:19 -0400 Subject: [PATCH 3/4] As a novel idea, save JSON data in the database as a JSON type and not as a string --- app/controllers/test_results_controller.rb | 2 +- app/models/test_result.rb | 2 +- ...1_add_ember_try_results_to_test_results.rb | 2 +- db/schema.rb | 2 +- .../test_results_controller_test.rb | 40 +++++++++++-------- test/factories/test_results.rb | 2 +- test/models/test_result_test.rb | 2 +- 7 files changed, 30 insertions(+), 22 deletions(-) diff --git a/app/controllers/test_results_controller.rb b/app/controllers/test_results_controller.rb index 1e37619f..e93e4115 100644 --- a/app/controllers/test_results_controller.rb +++ b/app/controllers/test_results_controller.rb @@ -42,7 +42,7 @@ def create addon_version_id: build.addon_version.id, build_server: build.build_server, canary: build.canary?, - ember_try_results: @results ? JSON.generate(@results) : nil, + ember_try_results: @results, output: output, output_format: output_format, semver_string: semver_string, diff --git a/app/models/test_result.rb b/app/models/test_result.rb index 5e27584a..b5af3933 100644 --- a/app/models/test_result.rb +++ b/app/models/test_result.rb @@ -15,7 +15,7 @@ # semver_string :string # output :text # output_format :string default("text"), not null -# ember_try_results :text +# ember_try_results :jsonb # # Indexes # diff --git a/db/migrate/20190930010621_add_ember_try_results_to_test_results.rb b/db/migrate/20190930010621_add_ember_try_results_to_test_results.rb index 8e85629c..4c30194a 100644 --- a/db/migrate/20190930010621_add_ember_try_results_to_test_results.rb +++ b/db/migrate/20190930010621_add_ember_try_results_to_test_results.rb @@ -1,5 +1,5 @@ class AddEmberTryResultsToTestResults < ActiveRecord::Migration[5.1] def change - add_column :test_results, :ember_try_results, :text + add_column :test_results, :ember_try_results, :jsonb end end diff --git a/db/schema.rb b/db/schema.rb index 660eeca6..5d0c1a87 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -323,7 +323,7 @@ t.string "semver_string" t.text "output" t.string "output_format", default: "text", null: false - t.text "ember_try_results" + t.jsonb "ember_try_results" t.index ["addon_version_id"], name: "index_test_results_on_addon_version_id" t.index ["build_server_id"], name: "index_test_results_on_build_server_id" end diff --git a/test/controllers/test_results_controller_test.rb b/test/controllers/test_results_controller_test.rb index 6b4812ee..75020130 100644 --- a/test/controllers/test_results_controller_test.rb +++ b/test/controllers/test_results_controller_test.rb @@ -139,7 +139,7 @@ class TestResultsControllerTest < ControllerTest end test "captures provided output from param when 'format' is 'json'" do - output = JSON.generate(foo: 'bar') + output = { foo: 'bar' }.to_json authed_post :create, pending_build_id: @pending_build.id, format: 'json', status: 'succeeded', results: build_test_result_string(1), output: output test_result = TestResult.find_by(addon_version_id: @pending_build.addon_version_id) @@ -154,7 +154,7 @@ class TestResultsControllerTest < ControllerTest end test "saves value for 'format' param" do - output = JSON.generate(foo: 'bar') + output = { foo: 'bar' }.to_json authed_post :create, pending_build_id: @pending_build.id, format: 'json', status: 'succeeded', results: build_test_result_string(1), output: output test_result = TestResult.find_by(addon_version_id: @pending_build.addon_version_id) @@ -162,15 +162,15 @@ class TestResultsControllerTest < ControllerTest end test 'saves provided results' do - output = JSON.generate(foo: 'bar') - ember_try_results = build_test_result_string(1) + output = { foo: 'bar' }.to_json + ember_try_results = build_test_result_string(7) authed_post :create, pending_build_id: @pending_build.id, format: 'json', status: 'succeeded', results: ember_try_results, output: output test_result = TestResult.find_by(addon_version_id: @pending_build.addon_version_id) - assert_equal ember_try_results, test_result.ember_try_results + assert_equal 7, test_result.ember_try_results['scenarios'].count end - test "'retry' action responds with HTTP unauthorized if request..." do + test "'retry' action responds with HTTP unauthorized if request is not authenticated" do test_result = create(:test_result) post :retry, params: { id: test_result.id } @@ -214,16 +214,24 @@ def build_server end def build_test_result_string(num_scenarios) - scenario_string = build_scenarios(num_scenarios) - %({"scenarios":[#{scenario_string}]}) - end - - def build_scenarios(n) - scenarios = [] - n.times do |i| - scenarios << %({"scenarioName":"ember-2.#{i}","passed":true,"allowedToFail":false,"dependencies":[{"name":"ember","versionSeen":"2.#{i}.2","versionExpected":"~2.#{i}.0","type":"bower"}]}) - end - scenarios.join(',') + scenarios = Array.new(num_scenarios) { |i| build_scenario(i) } + { scenarios: scenarios }.to_json + end + + def build_scenario(i) + { + scenarioName: "ember-3.#{i}", + passed: true, + allowedToFail: false, + dependencies: [ + { + name: 'ember-source', + versionSeen: "3.#{i}.2", + versionExpected: "~3.#{i}.0", + type: 'yarn' + } + ] + } end def authed_post(action, data = nil) diff --git a/test/factories/test_results.rb b/test/factories/test_results.rb index e535dd18..a674c851 100644 --- a/test/factories/test_results.rb +++ b/test/factories/test_results.rb @@ -15,7 +15,7 @@ # semver_string :string # output :text # output_format :string default("text"), not null -# ember_try_results :text +# ember_try_results :jsonb # # Indexes # diff --git a/test/models/test_result_test.rb b/test/models/test_result_test.rb index 99f96b02..24532a60 100644 --- a/test/models/test_result_test.rb +++ b/test/models/test_result_test.rb @@ -15,7 +15,7 @@ # semver_string :string # output :text # output_format :string default("text"), not null -# ember_try_results :text +# ember_try_results :jsonb # # Indexes # From d3f919b2438cf81a46ad35e9f9ce070b40a26c03 Mon Sep 17 00:00:00 2001 From: Phil Gengler Date: Sat, 25 Jan 2020 19:00:03 -0500 Subject: [PATCH 4/4] Make @results less magic-y --- app/controllers/test_results_controller.rb | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/controllers/test_results_controller.rb b/app/controllers/test_results_controller.rb index e93e4115..981c3712 100644 --- a/app/controllers/test_results_controller.rb +++ b/app/controllers/test_results_controller.rb @@ -28,7 +28,7 @@ def create return end - if succeeded? && !verify_test_results(params[:results]) + if succeeded? && !verify_test_results head :unprocessable_entity return end @@ -42,7 +42,7 @@ def create addon_version_id: build.addon_version.id, build_server: build.build_server, canary: build.canary?, - ember_try_results: @results, + ember_try_results: results, output: output, output_format: output_format, semver_string: semver_string, @@ -93,15 +93,17 @@ def succeeded? params[:status] == 'succeeded' end - def verify_test_results(results_str) - return false unless results_str - begin - @results = JSON.parse(results_str) + def results + @results ||= begin + JSON.parse(params[:results] || '') rescue JSON::ParserError - return false + return nil end + end - return false if @results['scenarios'].empty? + def verify_test_results + return false if results.nil? + return false if results['scenarios'].empty? true end