Better queue config warning.

[hjoliver]

This is a small change with no associated Issue. Result of a user complaint today.

The following queue validation warning applies if the task bar (assigned to a queue) is not defined under [runtime] or if it is defined there but is simply not used in the graph:

WARNING - Queue configuration warnings:
        + q1: ignoring bar (task not defined)

On this branch, the warning in the latter case changes to:

WARNING - Queue configuration warnings:
        + q1: ignoring bar (task not used in the graph)

Requirements 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).

• Appropriate tests are included (unit and/or functional).

• Appropriate change log entry included.

• No documentation update required.

[hjoliver]

This trivial change really demands a simple unit test, except that it requires the whole monolithic suite config class so at the moment a functional test would be much easier to write 😬 Might be best to wait on @oliver-sanders latest experiment? #3616 (which I have seen but not read yet...).

[hjoliver]

That's weird, style fail in cylc/flow/cfgspec/globalcfg.py not touched by this branch 🤔

[kinow]

@hjoliver I merged a PR that had pycodestyle issues today, sorry. I thought it failed because of pending merge of other PR's in cylc-doc/cylc-sphinx-extensions.

https://github.com/cylc/cylc-flow/pull/3559/checks?check_run_id=685749339

[oliver-sanders]

This is definitely a unit-test problem rather than an integration-test.

Unit-testing this isn't especially easy due to complex objects getting in the way, it would be a lot easier if the code was functional, however, it is still doable:

Create a Mock object, then have a quick scan down the code searching for self. to see what you have to bolt onto it to get to the required branch in the code. You usually only need a few things:

# untested

def test_queue_thinggy(caplog):
    config = Mock()
    config.Q_DEFAULT = 'default'
    config.cfg = {
        'scheduling': {
            'queues': {
                'default': {}
            }
        },
        'runtime': {}
    }
    config.runtime = {
        'descendents': []
    }

    caplog.set_level(logging.INFO, logger='cylc')
    Config.configure_queues(config)
    assert caplog.reccord_tuples = [
        # expected log messages
    ]

Some examples of this mocking based approach to testing methods here: https://github.com/cylc/cylc-flow/tree/master/cylc/flow/tests/main_loop

[hjoliver]

Ha, there's already a test_queue_config function in tests/test_config.py (using caplog too) ... and what's more, git blame says I wrote it. I think I need my brain looked at 🤣 It's not quite as clean as your suggestion as it uses a predefined SuiteConfig object (with a bunch of mocked bits inside) but it'll do.

[hjoliver]

Rebased to pick up the off-topic style fix; unit test added.

[oliver-sanders]

🍌