Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the guts of the CLI into a class for easier testing #220

Merged
merged 1 commit into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 3 additions & 72 deletions exe/floe
Original file line number Diff line number Diff line change
Expand Up @@ -3,75 +3,6 @@

$LOAD_PATH.unshift(File.expand_path("../lib", __dir__))

require "optimist"
require "floe"
require "floe/container_runner"

opts = Optimist.options do
version("v#{Floe::VERSION}\n")
usage("[options] workflow input [workflow2 input2]")

opt :workflow, "Path to your workflow json file (alternative to passing a bare workflow)", :type => :string
opt :input, <<~EOMSG, :type => :string
JSON payload of the Input to the workflow
If --input is passed and --workflow is not passed, will be used for all bare workflows listed.
If --input is not passed and --workflow is passed, defaults to '{}'.
EOMSG
opt :context, "JSON payload of the Context", :type => :string
opt :credentials, "JSON payload with Credentials", :type => :string
opt :credentials_file, "Path to a file with Credentials", :type => :string

Floe::ContainerRunner.cli_options(self)

banner("")
banner("General options:")
end

# Create workflow/input pairs from the various combinations of paramaters
args =
if opts[:workflow_given]
Optimist.die("cannot specify both --workflow and bare workflows") if ARGV.any?

[opts[:workflow], opts.fetch(:input, "{}")]
elsif opts[:input_given]
Optimist.die("workflow(s) must be specified") if ARGV.empty?

ARGV.flat_map { |w| [w, opts[:input].dup] }
else
Optimist.die("workflow/input pairs must be specified") if ARGV.empty? || (ARGV.size > 1 && ARGV.size.odd?)

ARGV
end

Floe::ContainerRunner.resolve_cli_options!(opts)

require "logger"
Floe.logger = Logger.new($stdout)

credentials =
if opts[:credentials_given]
opts[:credentials] == "-" ? $stdin.read : opts[:credentials]
elsif opts[:credentials_file_given]
File.read(opts[:credentials_file])
end

workflows =
args.each_slice(2).map do |workflow, input|
context = Floe::Workflow::Context.new(opts[:context], :input => input, :credentials => credentials)
Floe::Workflow.load(workflow, context)
end

# run

Floe::Workflow.wait(workflows, &:run_nonblock)

# display status

