From 5d12e53242c3bf56d6c024207e8a93323902576c Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Thu, 11 Apr 2024 14:55:46 -0700 Subject: [PATCH] Refactored test setup into real-world fixes --- .iex.exs | 3 - lib/pinchflat/killed_worker.ex | 58 ------------------- lib/pinchflat/notifications/command_runner.ex | 2 +- lib/pinchflat/utils/cli_utils.ex | 17 ++++++ lib/pinchflat/yt_dlp/command_runner.ex | 2 +- wrapper.sh => priv/cmd_wrapper.sh | 4 ++ slow.sh | 8 --- test/pinchflat/utils/cli_utils_test.exs | 6 ++ 8 files changed, 29 insertions(+), 71 deletions(-) delete mode 100644 lib/pinchflat/killed_worker.ex rename wrapper.sh => priv/cmd_wrapper.sh (73%) delete mode 100755 slow.sh diff --git a/.iex.exs b/.iex.exs index da010ee..ebe1a24 100644 --- a/.iex.exs +++ b/.iex.exs @@ -22,7 +22,4 @@ alias Pinchflat.Metadata.MetadataFileHelpers alias Pinchflat.SlowIndexing.FileFollowerServer -# TODO: remove -alias Pinchflat.KilledWorker - Pinchflat.Release.check_file_permissions() diff --git a/lib/pinchflat/killed_worker.ex b/lib/pinchflat/killed_worker.ex deleted file mode 100644 index 72fdadd..0000000 --- a/lib/pinchflat/killed_worker.ex +++ /dev/null @@ -1,58 +0,0 @@ -defmodule Pinchflat.KilledWorker do - @moduledoc false - - use Oban.Worker, - queue: :default - - import Ecto.Query, warn: false - - require Logger - - alias __MODULE__ - alias Pinchflat.Repo - - def kickoff(job_args \\ %{}, opts \\ []) do - job_args - |> KilledWorker.new(opts) - |> Repo.insert_unique_job() - - :ok - end - - def cancel do - Oban.Job - |> where(worker: "Pinchflat.KilledWorker") - |> Oban.cancel_all_jobs() - end - - def start_stop do - kickoff() - Process.sleep(2000) - cancel() - end - - @impl Oban.Worker - def perform(%Oban.Job{}) do - # case System.cmd("/app/wrapper.sh", ["/app/slow.sh"]) do - args = [ - "/usr/local/bin/yt-dlp", - "https://www.youtube.com/@OverSimplified", - "--simulate", - "--print", - "%(title)s" - ] - - case System.cmd("/app/wrapper.sh", args) do - {output, 0} -> - Logger.warning("KilledWorker: #{output}") - {:ok, output} - - {output, _} -> - Logger.error("KilledWorker: #{output}") - {:error, output} - end - - Logger.warning("KilledWorker: done") - :ok - end -end diff --git a/lib/pinchflat/notifications/command_runner.ex b/lib/pinchflat/notifications/command_runner.ex index 8f1a806..5f51823 100644 --- a/lib/pinchflat/notifications/command_runner.ex +++ b/lib/pinchflat/notifications/command_runner.ex @@ -45,7 +45,7 @@ defmodule Pinchflat.Notifications.CommandRunner do """ @impl AppriseCommandRunner def version do - case System.cmd(backend_executable(), ["--version"]) do + case CliUtils.wrap_cmd(backend_executable(), ["--version"]) do {output, 0} -> output |> String.split(~r{\r?\n}) diff --git a/lib/pinchflat/utils/cli_utils.ex b/lib/pinchflat/utils/cli_utils.ex index 4e2a488..8d6c1d4 100644 --- a/lib/pinchflat/utils/cli_utils.ex +++ b/lib/pinchflat/utils/cli_utils.ex @@ -5,6 +5,23 @@ defmodule Pinchflat.Utils.CliUtils do alias Pinchflat.Utils.StringUtils + @doc """ + Wraps a command in a shell script that will terminate + the command if stdin is closed. Useful for stopping + commands if the job runner is cancelled. + + Delegates to `System.cmd/3` and any options/output + are passed through. + + Returns {binary(), integer()} + """ + def wrap_cmd(command, args, opts \\ []) do + wrapper_command = Path.join(:code.priv_dir(:pinchflat), "cmd_wrapper.sh") + actual_command = [command] ++ args + + System.cmd(wrapper_command, actual_command, opts) + end + @doc """ Parses a list of command options into a list of strings suitable for passing to `System.cmd/3`. diff --git a/lib/pinchflat/yt_dlp/command_runner.ex b/lib/pinchflat/yt_dlp/command_runner.ex index ed13d33..572c3ac 100644 --- a/lib/pinchflat/yt_dlp/command_runner.ex +++ b/lib/pinchflat/yt_dlp/command_runner.ex @@ -37,7 +37,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do formatted_command_opts = [url] ++ CliUtils.parse_options(all_opts) Logger.info("[yt-dlp] called with: #{Enum.join(formatted_command_opts, " ")}") - case System.cmd(command, formatted_command_opts, stderr_to_stdout: true) do + case CliUtils.wrap_cmd(command, formatted_command_opts, stderr_to_stdout: true) do {_, 0} -> # IDEA: consider deleting the file after reading it. It's in the tmp dir, so it's not # a huge deal, but it's still a good idea to clean up after ourselves. diff --git a/wrapper.sh b/priv/cmd_wrapper.sh similarity index 73% rename from wrapper.sh rename to priv/cmd_wrapper.sh index 6121c23..fe5df29 100755 --- a/wrapper.sh +++ b/priv/cmd_wrapper.sh @@ -1,5 +1,9 @@ #!/usr/bin/env bash +# This script is a wrapper for other programs +# that ensures they are killed when stdin closes +# (eg: a job terminates) + # Start the program in the background exec "$@" & pid1=$! diff --git a/slow.sh b/slow.sh deleted file mode 100755 index dcbae27..0000000 --- a/slow.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash - -# Runs for N seconds -for i in {1..15}; do - echo "Slow script running for $i seconds" - sleep 1 -done -echo "Slow script done" diff --git a/test/pinchflat/utils/cli_utils_test.exs b/test/pinchflat/utils/cli_utils_test.exs index b053158..0c676b7 100644 --- a/test/pinchflat/utils/cli_utils_test.exs +++ b/test/pinchflat/utils/cli_utils_test.exs @@ -3,6 +3,12 @@ defmodule Pinchflat.Utils.CliUtilsTest do alias Pinchflat.Utils.CliUtils + describe "wrap_cmd/3" do + test "delegates to System.cmd/3" do + assert {"output\n", 0} = CliUtils.wrap_cmd("echo", ["output"]) + end + end + describe "parse_options/1" do test "it converts symbol k-v arg keys to kebab case" do assert ["--buffer-size", "1024"] = CliUtils.parse_options(buffer_size: 1024)