In tests verify config file contents instead of templatesymlink matcher tests.
In https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/3001, I changed tests checking for rendered file contents to be proper LWRP tests (for `templatesymlink` resource) using matchers. This was to ensure we don't step into resources unnecessarily in tests and followed proper LWRP testing procedure. However, now I think that was a wrong move to make, especially because of the somewhat unique nature of `templatesymlink` resource.
The main problem being it is very easy for someone to add variables to gitlab.rb and set default attributes but don't realize they have to explicitly use it in the config template - as seen in https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/3681#note_231173814, especially if they are not extremely familiar with omnibus-gitlab codebase. If our other tests suggested verifying the file contents and they referred those, I think this could've been caught early by the developers themselves.
I think we should make an exception to our "do not step into resources unnecessarily" guideline for `templatesymlink`, and go back to checking the actual rendered file for its contents. This is because input to `templatesymlink` and results of its actions can vary wildly compared to other resources.
@ibaum @twk3 I would like to hear your thoughts on this.
On that note, I also like what is being done in https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/3692 - instead of regexing the rendered file, let's load the rendered file as a Ruby implementation of yaml/toml, and then do equality checking. I am not sure of its performance implications yet, though.
issue