(GH-cat-9) Update module to match current syntax standard #2235
Conversation
david22swan
force-pushed
the
GH-cat-9/syntax_standards
branch
from
65338bc
to
d43fd94
Compare
david22swan
force-pushed
the
GH-cat-9/syntax_standards
branch
3 times, most recently
from
14da50b
to
cab1b0d
Compare
spec/classes/mod/jk_spec.rb
Outdated
| @@ -67,7 +67,7 @@ | |||
| include_examples 'Debian 11' | |||
|
|
|||
| context 'with only required facts and default parameters' do | |||
| let(:facts) { super().merge('ipaddress' => default_ip) } | |||
| let(:facts) { super().merge(networking: { ip: default_ip }) } | |||
There was a problem hiding this comment.
This breaks things and you need to do a deep merge. In VP we have a helper for this: override_facts(). https://github.com/voxpupuli/voxpupuli-test#fact-handling describes the reasoning. Ideally this would be easier (maybe something in rspec-puppet?), but your current approach remotes all other networking facts (like FQDN).
There was a problem hiding this comment.
See reply to below comment
spec/classes/mod/proxy_html_spec.rb
Outdated
| @@ -20,7 +20,7 @@ | |||
|
|
|||
| context 'on i386' do | |||
| let(:facts) do | |||
| super().merge(hardwaremodel: 'i686', | |||
| super().merge(os: { family: 'Debian', name: 'Debian', release: { full: '11', major: '11' }, hardware: 'i686', }, | |||
There was a problem hiding this comment.
Why is it needed to override the Debian facts here? It should inherit that from super().
There was a problem hiding this comment.
Since hardware is contained within os I can't merge it without it overwriting everything.
Since I don't like the look of this I'm considering implementing the test handling feature you linked above, though will have to talk with the others regarding bringing in a new gem first.
Currently, leaving it to the end to do as I want to finish with the rest of the rules first, and see whether or not any other issues come up.
There was a problem hiding this comment.
I think it would be good to fix voxpupuli/rspec-puppet-facts#112 and solve it at that level. It's already pulled in and it's probably the right place to fix it (or rspec-puppet itself). IMHO that would be best for the ecosystem. Fixing it in voxpupuli-test was just my way of making quick progress.
I can't promise to have time for it, but if someone contributes the PR I'd be happy.
There was a problem hiding this comment.
Was unable to include the voxpupuli-test gem added due to conflicts with existing gems so have simply added the override_facts code to the spec_helper_local.rb for now if you don't mind me copying your code.
david22swan
force-pushed
the
GH-cat-9/syntax_standards
branch
21 times, most recently
from
70abe82
to
4b2373a
Compare
david22swan
force-pushed
the
GH-cat-9/syntax_standards
branch
6 times, most recently
from
38fc4eb
to
ad8b7fe
Compare
david22swan
force-pushed
the
GH-cat-9/syntax_standards
branch
from
f7b67cc
to
92aa441
Compare
Code now compliant with the following rules: - manifest_whitespace_closing_bracket_after - relative_classname_inclusion - relative_classname_reference - version_conparison - legacy_facts - top_scope_facts
Code now compliant with the following rules: - parameter_documentation - parameter_types
david22swan
force-pushed
the
GH-cat-9/syntax_standards
branch
from
d1d0fe2
to
9394073
Compare
david22swan
force-pushed
the
GH-cat-9/syntax_standards
branch
from
9394073
to
421c483
Compare
david22swan
force-pushed
the
GH-cat-9/syntax_standards
branch
from
421c483
to
a1c95aa
Compare
Code now compliant with the following rules: