tests/functional: simplify rose-stem tests #268

oliver-sanders · 2023-11-27T12:30:49Z

Change class scoped fixtures to module scoped. This avoids test classes and ensures fixtures are only initiated once.
Merge fixtures which were only used once in with the test which used them.
Remove parameterisations and fail testing code which weren't used by the tests.
Remove monkeypatch.delenv where it wasn't needed (monkeypatch resets things by default).

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 cylc/cylc-doc/pull/XXXX.
If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

* Change class scoped fixtures to module scoped. This avoids test classes and ensures fixtures are only initiated once. * Merge fixtures which were only used once in with the test which used them. * Remove parameterisations and fail testing code which weren't used by the tests. * Remove monkeypatch.delenv where it wasn't needed (monkeypatch resets things by default).

oliver-sanders · 2023-11-27T12:32:04Z

tests/functional/test_rose_stem.py

@@ -251,234 +252,83 @@ def _inner_fn(rose_stem_opts, verbosity=verbosity):
    yield _inner_fn


-@pytest.fixture(scope='class')


Class-scoped fixture wasn't needed as it was only used once (it's a bit too specific to the test to be reusable).

Much of the class scoped stuff was done in the name of doing as little re-writing as possible when moving the tests from Stuarts work.

oliver-sanders · 2023-11-27T12:32:25Z

tests/functional/test_rose_stem.py

    rose_stem_opts = {
        'stem_groups': [],
        'stem_sources': [
            str(setup_stem_repo['workingcopy']), "fcm:foo.x_tr@head"
        ],
    }
-    yield rose_stem_run_template(rose_stem_opts)
+    plugin_result = rose_stem_run_template(rose_stem_opts)


I'm not sure plugin_result is the right name for this, any suggestions?

stem_outputs

stem_variables

oliver-sanders · 2023-11-27T12:33:35Z

tests/functional/test_rose_stem.py

-        'expected',
-        [
-            "run_ok",
-            "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']",
-            "SOURCE_FOO=\"{workingcopy} fcm:foo.x_tr@head\"",
-            "HOST_SOURCE_FOO=\"{hostname}:{workingcopy} fcm:foo.x_tr@head\"",
-            "SOURCE_FOO_BASE=\"{workingcopy}\"\n",
-            "SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"\n",
-            "SOURCE_FOO_REV=\"\"\n",
-            "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"\n",
-        ]


This parameterisation only defines on variable, expected, which is always run_ok.

I don't think the other stuff did anything?

By removing this, have I killed what this was supposed to be testing?

I don't think that's right. Here is how I would rewrite the test

@@ -265,5 +265,5 @@ def test_rose_stem_run_really_basic(rose_stem_run_template, setup_stem_repo): -def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): +def test_rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): """Check that assorted variables have been exported.""" rose_stem_opts = { @@ -277,4 +277,18 @@ def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): assert plugin_result['run_stem'].returncode == 0 + for line in [ + "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", + 'SOURCE_FOO="{workingcopy} fcm:foo.x_tr@head"', + 'HOST_SOURCE_FOO="{hostname}:{workingcopy} fcm:foo.x_tr@head"', + 'SOURCE_FOO_BASE="{hostname}:{workingcopy}"', + 'SOURCE_FOO_REV=""', + 'SOURCE_FOO_MIRROR="fcm:foo.xm/trunk@1"', + ]: + line = line.format( + **plugin_result, + hostname=HOST + ) + assert line in plugin_result['jobout_content']

Yes, I had misread that parametrisation, drafted.

oliver-sanders · 2023-11-27T12:33:54Z

tests/functional/test_rose_stem.py

-        else:
-            expected = expected.format(
-                workingcopy=rose_stem_run_basic['workingcopy'],
-                hostname=HOST
-            )
-            assert expected in rose_stem_run_basic['jobout_content']


expected is always run_ok so this never got run?

oliver-sanders · 2023-11-27T12:35:34Z

tests/functional/test_rose_stem.py

-            assert expected in subdirectory['jobout_content']
-
-
-@pytest.fixture(scope='class')


If you only want a fixture to be initiated once, use the module-scope. The class-scope allows multiple test classes which would initialise this fixture once each.

oliver-sanders · 2023-11-27T12:36:09Z

tests/functional/test_rose_stem.py


 from cylc.rose.stem import (
    RoseStemVersionException,
    get_rose_stem_opts,
    rose_stem,
 )

+
 HOST = get_host().split('.')[0]


This variable doesn't appear to be used any more?

Only at lines 306, 355, 392, 431, 471 & 524.

oliver-sanders · 2023-11-27T12:36:46Z

tests/functional/test_rose_stem.py

        'ROSE_CONF_PATH', str(setup_stem_repo['basetemp'])
    )
