modulesync 5.2.0 #128
modulesync 5.2.0 #128
Conversation
bastelfreak
force-pushed
the
modulesync
branch
2 times, most recently
from
5847ce4 to
1946415
Compare
| Rakefile: | ||
| changelog_version_tag_pattern: '%s' |
There was a problem hiding this comment.
This may be something we do want to consider keeping. Rather than prefixing with v everywhere.
There was a problem hiding this comment.
I've never really seen the point of the v prefix, which is why we went with bare tags in our projects.
There was a problem hiding this comment.
I agree with that. However, I prefer consistency more so I won't argue for changing it in a project. In some new projects I also started without a v prefix.
spec/defines/openssl_dhparam_spec.rb
Outdated
| expect do | ||
| is_expected.to contain_file('/etc/ssl/dhparam.pem') | ||
| }.to raise_error(%r{got ['barz'|String]}) | ||
| end.to raise_error(%r{got ['barz'|String]}) |
There was a problem hiding this comment.
Not sure why I can't make a suggestion, but this is the native way to test for this:
is_expected.to compile.and_raise_error(%r{got ['barz'|String]})This repeats all over this file
There was a problem hiding this comment.
The current syntax is probably a remnant from old rspec/rspec-puppet syntax
There was a problem hiding this comment.
Very likely. Technically it kind of works, but if we're making changes now we might as well go for the correct way :)
bastelfreak
force-pushed
the
modulesync
branch
3 times, most recently
from
6205a60 to
f4fa7eb
Compare
|
Ci passes now. I had to disable a bunch of rubocop cops and drop puppet 7. the tests currently don't pass on puppet 7. I would like to get this merged and afterwards have a look at puppet 7 again. the diff is already quite huge. |
Fixes tests due to #130.
| expect do | ||
| is_expected.to contain_file('/etc/ssl/certs/foo.cnf') | ||
| }.to raise_error(%r{got ['barz'|String]}) | ||
| end.to raise_error(%r{got ['barz|Sting]}) |
There was a problem hiding this comment.
This (and others in this file) should really be changed to:
is_expected.to compile.and_raise_error(%r{got ['barz|Sting]})
There was a problem hiding this comment.
should we remove the contain_file test or do you want me to keep them?
diff --git a/spec/defines/openssl_export_pem_cert_spec.rb b/spec/defines/openssl_export_pem_cert_spec.rb
index a296c7f..5bb4b30 100644
--- a/spec/defines/openssl_export_pem_cert_spec.rb
+++ b/spec/defines/openssl_export_pem_cert_spec.rb
@@ -17,11 +17,7 @@ describe 'openssl::export::pem_cert' do
}
end
- it 'fails' do
- expect do
- is_expected.to contain_exec('Export /etc/ssl/certs/foo.pfx to /etc/ssl/certs/foo.pem')
- end.to raise_error(Puppet::Error, %r{either})
- end
+ it { is_expected.to compile.and_raise_error(%r{either}) }
end
context 'when enable and pfx and der cert is provided' doThere was a problem hiding this comment.
I can take a look at changing those, but would prefer to do it after this PR got merged
modulesync 4.1.0