systemd::timer: fix before's argument to use the proper syntax

[simondeziel]

I was reading the code and vim wouldn't highlight that line properly...

[ekohl]

Oh, good catch. I wonder if Unit_File is every really valid. If not, perhaps it's a lint rule or even syntax rule that could catch this.

What I'm also surprised about is that the unit tests didn't catch this. We have it covered here:

puppet-systemd/spec/defines/timer_spec.rb

Lines 30 to 32 in a8712e4

	expect(subject).to contain_systemd__unit_file('foobar.service').with(
	content: "[Service]\nExecStart=/bin/touch /tmp/foobar"
	).that_comes_before('Systemd::Unit_file[foobar.timer]')

[simondeziel]

What I'm also surprised about is that the unit tests didn't catch this.

I'm not up to date on how Puppet (and rspec) evaluates manifests theses days. I vaguely remember that at some point it stopped doing a random order to switch to the more natural top to bottom order. But maybe I am misremembering.

[jhoblitt]

Yikes. We obviously have an acceptance test coverage problem.

[jhoblitt]

The fixed will be included in the #317 release.

[ekohl]

I recall that Puppet was case insensitive about some of these matters (pretty sure that was true for type aliases), but then not all tooling understands that. So it may be valid, but not the preferred style. Personally I think there should just be a single valid style and enforce that

[alexjfisher]

I'm not up to date on how Puppet (and rspec) evaluates manifests theses days. I vaguely remember that at some point it stopped doing a random order to switch to the more natural top to bottom order. But maybe I am misremembering.

The ordering parameter was removed in Puppet 6.0 and is now always top to bottom unless there's explicit ordering. That said, I'm fairly certain the implementation doesn't create actual relationships in the resource graph, so rspec puppet wouldn't see these. I'm guessing it's just a case insensitivity thing in Puppet.