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

(SDK-285) Add --auto-correct flag to validators that support it #104

Merged
merged 3 commits into from
Jun 28, 2017

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented Jun 23, 2017

This will just silently no-op for validators that don't support auto correction. There's no acceptance tests for puppet-lint integration at the moment, so I've only added an acceptance test for auto-correcting rubocop violations.

@@ -1,7 +1,8 @@

shared_context 'in a new module' do |name|
before(:all) do
system("pdk new module #{name} --skip-interview") || raise
output = `pdk new module #{name} --skip-interview`

Choose a reason for hiding this comment

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

I'm not sure this captures stderr, appending 2>&1 should work on all platforms

Copy link

@james-stocks james-stocks left a comment

Choose a reason for hiding this comment

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

LGTM - added a small adjustment to acceptance so that we get stderr if module folder creation fails

Copy link
Contributor

@DavidS DavidS left a comment

Choose a reason for hiding this comment

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

Even if there are no puppet-lint acceptance tests yet, please do start them when you're adding to it. The lack of testing over there, won't get better by ignoring it!

@@ -1,7 +1,8 @@

shared_context 'in a new module' do |name|
before(:all) do
system("pdk new module #{name} --skip-interview") || raise
output = `pdk new module #{name} --skip-interview 2>&1`
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -1,7 +1,8 @@

shared_context 'in a new module' do |name|
before(:all) do
system("pdk new module #{name} --skip-interview") || raise
output = `pdk new module #{name} --skip-interview 2>&1`
raise "Failed to create test module:\n#{output}" unless File.directory?(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Then you can also raise here on the exit status returned from capture2e, and not rely on environmental factors.

@DavidS DavidS added the feature label Jun 26, 2017
@DavidS
Copy link
Contributor

DavidS commented Jun 26, 2017

Please also add a note to the README about how and where to use this flag.

@rodjek
Copy link
Contributor Author

rodjek commented Jun 27, 2017

Even if there are no puppet-lint acceptance tests yet, please do start them when you're adding to it. The lack of testing over there, won't get better by ignoring it!

Sorry, didn't mean to imply that I was ignoring the lack of tests. I knew that puppet-lint integration was still a work in progress with open tickets and didn't want to duplicate work.

@rodjek
Copy link
Contributor Author

rodjek commented Jun 27, 2017

Updated the in a new module context to use Open3.capture2e. Will open a new PR to add the basic puppet-lint acceptance tests and then loop back to this PR to add acceptance tests for puppet-lint auto-correct here so that we keep this PR on topic.

@rodjek
Copy link
Contributor Author

rodjek commented Jun 27, 2017

Acceptance tests for puppet-lint integration added in #109. Once they are merged in, I'll come back to this and add acceptance tests for puppet-lint with --auto-correct.

@rodjek rodjek force-pushed the sdk-285 branch 2 times, most recently from 4fd8f8f to b3d5ad5 Compare June 28, 2017 06:41
@rodjek
Copy link
Contributor Author

rodjek commented Jun 28, 2017

@DavidS acceptance tests added for auto-correcting puppet-lint problems

@rodjek rodjek merged commit 55dff9f into puppetlabs:master Jun 28, 2017
@rodjek rodjek deleted the sdk-285 branch June 28, 2017 12:12
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.

3 participants