Skip to content

Commit

Permalink
Assert the logger works in Capistrano
Browse files Browse the repository at this point in the history
Follow up on the previous commits to remove the logger instance from the
Config class, which was set by Capistrano.

It didn't actually log anything before, as it wrote to a `StringIO.new`.
Log now to the configured logger based on the application's config.

I see no harm in making the logger work. By default it will log to the
`appsignal.log` file. If apps have configured it to log to stdout, they
have deliberately configured that so there should be no problem.
  • Loading branch information
tombruijn committed Sep 26, 2024
1 parent e671a35 commit 6ae1c36
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
8 changes: 7 additions & 1 deletion spec/lib/appsignal/capistrano2_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
let(:out_stream) { std_stream }
let(:output) { out_stream.read }
let(:config) { build_config }
let(:log_stream) { StringIO.new }
let(:logs) { log_contents(log_stream) }
let(:capistrano_config) do
Capistrano::Configuration.new.tap do |c|
c.set(:rails_env, "production")
Expand All @@ -17,7 +19,10 @@
c.dry_run = false
end
end
before { Appsignal::Integrations::Capistrano.tasks(capistrano_config) }
before do
Appsignal.internal_logger = test_logger(log_stream)
Appsignal::Integrations::Capistrano.tasks(capistrano_config)
end

def run
capture_stdout(out_stream) do
Expand Down Expand Up @@ -125,6 +130,7 @@ def run
run
expect(output).to include \
"Not notifying of deploy, config is not active for environment: production"
expect(logs).to contains_log(:error, "Push API key not set after loading config")
end
end
end
Expand Down
10 changes: 7 additions & 3 deletions spec/lib/appsignal/capistrano3_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
let(:config) { build_config(:env => env, :options => options) }
let(:out_stream) { std_stream }
let(:output) { out_stream.read }
let(:logger) { Logger.new(out_stream) }
let(:log_stream) { StringIO.new }
let(:logs) { log_contents(log_stream) }
let!(:capistrano_config) do
Capistrano::Configuration.reset!
Capistrano::Configuration.env.tap do |c|
c.set(:log_level, :error)
c.set(:logger, logger)
c.set(:rails_env, "production")
c.set(:repository, "main")
c.set(:deploy_to, "/home/username/app")
Expand All @@ -29,7 +29,10 @@
:user => "batman"
}
end
before { Rake::Task["appsignal:deploy"].reenable }
before do
Appsignal.internal_logger = test_logger(log_stream)
Rake::Task["appsignal:deploy"].reenable
end

def run
capture_std_streams(out_stream, out_stream) do
Expand Down Expand Up @@ -141,6 +144,7 @@ def run
run
expect(output).to include \
"Not notifying of deploy, config is not active for environment: production"
expect(logs).to contains_log(:error, "Push API key not set after loading config")
end
end
end
Expand Down

0 comments on commit 6ae1c36

Please sign in to comment.