workflows.each do |workflow|
puts "", "#{workflow.name}#{" (#{workflow.status})" unless workflow.context.success?}", "===" if workflows.size > 1
puts workflow.output.inspect
end

# exit status

exit workflows.all? { |workflow| workflow.context.success? } ? 0 : 1
require "floe/cli"
success = Floe::CLI.new.run(ARGV)
exit(success ? 0 : 1)
83 changes: 83 additions & 0 deletions lib/floe/cli.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
module Floe
class CLI
def initialize
require "optimist"
require "floe"
require "floe/container_runner"
require "logger"

Floe.logger = Logger.new($stdout)
end

def run(args = ARGV)
workflows_inputs, opts = parse_options!(args)

credentials =
if opts[:credentials_given]
opts[:credentials] == "-" ? $stdin.read : opts[:credentials]
elsif opts[:credentials_file_given]
File.read(opts[:credentials_file])
end

workflows =
workflows_inputs.each_slice(2).map do |workflow, input|
context = Floe::Workflow::Context.new(opts[:context], :input => input, :credentials => credentials)
Floe::Workflow.load(workflow, context)
end

Floe::Workflow.wait(workflows, &:run_nonblock)

# Display status
workflows.each do |workflow|
puts "", "#{workflow.name}#{" (#{workflow.status})" unless workflow.context.success?}", "===" if workflows.size > 1
puts workflow.output.inspect
end

workflows.all? { |workflow| workflow.context.success? }
end

private

def parse_options!(args)
opts = Optimist.options(args) do
version("v#{Floe::VERSION}\n")
usage("[options] workflow input [workflow2 input2]")

opt :workflow, "Path to your workflow json file (alternative to passing a bare workflow)", :type => :string
opt :input, <<~EOMSG, :type => :string
JSON payload of the Input to the workflow
If --input is passed and --workflow is not passed, will be used for all bare workflows listed.
If --input is not passed and --workflow is passed, defaults to '{}'.
EOMSG
opt :context, "JSON payload of the Context", :type => :string
opt :credentials, "JSON payload with Credentials", :type => :string
opt :credentials_file, "Path to a file with Credentials", :type => :string

Floe::ContainerRunner.cli_options(self)

banner("")
banner("General options:")
end

# Create workflow/input pairs from the various combinations of paramaters
workflows_inputs =
if opts[:workflow_given]
Optimist.die("cannot specify both --workflow and bare workflows") if args.any?
Copy link
Member Author

@Fryguy Fryguy Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that this is in a class, these Optimist.die statements actually raise a SystemExit, which means this is kind of gross. I started refactoring to use an Optimist::Parser, but then this code started getting too complicated, so I left the PR as is to be a more pure moving of the code. We can follow up with better cleanup later if needed (or not - it's a CLI class after all).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I added raise SystemExit so we could test for this case (rather than using exit)


[opts[:workflow], opts.fetch(:input, "{}")]
elsif opts[:input_given]
Optimist.die("workflow(s) must be specified") if args.empty?

args.flat_map { |w| [w, opts[:input].dup] }
else
Optimist.die("workflow/input pairs must be specified") if args.empty? || (args.size > 1 && args.size.odd?)

args
end

Floe::ContainerRunner.resolve_cli_options!(opts)

return workflows_inputs, opts
end
end
end
146 changes: 146 additions & 0 deletions spec/cli_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
require "floe/cli"

RSpec.describe Floe::CLI do
describe "#run" do
let(:workflow) { File.expand_path("data/workflow.asl", __dir__) }

it "displays help" do
output, _error, result = run_cli("--help")
expect(result).to be true

lines = output.lines(:chomp => true)
expect(lines).to start_with("Usage: #{File.basename($PROGRAM_NAME)} [options] workflow input [workflow2 input2]")
expect(lines).to include("v#{Floe::VERSION}")

# it should also include options from runners
expect(lines).to include("Container runner options:")
end

it "with no parameters" do
_output, error, result = run_cli
expect(result).to be false

lines = error.lines(:chomp => true)
expect(lines.first).to eq("Error: workflow/input pairs must be specified.")
end

it "with a bare workflow and no input" do
output, _error, result = run_cli(workflow)
expect(result).to be true

lines = output.lines(:chomp => true)
expect(lines.first).to include("checking 1 workflows...")
expect(lines.last).to eq("{}")
end

it "with a bare workflow and input" do
output, _error, result = run_cli(workflow, '{"foo":1}')
expect(result).to be true

lines = output.lines(:chomp => true)
expect(lines.first).to include("checking 1 workflows...")
expect(lines.last).to eq('{"foo"=>1}')
end

it "with a bare workflow and --input" do
output, _error, result = run_cli(workflow, "--input", '{"foo":1}')
expect(result).to be true

lines = output.lines(:chomp => true)
expect(lines.first).to include("checking 1 workflows...")
expect(lines.last).to eq('{"foo"=>1}')
end

it "with --workflow and no input" do
output, _error, result = run_cli("--workflow", workflow)
expect(result).to be true

lines = output.lines(:chomp => true)
expect(lines.first).to include("checking 1 workflows...")
expect(lines.last).to eq("{}")
end

it "with --workflow and --input" do
output, _error, result = run_cli("--workflow", workflow, "--input", '{"foo":1}')
expect(result).to be true

lines = output.lines(:chomp => true)
expect(lines.first).to include("checking 1 workflows...")
expect(lines.last).to eq('{"foo"=>1}')
end

it "with a bare workflow and --workflow" do
_output, error, result = run_cli(workflow, "--workflow", workflow)
expect(result).to be false

lines = error.lines(:chomp => true)
expect(lines.first).to eq("Error: cannot specify both --workflow and bare workflows.")
end

it "with --input but no workflows" do
_output, error, result = run_cli("--input", '{"foo":1}')
expect(result).to be false

lines = error.lines(:chomp => true)
expect(lines.first).to eq("Error: workflow(s) must be specified.")
end

it "with multiple bare workflow/input pairs" do
output, _error, result = run_cli(workflow, '{"foo":1}', workflow, '{"foo":2}')
expect(result).to be true

lines = output.lines(:chomp => true)
expect(lines.first).to include("checking 2 workflows...")
expect(lines.last(7).join("\n")).to eq(<<~OUTPUT.chomp)
workflow
===
{"foo"=>1}

workflow
===
{"foo"=>2}
OUTPUT
end

it "with multiple bare workflows and --input" do
output, _error, result = run_cli(workflow, workflow, "--input", '{"foo":1}')
expect(result).to be true

lines = output.lines(:chomp => true)
expect(lines.first).to include("checking 2 workflows...")
expect(lines.last(7).join("\n")).to eq(<<~OUTPUT.chomp)
workflow
===
{"foo"=>1}

workflow
===
{"foo"=>1}
OUTPUT
end

it "with mismatched workflow/input pairs" do
_output, error, result = run_cli(workflow, workflow, '{"foo":2}')
expect(result).to be false

lines = error.lines(:chomp => true)
expect(lines.first).to eq("Error: workflow/input pairs must be specified.")
end

def run_cli(*args)
capture_io { described_class.new.run(args) }
end

def capture_io
output, error = StringIO.new, StringIO.new
stdout, stderr = $stdout, $stderr
$stdout, $stderr = output, error
result = yield
return output.string, error.string, result
rescue SystemExit => err
return output.string, error.string, err.success?
ensure
$stdout, $stderr = stdout, stderr
end
end
end
File renamed without changes.
Loading