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

Facter tests mutate the global logger state #2721

Closed
joshcooper opened this issue May 21, 2024 · 1 comment · Fixed by #2722
Closed

Facter tests mutate the global logger state #2721

joshcooper opened this issue May 21, 2024 · 1 comment · Fixed by #2722
Labels
bug Something isn't working triaged Jira issue has been created for this

Comments

@joshcooper
Copy link
Contributor

joshcooper commented May 21, 2024

Describe the Bug

While rearching #2664 and #2715 I noticed a few different classes of issues with Facter's logging system.

Tests often overwrite the global Logger with an instance_spy, but don't restore the original logger after each test:

before do
  Facter::Log.class_variable_set(:@@logger, logger_double)
end

This causes global state to leak across tests leading to rspec reporting:

 #<InstanceDouble(Facter::Log) (anonymous)> was originally created in one
    example but has leaked into another example and can no longer be used.

Tests also overwrite the global logger with an instance spy and restore it with a different logger instance instead of the original instance:

before do
  Facter::Log.class_variable_set(:@@logger, logger_double)
end

after do
  Facter::Log.class_variable_set(:@@logger, Logger.new(STDOUT))
end

A similar problem exists with resolvers as the each resolver class has a Facter::Log instance:

before do
  augeas.instance_variable_set(:@log, log_spy)
end

The problem is made worse, because some resolvers eagerly create their resolver:

class Uname < BaseResolver
  @log = Facter::Log.new(self)

While others rely on the BaseResolver to lazily create it:

class BaseResolver
  def self.log
    @log ||= Log.new(self)

As a result, you can never be sure if a resolver has a logger, and if so, who created it (the code or test).

Finally if aggregated facts are evaluated twice, they attempt to log a message to a nil logger.

Expected Behavior

Since Facter's Logger and Facter::Log are global state, they should be immutable.

@joshcooper joshcooper added the bug Something isn't working label May 21, 2024
@cthorn42 cthorn42 added accepted Valid issue that we intend to work on when we have the bandwidth triaged Jira issue has been created for this and removed accepted Valid issue that we intend to work on when we have the bandwidth labels May 21, 2024
Copy link

Migrated issue to FACT-3468

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged Jira issue has been created for this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants