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

Refactor fact #30

Closed
wants to merge 2 commits into from
Closed

Conversation

Phil-Friderici
Copy link
Contributor

No description provided.

@ghoneycutt
Copy link
Contributor

Are you seeing the unit test timeout with ruby 2.3.1? I'm seeing that with python.

voxpupuli/puppet-python#336

You can allow ruby 2.3.1 to fail with this

https://github.com/stankevich/puppet-python/pull/337/files

@Phil-Friderici
Copy link
Contributor Author

The downsize would be that Travis is waiting around 10 minutes before failing. This will slow down the testing results quite a lot :(

@Phil-Friderici
Copy link
Contributor Author

Think I will add only puppet latests to run with ruby 2.3.1. This would only cost 10 minutes.

@Phil-Friderici
Copy link
Contributor Author

Breaking news
The new version of the fact will only search for sudo within the given systems path.

Maybe time is good for a 1.0.0 release ?


with_fixtures = {
'Debian' => {
:sudo_output => 'Sudo version 1.8.3p1\nSudoers policy plugin version 1.8.3p1\nSudoers file grammar version 40\nSudoers I/O plugin version 1.8.3p1\n',
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the fixture stay the same as the output has not changed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it for better alignment. Btw: I have seen likewise output on RedHatish systems as well.
The regex /^Sudo version +(\S+)$/ should be specific enough to not need to test that it ignores the rest of the string.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, but we wouldn't know without the whole output. What if we want to tweak our regex? When it comes to fixtures, I think we should take the entire output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you'll like my latest change :)

@ghoneycutt
Copy link
Contributor

+1 for a v1.0.0 release

@Phil-Friderici
Copy link
Contributor Author

The fact does proactivly search for /opt/quest/bin/sudo again. This ensures fully backward compatibility again :)

@ghoneycutt
Copy link
Contributor

+1

@Phil-Friderici
Copy link
Contributor Author

moving on to #32 to prepare 1.0.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants