Skip to content

Commit 406bc4e

Browse files
author
Glyn Normington
committed
Improved logging robustness and handling in tests
Make the logging code more robust and add diagnostics. Use before(:all) in spec_helper.rb to ensure a normal logger is present. This avoids example groups having to restore a normal logger if they have created a special logger. [#56061680] Suppress output from download_cache_spec.rb Create logger based on StringIO $stderr to avoid logs appearing on STDERR. Set $stdout to a StringIO to avoid download failures appearing on STDOUT. [#56330430]
1 parent d9c480c commit 406bc4e

6 files changed

Lines changed: 80 additions & 35 deletions

File tree

‎lib/java_buildpack/diagnostics/logger_factory.rb‎

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ def self.create_logger(app_dir)
3434
FileUtils.mkdir_p diagnostics_directory
3535
log_file = File.join(diagnostics_directory, JavaBuildpack::Diagnostics::LOG_FILE_NAME)
3636

37-
if (defined? @@logger) && (!@@logger.nil?)
37+
if (defined? @@logger) && !@@logger.nil?
3838
logger_recreated = true
39-
@@logger.warn("Logger is being re-created by #{caller[0]}")
39+
get_logger.warn("Logger is being re-created by #{caller[0]}")
4040
else
4141
logger_recreated = false
4242
end
@@ -47,20 +47,25 @@ def self.create_logger(app_dir)
4747

4848
set_log_level
4949

50-
@@logger.debug(log_file)
50+
get_logger.debug(log_file)
51+
get_logger.debug { "Logger creation stack: #{caller.join("\n")}" }
5152
if logger_recreated
52-
@@logger.warn("Logger was re-created by #{caller[0]}")
53+
get_logger.warn("Logger was re-created by #{caller[0]}")
5354
end
54-
@@logger
55+
get_logger
5556
end
5657

5758
# Gets the current logger instance.
5859
#
59-
# @return [Logger, nil] the current Logger instance or `nil` if there is no such instance
60+
# @return [Logger] the current Logger instance
61+
# @raise if there is no current logger instance
6062
def self.get_logger
61-
@@monitor.synchronize do
62-
@@logger
63+
logger = get_logger_internal
64+
unless logger
65+
STDERR.puts "Attempt to get nil logger from: #{caller}"
66+
raise 'no logger'
6367
end
68+
logger
6469
end
6570

6671
private_class_method :new
@@ -85,25 +90,31 @@ def self.get_logger
8590

8691
@@monitor = Monitor.new
8792

93+
def self.get_logger_internal
94+
@@monitor.synchronize do
95+
(defined? @@logger) ? @@logger : nil # rubocop:disable ParenthesesAroundCondition
96+
end
97+
end
98+
8899
def self.set_log_level
89100
logging_configuration = get_configuration
90101
switched_log_level = $VERBOSE || $DEBUG ? DEBUG_SEVERITY_STRING : nil
91102
log_level = (ENV[LOG_LEVEL_ENVIRONMENT_VARIABLE] || switched_log_level || logging_configuration[DEFAULT_LOG_LEVEL_CONFIGURATION_KEY]).upcase
92103

93-
@@logger.sev_threshold = case
94-
when log_level == DEBUG_SEVERITY_STRING then
95-
::Logger::DEBUG
96-
when log_level == INFO_SEVERITY_STRING then
97-
::Logger::INFO
98-
when log_level == WARN_SEVERITY_STRING then
99-
::Logger::WARN
100-
when log_level == ERROR_SEVERITY_STRING then
101-
::Logger::ERROR
102-
when log_level == FATAL_SEVERITY_STRING then
103-
::Logger::FATAL
104-
else
105-
::Logger::DEBUG
106-
end
104+
get_logger.sev_threshold = case
105+
when log_level == DEBUG_SEVERITY_STRING then
106+
::Logger::DEBUG
107+
when log_level == INFO_SEVERITY_STRING then
108+
::Logger::INFO
109+
when log_level == WARN_SEVERITY_STRING then
110+
::Logger::WARN
111+
when log_level == ERROR_SEVERITY_STRING then
112+
::Logger::ERROR
113+
when log_level == FATAL_SEVERITY_STRING then
114+
::Logger::FATAL
115+
else
116+
::Logger::DEBUG
117+
end
107118
end
108119

109120
def self.get_configuration
@@ -113,7 +124,10 @@ def self.get_configuration
113124

114125
def self.close
115126
@@monitor.synchronize do
116-
@@logger = nil
127+
if (defined? @@logger) && @@logger
128+
@@logger.debug { "Logger close stack: #{caller.join("\n")}" }
129+
@@logger = nil
130+
end
117131
end
118132
end
119133

‎spec/java_buildpack/buildpack_spec.rb‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@ module JavaBuildpack
5050
Test::StubJre1.stub(:new).and_return(stub_jre1)
5151
Test::StubJre2.stub(:new).and_return(stub_jre2)
5252

53+
JavaBuildpack::Diagnostics::LoggerFactory.send :close
5354
$stderr = StringIO.new
55+
tmpdir = Dir.tmpdir
56+
diagnostics_directory = File.join(tmpdir, JavaBuildpack::Diagnostics::DIAGNOSTICS_DIRECTORY)
57+
FileUtils.rm_rf diagnostics_directory
58+
JavaBuildpack::Diagnostics::LoggerFactory.create_logger tmpdir
5459
end
5560

5661
it 'should raise an error if more than one container can run an application' do

‎spec/java_buildpack/diagnostics/logger_factory_spec.rb‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ module JavaBuildpack::Diagnostics
2424
LOG_MESSAGE = 'a log message'
2525

2626
before do
27+
JavaBuildpack::Diagnostics::LoggerFactory.send :close
2728
$stderr = StringIO.new
29+
tmpdir = Dir.tmpdir
30+
diagnostics_directory = File.join(tmpdir, JavaBuildpack::Diagnostics::DIAGNOSTICS_DIRECTORY)
31+
FileUtils.rm_rf diagnostics_directory
32+
JavaBuildpack::Diagnostics::LoggerFactory.create_logger tmpdir
2833
end
2934

3035
it 'should create a logger' do
@@ -146,10 +151,15 @@ module JavaBuildpack::Diagnostics
146151
it 'should take the default log level from a YAML file' do
147152
YAML.stub(:load_file).with(File.expand_path('config/logging.yml')).and_return(
148153
'default_log_level' => 'DEBUG')
149-
Dir.mktmpdir do |app_dir|
150-
logger = new_logger app_dir
151-
logger.debug(LOG_MESSAGE)
152-
expect($stderr.string).to match(/#{LOG_MESSAGE}/)
154+
begin
155+
Dir.mktmpdir do |app_dir|
156+
logger = new_logger app_dir
157+
logger.debug(LOG_MESSAGE)
158+
expect($stderr.string).to match(/#{LOG_MESSAGE}/)
159+
end
160+
ensure
161+
YAML.stub(:load_file).with(File.expand_path('config/logging.yml')).and_return(
162+
'default_log_level' => 'INFO')
153163
end
154164
end
155165

‎spec/java_buildpack/jre/memory/weight_balancing_memory_heuristic_spec.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ module JavaBuildpack::Jre
4747

4848
before do
4949
JavaBuildpack::Diagnostics::LoggerFactory.send :close # suppress warnings
50+
$stderr = StringIO.new
5051
File.delete(JavaBuildpack::Diagnostics.get_buildpack_log Dir.tmpdir)
5152
JavaBuildpack::Diagnostics::LoggerFactory.create_logger Dir.tmpdir
5253
end

‎spec/java_buildpack/util/download_cache_spec.rb‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@ module JavaBuildpack::Util
2323
describe DownloadCache do
2424

2525
before do
26+
JavaBuildpack::Diagnostics::LoggerFactory.send :close
2627
$stderr = StringIO.new
28+
tmpdir = Dir.tmpdir
29+
diagnostics_directory = File.join(tmpdir, JavaBuildpack::Diagnostics::DIAGNOSTICS_DIRECTORY)
30+
FileUtils.rm_rf diagnostics_directory
31+
JavaBuildpack::Diagnostics::LoggerFactory.create_logger tmpdir
32+
$stdout = StringIO.new
2733
end
2834

2935
it 'should download from a uri if the cached file does not exist' do

‎spec/spec_helper.rb‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,27 @@
2424

2525
require 'tmpdir'
2626
require 'webmock/rspec'
27+
require 'fileutils'
28+
require 'java_buildpack/diagnostics/common'
29+
require 'java_buildpack/diagnostics/logger_factory'
2730

2831
RSpec.configure do |config|
2932
config.treat_symbols_as_metadata_keys_with_true_values = true
3033
config.run_all_when_everything_filtered = true
3134
config.filter_run :focus
35+
config.before(:all) do
36+
# Ensure a logger exists before each example group is run. Example groups then do not need to tidy up if they
37+
# have created a special logger.
38+
JavaBuildpack::Diagnostics::LoggerFactory.send :close # avoid warning if logger already exists
39+
tmpdir = Dir.tmpdir
40+
diagnostics_directory = File.join(tmpdir, JavaBuildpack::Diagnostics::DIAGNOSTICS_DIRECTORY)
41+
FileUtils.rm_rf diagnostics_directory
42+
JavaBuildpack::Diagnostics::LoggerFactory.create_logger tmpdir
43+
end
44+
config.after(:all) do
45+
# Reset stream variables that tests may have modified.
46+
$stderr = STDERR
47+
$stdout = STDOUT
48+
end
3249
end
3350

34-
# Ensure a logger exists for any class under test that needs one.
35-
require 'fileutils'
36-
require 'java_buildpack/diagnostics/common'
37-
require 'java_buildpack/diagnostics/logger_factory'
38-
tmpdir = Dir.tmpdir
39-
diagnostics_directory = File.join(tmpdir, JavaBuildpack::Diagnostics::DIAGNOSTICS_DIRECTORY)
40-
FileUtils.rm_rf diagnostics_directory
41-
JavaBuildpack::Diagnostics::LoggerFactory.create_logger tmpdir

0 commit comments

Comments
 (0)