-    yield rose_stem_run_template(rose_stem_opts)
-    monkeymodule.delenv('ROSE_CONF_PATH')


Monkeypatch/module should do this by itself?

At the end of the module. I think we want to remove this now, this should be monekypatch (as you have done!).

oliver-sanders · 2023-11-27T12:45:02Z

Test failures are now all reinstall tests.

MetRonnie · 2023-11-27T16:04:07Z

tests/functional/test_rose_stem.py

-        'expected',
-        [
-            "run_ok",
-            "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']",
-            "SOURCE_FOO=\"{workingcopy} fcm:foo.x_tr@head\"",
-            "HOST_SOURCE_FOO=\"{hostname}:{workingcopy} fcm:foo.x_tr@head\"",
-            "SOURCE_FOO_BASE=\"{workingcopy}\"\n",
-            "SOURCE_FOO_BASE=\"{hostname}:{workingcopy}\"\n",
-            "SOURCE_FOO_REV=\"\"\n",
-            "SOURCE_FOO_MIRROR=\"fcm:foo.xm/trunk@1\"\n",
-        ]


I don't think that's right. Here is how I would rewrite the test

@@ -265,5 +265,5 @@ def test_rose_stem_run_really_basic(rose_stem_run_template, setup_stem_repo): -def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): +def test_rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): """Check that assorted variables have been exported.""" rose_stem_opts = { @@ -277,4 +277,18 @@ def rose_stem_run_basic(rose_stem_run_template, setup_stem_repo): assert plugin_result['run_stem'].returncode == 0 + for line in [ + "RUN_NAMES=['earl_grey', 'milk', 'sugar', 'spoon', 'cup', 'milk']", + 'SOURCE_FOO="{workingcopy} fcm:foo.x_tr@head"', + 'HOST_SOURCE_FOO="{hostname}:{workingcopy} fcm:foo.x_tr@head"', + 'SOURCE_FOO_BASE="{hostname}:{workingcopy}"', + 'SOURCE_FOO_REV=""', + 'SOURCE_FOO_MIRROR="fcm:foo.xm/trunk@1"', + ]: + line = line.format( + **plugin_result, + hostname=HOST + ) + assert line in plugin_result['jobout_content']

MetRonnie · 2023-11-27T16:04:14Z

tests/functional/test_rose_stem.py

 import pytest
+from _pytest.monkeypatch import MonkeyPatch


Suggested change

from _pytest.monkeypatch import MonkeyPatch

from pytest import MonkeyPatch

oliver-sanders · 2024-04-23T17:10:16Z

Superseded by #314

oliver-sanders added this to the 1.4.0 milestone Nov 27, 2023

oliver-sanders commented Nov 27, 2023

View reviewed changes

oliver-sanders mentioned this pull request Nov 27, 2023

Exit immediately if no rose suite.conf #267

Merged

8 tasks

oliver-sanders requested review from wxtim and MetRonnie November 27, 2023 12:44

MetRonnie reviewed Nov 27, 2023

View reviewed changes

oliver-sanders marked this pull request as draft November 27, 2023 16:20

oliver-sanders mentioned this pull request Apr 23, 2024

simplify rose stem tests #314

Merged

8 tasks

oliver-sanders closed this Apr 23, 2024

oliver-sanders removed this from the 1.4.0 milestone Apr 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests/functional: simplify rose-stem tests #268

tests/functional: simplify rose-stem tests #268

oliver-sanders commented Nov 27, 2023

oliver-sanders Nov 27, 2023

wxtim Nov 28, 2023

oliver-sanders Nov 27, 2023

wxtim Nov 28, 2023

oliver-sanders Nov 27, 2023

MetRonnie Nov 27, 2023

oliver-sanders Nov 27, 2023

oliver-sanders Nov 27, 2023

oliver-sanders Nov 27, 2023

oliver-sanders Nov 27, 2023

wxtim Nov 28, 2023

oliver-sanders Nov 27, 2023

wxtim Nov 28, 2023 •

edited

Loading

oliver-sanders commented Nov 27, 2023

MetRonnie Nov 27, 2023

MetRonnie Nov 27, 2023

oliver-sanders commented Apr 23, 2024

		@@ -251,234 +252,83 @@ def _inner_fn(rose_stem_opts, verbosity=verbosity):
		yield _inner_fn


		@pytest.fixture(scope='class')

		assert expected in subdirectory['jobout_content']


		@pytest.fixture(scope='class')

	from _pytest.monkeypatch import MonkeyPatch
	from pytest import MonkeyPatch

tests/functional: simplify rose-stem tests #268

tests/functional: simplify rose-stem tests #268

Conversation

oliver-sanders commented Nov 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxtim Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

oliver-sanders commented Nov 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliver-sanders commented Apr 23, 2024

wxtim Nov 28, 2023 •

edited

Loading