Add CYLC_ variables to template engine globals.

[hjoliver]

What it says. Make this sort of thing work during flow.cylc parsing:

{{ CYLC_WORKFLOW_RUN_DIR | default("not defined") }}

See explanation in #5570 (comment)

If agreed, I'll document this, and do the same for Empy.

Will need to document that this should not be used in place of the associated environment variables in task definitions (they get evaluated at job run time and will therefore always be defined).

[UPDATE: for the linked use case, #5589 makes this PR redundant. But this might be useful for other reasons]

Check List

• I have read CONTRIBUTING.md and added my name as a Code Contributor.
• Contains logically grouped changes (else tidy your branch by rebase).
• Does not contain off-topic changes (use other PRs for other changes).
• Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
• Tests are included (or explain why tests are not needed).
• CHANGES.md entry included if this is a change that can affect users
• Cylc-Doc pull request opened if required at Update Jinja2 and EmPy templating sections. cylc-doc#631.
• If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

[oliver-sanders]

(remote tests unhappy)

[oliver-sanders]

Hmm, found this in the debug section for the failed test:

Can't find:
===========
CYLC_JOB_RUNNER_NAME=background
===========
in:
===========
CYLC_JOB_RUNNER_NAME=at
...

???

[wxtim]

I've not put anything demanding radical changes in my comments, mostly suggestions for shrinking the test, not of which I'm emotionally attached to (apart from match=, I like that).

I've had a play with a checked out copy.

Will approve once conflicts are resolved.

[wxtim]

@oliver-sanders

I've done a deconflict merge.

Could you do a final check that my merge hasn't broken anything?

[oliver-sanders]

Could you do a final check that my merge hasn't broken anything?

This PR will still have tests failing, see above.

[MetRonnie]

The test that is failing had this in it:

cylc-flow/tests/functional/cylc-cat-log/01-remote/flow.cylc

Line 19 in 6f8a201

platform = {{CYLC_TEST_PLATFORM | default("localhost")}}

Before this PR I think CYLC_TEST_PLATFORM was always an undefined Jinja2 variable so the workflow was not actually running remotely and the test was not doing what it was supposed to be doing (it was testing local cat-log not remote cat-log)

[oliver-sanders]

That doesn't make sense to me, there are other tests which follow this pattern:

cylc-flow/tests/functional/remote/00-basic.t

Line 19 in 6f8a201

export REQUIRE_PLATFORM='loc:remote'

But don't provide a default:

cylc-flow/tests/functional/remote/basic/flow.cylc

Line 9 in 6f8a201

platform = {{ environ['CYLC_TEST_PLATFORM'] }}

So would get a Jinja2Undefined error if so.

[oliver-sanders]

Oh, sorry, didn't read that properly, it's a Jinja2 variable 🤦

[oliver-sanders]

Here's a test that runs in CI and uses the Jinja2 variable:

cylc-flow/tests/functional/cylc-kill/01-multi-hosts/flow.cylc

Line 23 in 6f8a201

platform = {{ CYLC_TEST_PLATFORM }}

[MetRonnie]

In that one it uses cylc play -s "CYLC_TEST_PLATFORM=$CYLC_TEST_PLATFORM" but not in the failing one

[hjoliver]

Addressed review comments.
Extended to empy.
Rebased.

[hjoliver]

In that one it uses cylc play -s "CYLC_TEST_PLATFORM=$CYLC_TEST_PLATFORM" but not in the failing one

And fixed the failing test (it now runs remote jobs, as advertised).

[hjoliver]

Should be good to go now. Cylc-doc PR up too.

[hjoliver]

Right, all done.

I think tests/functional/broadcast/00-simple.t is going to fail, but is also failing on 8.2.x ... I've run out of time to investigate for now.

[MetRonnie]

Regression in tests/f/cylc-cat-log/01-remote.t failing again

Btw as this is an enhancement should it be on the 8.3.0 milestone (and base changed to master)?

[hjoliver]

Regression in tests/f/cylc-cat-log/01-remote.t failing again

Damn it, problem was I stupidly force-pushed from my outdated branch on the laptop at home today after yesterday's fixes at work. I had to redo some changes, but forgot this one.

And yes, it should be rebased to master.

[MetRonnie]

I've cherry picked the missing commit. And changed the base + milestone; no need to rebase however (and no need to fiddle with the changelog anymore 😁)

[hjoliver]

Great, thanks! (change base on the PR is what I meant)