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

(FIXUP) Make system config path absolute instead of relative on POSIX hosts #862

Merged
merged 1 commit into from
Feb 28, 2020

Conversation

scotje
Copy link
Contributor

@scotje scotje commented Feb 27, 2020

Pretty much what it says on the tin.

@scotje scotje requested a review from a team as a code owner February 27, 2020 20:16
@@ -349,7 +349,7 @@
end

it 'returns a path inside the system opt directory' do
is_expected.to eq(File.join('opt', 'puppetlabs', 'pdk', 'config'))
is_expected.to match(%r{\A/opt/})
Copy link
Contributor Author

@scotje scotje Feb 27, 2020

Choose a reason for hiding this comment

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

We can make this more specific if we want, but this test now better matches its description.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.27% when pulling 9847532 on scotje:fix_system_config_root into 0489ba5 on puppetlabs:master.

@rodjek
Copy link
Contributor

rodjek commented Feb 27, 2020

We should look at getting some acceptance tests going for this as well

@rodjek rodjek mentioned this pull request Feb 28, 2020
4 tasks
@glennsarti
Copy link
Contributor

glennsarti commented Feb 28, 2020

@rodjek Did you want the acceptance tests in this PR?

Also acceptance tests for this particular method seem difficult to write. It's only a 4 line method. I agree that there should be acceptance (or packaging) tests for the things that consume this.

IMO, In this case adding more tests wouldn't help confidence, because the original tests were wrong, not that they were incomplete.

@glennsarti
Copy link
Contributor

glennsarti commented Feb 28, 2020

I've added some tests in #859 (Commit a3b68db) Which should cover this.

@scotje scotje merged commit 843c7af into puppetlabs:master Feb 28, 2020
@scotje scotje deleted the fix_system_config_root branch February 28, 2020 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants