Refactor JobTemplates and fix regex substitution

[perlpunk]

Refactor

Split into subroutines, move code to
OpenQA::Schema:Result::JobGroups
OpenQA::Schema::ResultSet::JobTemplates

This will make testing easier.

Fix substitution for job_templates schedule list

Some templates don't have a final newline, which resulted in the last line not being indented correctly

This is a followup to #2589

Related ticket: https://progress.opensuse.org/issues/60782

[okurz]

+1

[perlpunk]

I think I need to make another change. Duplicate jobtemplates aren't checked in preview mode now, but there wasn't a test for this yet.

[perlpunk]

I fixed the preview mode.

[perlpunk]

rebased to master

[Martchus]

Looks good. Only a few style issues (in code you've just moved).

[kalikiana]

I'm liking the refactor! Not much to criticize about the implementation, although I have a few comments regarding the tests. There should be a brown review state of almost-approved 😉

[kalikiana]

It strikes me as odd that this is sub takes $self but doesn't use it. The result is not the expansion of what's in the group, but the expansion of the given hash. 🤔

[perlpunk]

I actually thought about putting some functions into their own module because they do not depend on database objects, but skipped it for now.

[okurz]

There should be a brown review state of almost-approved

What I tend to use in these cases is a comment like: "Just this (…) minor issue, then auto-approved" or something :) We should use that more often to not need the additional overhead of "official approval" when it is clear what only remaining trivial issues exist before the approval would be implicit anyway.

[kalikiana]

Thanks! I'm okay with it as is. The tests can also be extended in another branch!

[okurz]

So far I found one test that failed:

https://app.circleci.com/jobs/github/perlpunk/openQA/79/parallel-runs/0/steps/0-108

t/25-cache-service.t ........................... 16/? 
    #   Failed test 'job enqueued'
    #   at t/25-cache-service.t line 462.
    # Looks like you failed 1 test of 3.

#   Failed test 'Test Minion Sync task'
#   at t/25-cache-service.t line 471.
Can't call method "perform" on an undefined value at t/25-cache-service.t line 463.
rsync: change_dir "/tmp/4c7ipQOUBc" failed: No such file or directory (2)
rsync: mkdir "/tmp/Z6IJ3ghuij/tests" failed: No such file or directory (2)
rsync error: error in file IO (code 11) at main.c(664) [Receiver=3.1.3]
t/25-cache-service.t ........................... 18/? # Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 255 just after 18.
                                                       t/25-cache-service.t ........................... Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 1/18 subtests 

[perlpunk]

So far I found one test that failed:

Hm. the test didn't fail before my style fixes, and also doesn't fail in #2602 which is based on this branch...

[Martchus]

You can try to retrigger that test.