Conversation
With this patch we're replacing puppetlabs_spec_helper with voxpupuli-test. That one brings in a bunch of nifty gems along with it that makes running tests easier. We need to remove references to puppetlabs_spec_helper since it causes some rake tasks to be defined twice and generally not work. The `--pidfile-workaround` argument to `metadata2gha` was removed. Another effect of this patch is that we're now tracking voxpupuli-test's configuration for rubocop instead of maintaining one of our own. The change will expose many issues that we'll need to fix. Plus bump all github actions to be up to date.
This was done using `bundle exec rake rubocop:autocorrect`, and then also `bundle exec rubocop -A` to also fix the more complex cases. The most notable change here is that now trocla_helper() and trocla_get() are using YAML.safe_load() instead of YAML.load()
This only covers the problems that puppet-lint could autofix. There are more issues remaining that we need to fix manually.
Those were the last remaining puppet-lint issues that we needed to fix. Changing to the puppet-strings format will make it possible to automate rendering of a REFERENCE.md file using `bundle exec rake strings:generate:reference` Some parameters were not defined so I've added documentation to the best of my understanding of what they are used for.
Perforce has completely destroyed upstream documentation links. Thanks for not knowing how to handle redirections... or just not caring about the community. I couldn't find a section that specifically showed how to configure trocla, so I took something that was possibly the closest. But it's not a perfect fit. We do have information about this in the README file though, so we can point folks to that, too.
The choice is currently supported debian, ubuntu and redhat releases. This addition to the metadata.json file make the `on_supported_os` function automatically run tests for these distros and releases. We also get a set of facts for each distro/release.
The main change in this patch is that we're not wrapping test contexts with `on_supported_os`. That multiplies the amount of tests being run but it ensures that no special case for a distro + release will cause unexpected issues. Some tests were failing to run so we need to fix them Finally, the tests show me that the assumption I had made for the type of the `trocla::config::encryption` parameter was wrong. I'm now including String as well so that we don't break user calls.
This is useful for quick access to technical documentation. CI will ensure that we keep this file up to date. We can use a rake task to generate an updated version of the file: `bundle exec rake strings:generate:reference` There are some warnings during generation about some missing @return tags on ruby functions, but when I try to add those I get told that it should rather be documented in the call to `dispatch`.. but I have no idea how to do this.
|
wow, thank you a lot. I definitely want this to get merged. I think I can figure out the issue with gsub. Let me try and have a look at it before merging it. |
|
@duritong I just came up with something for gsub: I created a mock function to abstract away |
|
I pushed my work on mocking out Since CI is now passing I think we can go ahead with the merge (of course only if the changes are to your liking.. sorry it's a pretty hefty PR). I'll defer transforming functions to puppet 4.x syntax to another PR (coupled with writing unit tests for them beforehand to make sure that the transformation doesn't break anythng). It'll keep things more focuses to have that work in a PR of its own |
The tests are currently calling out to trocla() which expects config files and actual storage file to be in place. In the current unit test, we are verifying what gsub() is doing with values returned by trocla() so we should actually decouple the test from the actual behavior of trocla() by mocking out trocla() With the `trocla()` function decoupled, we don't need to set the :config setting anymore.
|
the last force-push was the last movement coming from me. moving away from this now and letting the space to you. I realized that with the mocking out of the |
|
excellent, many thanks! yeah I also saw the old puppet 3 functions. would be great to port them! |
|
btw: I also rebased a few local changes onto your changes and pushed them to master^main |
2 participants
This patch series makes unit tests run correctly in github CI.
...mostly. there is still only one detail that I don't know how to address: the tests for the
gsubfunction are failing when trying to open atroclarc.yamlfile from the fixtures directory but that file does not exist. From what I can read from the body of the function, the only thing that I think could trigger this would be the call tocall_function('trocla', trocla_key, 'plain')@duritong do you have an idea what's happening with the gsub test?
otherwise the style of tests is changed a little bit: I've decided to make the tests use rspec-puppet-facts's
on_supported_osto make the tests run against a fact set for each supported OS/release. I've had to declare supported distros and releases out of the blue for this to work though.some new code style is now imposed on ruby code via the set of rules that voxpupuli defines in the
voxpupuli-testgem.another detail that this series changes is that the module now has a REFERENCE.md file and CI will fail if that file is not updated accordingly. It can however be easily regenerated with
bundle exec rake strings:generate:referencelast but not least: I've used upper bounds on the gems in
Gemfilein order to keep CI runs more stable. I would recommend to setup some automatic dependency update mechanism like renovatebot, but I haven't enabled this since I don't know if it's something you actually desire to use.