Skip to content
This repository has been archived by the owner on Jun 19, 2020. It is now read-only.

(maint) Fix trace acceptance test #405

Merged
merged 4 commits into from
Mar 25, 2020
Merged

(maint) Fix trace acceptance test #405

merged 4 commits into from
Mar 25, 2020

Conversation

oanatmaria
Copy link
Contributor

No description provided.

BogdanIrimie
BogdanIrimie previously approved these changes Mar 24, 2020
.rubocop.yml Outdated Show resolved Hide resolved
lib/facter.rb Outdated
@@ -202,6 +203,7 @@ def to_hash
# @api public
def trace?
LegacyFacter.trace?
Copy link
Contributor

Choose a reason for hiding this comment

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

is this call needed anymore?

@@ -212,6 +214,7 @@ def trace?
# @api public
def trace(bool)
LegacyFacter.trace(bool)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this call needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LegacyFacter.log_exception is still used in other classes like collection.rb, so setting trace in LegacyFacter is still needed.

describe '#log_exception' do
context 'when trace options is true' do
before do
Facter.instance_variable_set(:@trace, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

why not set it using Facter.trace method?

Copy link
Contributor

Choose a reason for hiding this comment

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

also i think it should be set to false, after running the test

@BogdanIrimie BogdanIrimie merged commit 641fb48 into master Mar 25, 2020
@oanatmaria oanatmaria deleted the fix_acceptance branch April 2, 2020 11:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants