From 3736d66f17e3e638c74e9363381d6f41e9a12ca6 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Tue, 27 Feb 2024 11:33:28 +0100 Subject: [PATCH] DEV: Extensively use `exception: true` in `system()` (#25911) Specifically fixes a bug in smoke-test where it would just move on after failing to install latest js dependencies with yarn. --- lib/tasks/annotate.rake | 11 +++++++---- lib/tasks/db.rake | 9 ++++++--- lib/tasks/javascript.rake | 7 +++---- lib/tasks/qunit.rake | 10 ++++++++-- lib/tasks/smoke_test.rake | 2 +- lib/tasks/svg_icons.rake | 3 +-- migrations/spec/import_spec.rb | 2 +- 7 files changed, 27 insertions(+), 17 deletions(-) diff --git a/lib/tasks/annotate.rake b/lib/tasks/annotate.rake index 57bc0129ac3..56551659d88 100644 --- a/lib/tasks/annotate.rake +++ b/lib/tasks/annotate.rake @@ -2,7 +2,7 @@ desc "ensure the asynchronously-created post_search_data index is present" task "annotate" => :environment do |task, args| - raise if !system "bin/annotate --models" + system("bin/annotate --models", exception: true) STDERR.puts "Annotate executed successfully" non_core_plugins = @@ -27,9 +27,12 @@ task "annotate:clean" => :environment do |task, args| db = TemporaryDb.new db.start db.with_env do - raise if !system "RAILS_ENV=test LOAD_PLUGINS=0 bin/rake db:migrate" - raise if !system "RAILS_ENV=test LOAD_PLUGINS=0 bin/rake annotate:ensure_all_indexes" - raise if !system "RAILS_ENV=test LOAD_PLUGINS=0 bin/annotate --models --model-dir app/models" + system("RAILS_ENV=test LOAD_PLUGINS=0 bin/rake db:migrate", exception: true) + system("RAILS_ENV=test LOAD_PLUGINS=0 bin/rake annotate:ensure_all_indexes", exception: true) + system( + "RAILS_ENV=test LOAD_PLUGINS=0 bin/annotate --models --model-dir app/models", + exception: true, + ) end STDERR.puts "Annotate executed successfully" ensure diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index 006a6a06ea0..4199f981d72 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -27,7 +27,10 @@ end task "db:environment:set" => [:load_config] do |_, args| if MultisiteTestHelpers.load_multisite? - system("RAILS_ENV=test RAILS_DB=discourse_test_multisite rake db:environment:set") + system( + "RAILS_ENV=test RAILS_DB=discourse_test_multisite rake db:environment:set", + exception: true, + ) end end @@ -56,7 +59,7 @@ end task "db:drop" => [:load_config] do |_, args| if MultisiteTestHelpers.create_multisite? - system("RAILS_DB=discourse_test_multisite RAILS_ENV=test rake db:drop") + system("RAILS_DB=discourse_test_multisite RAILS_ENV=test rake db:drop", exception: true) end end @@ -266,7 +269,7 @@ task "db:migrate" => %w[ end if !Discourse.is_parallel_test? && MultisiteTestHelpers.load_multisite? - system("RAILS_DB=discourse_test_multisite rake db:migrate") + system("RAILS_DB=discourse_test_multisite rake db:migrate", exception: true) end end diff --git a/lib/tasks/javascript.rake b/lib/tasks/javascript.rake index 0e63464091d..2ed9dfcb428 100644 --- a/lib/tasks/javascript.rake +++ b/lib/tasks/javascript.rake @@ -46,7 +46,7 @@ def write_template(path, task_name, template) File.write(output_path, "#{header}\n\n#{template}") puts "#{basename} created" - `yarn run prettier --write #{output_path}` + system("yarn run prettier --write #{output_path}", exception: true) puts "#{basename} prettified" end @@ -59,7 +59,7 @@ def write_hbs_template(path, task_name, template) basename = File.basename(path) output_path = "#{Rails.root}/app/assets/javascripts/#{path}" File.write(output_path, "#{header}\n#{template}") - `yarn run prettier --write #{output_path}` + system("yarn run prettier --write #{output_path}", exception: true) puts "#{basename} created" end @@ -203,8 +203,7 @@ end task "javascript:update" => "clean_up" do require "uglifier" - yarn = system("yarn install") - abort('Unable to run "yarn install"') unless yarn + system("yarn install", exception: true) versions = {} start = Time.now diff --git a/lib/tasks/qunit.rake b/lib/tasks/qunit.rake index 51eb704b140..c8fe5bf123b 100644 --- a/lib/tasks/qunit.rake +++ b/lib/tasks/qunit.rake @@ -17,7 +17,7 @@ task "qunit:test", %i[timeout qunit_path filter] do |_, args| report_requests = ENV["REPORT_REQUESTS"] == "1" - system("yarn install") + system("yarn install", exception: true) # ensure we have this port available def port_available?(port) @@ -108,7 +108,13 @@ task "qunit:test", %i[timeout qunit_path filter] do |_, args| if qunit_path # Bypass `ember test` - it only works properly for the `/tests` path. # We have to trigger a `build` manually so that JS is available for rails to serve. - system("yarn", "ember", "build", chdir: "#{Rails.root}/app/assets/javascripts/discourse") + system( + "yarn", + "ember", + "build", + chdir: "#{Rails.root}/app/assets/javascripts/discourse", + exception: true, + ) env["THEME_TEST_PAGES"] = if ENV["THEME_IDS"] ENV["THEME_IDS"] diff --git a/lib/tasks/smoke_test.rake b/lib/tasks/smoke_test.rake index cab344e3946..46739a268b3 100644 --- a/lib/tasks/smoke_test.rake +++ b/lib/tasks/smoke_test.rake @@ -10,7 +10,7 @@ task "smoke:test" do abort err.message end - system("yarn install") + system("yarn install", exception: true) url = ENV["URL"] if !url diff --git a/lib/tasks/svg_icons.rake b/lib/tasks/svg_icons.rake index 7f443447091..7255faba322 100644 --- a/lib/tasks/svg_icons.rake +++ b/lib/tasks/svg_icons.rake @@ -9,8 +9,7 @@ def library_src end task "svgicons:update" do - yarn = system("yarn install") - abort('Unable to run "yarn install"') unless yarn + system("yarn install", exception: true) dependencies = [{ source: "@fortawesome/fontawesome-free/sprites", destination: "fontawesome" }] diff --git a/migrations/spec/import_spec.rb b/migrations/spec/import_spec.rb index 683b2d32d0c..1e40bf5cdef 100644 --- a/migrations/spec/import_spec.rb +++ b/migrations/spec/import_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe "Migrations::Import" do - subject(:cli) { system "migrations/import" } + subject(:cli) { system("migrations/import", exception: true) } it "works" do expect { cli }.to output(