From 9b95b7862c4419fb9e3b69621a8d447a5d09ced0 Mon Sep 17 00:00:00 2001 From: Michael Hanke Date: Tue, 10 Oct 2023 09:47:02 +0200 Subject: [PATCH] Reduce some of the noise in the test battery This is done by turning of the result rendering. This revealed that the command implementations cause uncondtional result rendering https://github.com/datalad/datalad-container/issues/241 --- datalad_container/tests/test_containers.py | 89 ++++++++++------- datalad_container/tests/test_run.py | 110 +++++++++++---------- 2 files changed, 110 insertions(+), 89 deletions(-) diff --git a/datalad_container/tests/test_containers.py b/datalad_container/tests/test_containers.py index 16cb58dc..b08601ce 100644 --- a/datalad_container/tests/test_containers.py +++ b/datalad_container/tests/test_containers.py @@ -29,27 +29,32 @@ from datalad_container.tests.utils import add_pyscript_image +common_kwargs = {'result_renderer': 'disabled'} + @with_tempfile def test_add_noop(path=None): - ds = Dataset(path).create() + ds = Dataset(path).create(**common_kwargs) ok_clean_git(ds.path) assert_raises(TypeError, ds.containers_add) # fails when there is no image - assert_status('error', ds.containers_add('name', on_failure='ignore')) + assert_status( + 'error', + ds.containers_add('name', on_failure='ignore', **common_kwargs)) # no config change ok_clean_git(ds.path) # place a dummy "image" file with open(op.join(ds.path, 'dummy'), 'w') as f: f.write('some') - ds.save('dummy') + ds.save('dummy', **common_kwargs) ok_clean_git(ds.path) # config will be added, as long as there is a file, even when URL access # fails res = ds.containers_add( 'broken', url='bogus-protocol://bogus-server', image='dummy', - on_failure='ignore' + on_failure='ignore', + **common_kwargs ) assert_status('ok', res) assert_result_count(res, 1, action='save', status='ok') @@ -59,7 +64,7 @@ def test_add_noop(path=None): @with_tree(tree={"foo.img": "doesn't matter 0", "bar.img": "doesn't matter 1"}) def test_add_local_path(path=None, local_file=None): - ds = Dataset(path).create() + ds = Dataset(path).create(**common_kwargs) res = ds.containers_add(name="foobert", url=op.join(local_file, "foo.img")) foo_target = op.join(path, ".datalad", "environments", "foobert", "image") @@ -94,12 +99,12 @@ def test_container_files(ds_path=None, local_file=None, url=None): local_file = get_local_file_url(op.join(local_file, 'some_container.img')) # prepare dataset: - ds = Dataset(ds_path).create() + ds = Dataset(ds_path).create(**common_kwargs) # non-default location: ds.config.add("datalad.containers.location", value=op.join(".datalad", "test-environments"), where='dataset') - ds.save(message="Configure container mountpoint") + ds.save(message="Configure container mountpoint", **common_kwargs) # no containers yet: res = ds.containers_list(**RAW_KWDS) @@ -108,7 +113,7 @@ def test_container_files(ds_path=None, local_file=None, url=None): # add first "image": must end up at the configured default location target_path = op.join( ds.path, ".datalad", "test-environments", "first", "image") - res = ds.containers_add(name="first", url=local_file) + res = ds.containers_add(name="first", url=local_file, **common_kwargs) ok_clean_git(ds.repo) assert_result_count(res, 1, status="ok", type="file", path=target_path, @@ -125,7 +130,7 @@ def test_container_files(ds_path=None, local_file=None, url=None): # and kill it again # but needs name assert_raises(TypeError, ds.containers_remove) - res = ds.containers_remove('first', remove_image=True) + res = ds.containers_remove('first', remove_image=True, **common_kwargs) assert_status('ok', res) assert_result_count(ds.containers_list(**RAW_KWDS), 0) # image removed @@ -143,25 +148,28 @@ def test_extra_inputs(ds_path=None): overlay2_file = 'overlay2.img' # prepare dataset: - ds = Dataset(ds_path).create(force=True) - ds.save() + ds = Dataset(ds_path).create(force=True, **common_kwargs) + ds.save(**common_kwargs) ds.containers_add( name="container", image=container_file, call_fmt="apptainer exec {img} {cmd}", + **common_kwargs ) ds.containers_add( name="container-with-overlay", image=container_file, call_fmt="apptainer exec --overlay {img_dirpath}/overlay1.img {img} {cmd}", - extra_input=[overlay1_file] + extra_input=[overlay1_file], + **common_kwargs ) ds.containers_add( name="container-with-two-overlays", image=container_file, call_fmt="apptainer exec --overlay {img_dirpath}/overlay1.img --overlay {img_dirpath}/overlay2.img:ro {img} {cmd}", - extra_input=[overlay1_file, overlay2_file] + extra_input=[overlay1_file, overlay2_file], + **common_kwargs ) res = ds.containers_list(**RAW_KWDS) @@ -181,28 +189,33 @@ def test_container_update(ds_path=None, local_file=None, url=None): url_bar = get_local_file_url(op.join(local_file, 'bar.img')) img = op.join(".datalad", "environments", "foo", "image") - ds = Dataset(ds_path).create() + ds = Dataset(ds_path).create(**common_kwargs) - ds.containers_add(name="foo", call_fmt="call-fmt1", url=url_foo) + ds.containers_add(name="foo", call_fmt="call-fmt1", url=url_foo, + **common_kwargs) # Abort without --update flag. - res = ds.containers_add(name="foo", on_failure="ignore") + res = ds.containers_add(name="foo", on_failure="ignore", + **common_kwargs) assert_result_count(res, 1, action="containers_add", status="impossible") # Abort if nothing to update is specified. - res = ds.containers_add(name="foo", update=True, on_failure="ignore") + res = ds.containers_add(name="foo", update=True, on_failure="ignore", + **common_kwargs) assert_result_count(res, 1, action="containers_add", status="impossible", message="No values to update specified") # Update call format. - ds.containers_add(name="foo", update=True, call_fmt="call-fmt2") + ds.containers_add(name="foo", update=True, call_fmt="call-fmt2", + **common_kwargs) assert_equal(ds.config.get("datalad.containers.foo.cmdexec"), "call-fmt2") ok_file_has_content(op.join(ds.path, img), "foo") # Update URL/image. - ds.drop(img) # Make sure it works even with absent content. - res = ds.containers_add(name="foo", update=True, url=url_bar) + ds.drop(img, **common_kwargs) # Make sure it works even with absent content. + res = ds.containers_add(name="foo", update=True, url=url_bar, + **common_kwargs) assert_in_results(res, action="remove", status="ok") assert_in_results(res, action="save", status="ok") ok_file_has_content(op.join(ds.path, img), "bar") @@ -213,7 +226,8 @@ def test_container_update(ds_path=None, local_file=None, url=None): assert_in("Update ", get_commit_msg()) # If we add a new image with update=True should say Configure - res = ds.containers_add(name="foo2", update=True, url=url_bar) + res = ds.containers_add(name="foo2", update=True, url=url_bar, + **common_kwargs) assert_in("Configure ", get_commit_msg()) @@ -223,16 +237,17 @@ def test_container_update(ds_path=None, local_file=None, url=None): def test_container_from_subdataset(ds_path=None, src_subds_path=None, local_file=None): # prepare a to-be subdataset with a registered container - src_subds = Dataset(src_subds_path).create() - src_subds.containers_add(name="first", - url=get_local_file_url(op.join(local_file, - 'some_container.img')) - ) + src_subds = Dataset(src_subds_path).create(**common_kwargs) + src_subds.containers_add( + name="first", + url=get_local_file_url(op.join(local_file, 'some_container.img')), + **common_kwargs + ) # add it as subdataset to a super ds: - ds = Dataset(ds_path).create() - subds = ds.install("sub", source=src_subds_path) + ds = Dataset(ds_path).create(**common_kwargs) + subds = ds.install("sub", source=src_subds_path, **common_kwargs) # add it again one level down to see actual recursion: - subds.install("subsub", source=src_subds_path) + subds.install("subsub", source=src_subds_path, **common_kwargs) # We come up empty without recursive: res = ds.containers_list(recursive=False, **RAW_KWDS) @@ -254,9 +269,9 @@ def test_container_from_subdataset(ds_path=None, src_subds_path=None, local_file ) # not installed subdataset doesn't pose an issue: - sub2 = ds.create("sub2") - assert_result_count(ds.subdatasets(), 2, type="dataset") - ds.uninstall("sub2", check=False) + sub2 = ds.create("sub2", **common_kwargs) + assert_result_count(ds.subdatasets(**common_kwargs), 2, type="dataset") + ds.uninstall("sub2", check=False, **common_kwargs) from datalad.tests.utils_pytest import assert_false assert_false(sub2.is_installed()) @@ -285,17 +300,17 @@ def test_container_from_subdataset(ds_path=None, src_subds_path=None, local_file @with_tempfile def test_list_contains(path=None): - ds = Dataset(path).create() - subds_a = ds.create("a") - subds_b = ds.create("b") - subds_a_c = subds_a.create("c") + ds = Dataset(path).create(**common_kwargs) + subds_a = ds.create("a", **common_kwargs) + subds_b = ds.create("b", **common_kwargs) + subds_a_c = subds_a.create("c", **common_kwargs) add_pyscript_image(subds_a_c, "in-c", "img") add_pyscript_image(subds_a, "in-a", "img") add_pyscript_image(subds_b, "in-b", "img") add_pyscript_image(ds, "in-top", "img") - ds.save(recursive=True) + ds.save(recursive=True, **common_kwargs) assert_result_count(ds.containers_list(recursive=True, **RAW_KWDS), 4) diff --git a/datalad_container/tests/test_run.py b/datalad_container/tests/test_run.py index 2c0bca9a..d9d4c5c7 100644 --- a/datalad_container/tests/test_run.py +++ b/datalad_container/tests/test_run.py @@ -41,40 +41,42 @@ testimg_url = 'shub://datalad/datalad-container:testhelper' +common_kwargs = {'result_renderer': 'disabled'} + @with_tree(tree={"dummy0.img": "doesn't matter 0", "dummy1.img": "doesn't matter 1"}) def test_run_mispecified(path=None): - ds = Dataset(path).create(force=True) - ds.save(path=["dummy0.img", "dummy1.img"]) + ds = Dataset(path).create(force=True, **common_kwargs) + ds.save(path=["dummy0.img", "dummy1.img"], **common_kwargs) ok_clean_git(path) # Abort if no containers exist. with assert_raises(ValueError) as cm: - ds.containers_run("doesn't matter") + ds.containers_run("doesn't matter", **common_kwargs) assert_in("No known containers", str(cm.value)) # Abort if more than one container exists but no container name is # specified. - ds.containers_add("d0", image="dummy0.img") - ds.containers_add("d1", image="dummy0.img") + ds.containers_add("d0", image="dummy0.img", **common_kwargs) + ds.containers_add("d1", image="dummy0.img", **common_kwargs) with assert_raises(ValueError) as cm: - ds.containers_run("doesn't matter") + ds.containers_run("doesn't matter", **common_kwargs) assert_in("explicitly specify container", str(cm.value)) # Abort if unknown container is specified. with assert_raises(ValueError) as cm: - ds.containers_run("doesn't matter", container_name="ghost") + ds.containers_run("doesn't matter", container_name="ghost", **common_kwargs) assert_in("Container selection impossible", str(cm.value)) @with_tree(tree={"i.img": "doesn't matter"}) def test_run_unknown_cmdexec_placeholder(path=None): ds = Dataset(path).create(force=True) - ds.containers_add("i", image="i.img", call_fmt="{youdontknowme}") + ds.containers_add("i", image="i.img", call_fmt="{youdontknowme}", **common_kwargs) assert_result_count( - ds.containers_run("doesn't matter", on_failure="ignore"), + ds.containers_run("doesn't matter", on_failure="ignore", **common_kwargs), 1, path=ds.path, action="run", @@ -95,9 +97,10 @@ def test_container_files(path=None, super_path=None): image='righthere', # the next one is auto-guessed #call_fmt='singularity exec {img} {cmd}' + **common_kwargs ) assert_result_count( - ds.containers_list(), 1, + ds.containers_list(**common_kwargs), 1, path=op.join(ds.path, 'righthere'), name='mycontainer') ok_clean_git(path) @@ -113,7 +116,7 @@ def assert_no_change(res, path): # now we can run stuff in the container # and because there is just one, we don't even have to name the container - res = ds.containers_run(cmd) + res = ds.containers_run(cmd, **common_kwargs) # container becomes an 'input' for `run` -> get request, but "notneeded" assert_result_count( res, 1, action='get', status='notneeded', @@ -122,7 +125,7 @@ def assert_no_change(res, path): # same thing as we specify the container by its name: res = ds.containers_run(cmd, - container_name='mycontainer') + container_name='mycontainer', **common_kwargs) # container becomes an 'input' for `run` -> get request, but "notneeded" assert_result_count( res, 1, action='get', status='notneeded', @@ -131,7 +134,7 @@ def assert_no_change(res, path): # we can also specify the container by its path: res = ds.containers_run(cmd, - container_name=op.join(ds.path, 'righthere')) + container_name=op.join(ds.path, 'righthere'), **common_kwargs) # container becomes an 'input' for `run` -> get request, but "notneeded" assert_result_count( res, 1, action='get', status='notneeded', @@ -141,15 +144,15 @@ def assert_no_change(res, path): # Now, test the same thing, but with this dataset being a subdataset of # another one: - super_ds = Dataset(super_path).create() - super_ds.install("sub", source=path) + super_ds = Dataset(super_path).create(**common_kwargs) + super_ds.install("sub", source=path, **common_kwargs) # When running, we don't discover containers in subdatasets with assert_raises(ValueError) as cm: - super_ds.containers_run(cmd) + super_ds.containers_run(cmd, **common_kwargs) assert_in("No known containers", str(cm.value)) # ... unless we need to specify the name - res = super_ds.containers_run(cmd, container_name="sub/mycontainer") + res = super_ds.containers_run(cmd, container_name="sub/mycontainer", **common_kwargs) # container becomes an 'input' for `run` -> get request (needed this time) assert_result_count( res, 1, action='get', status='ok', @@ -160,33 +163,34 @@ def assert_no_change(res, path): @with_tempfile @with_tree(tree={'some_container.img': "doesn't matter"}) def test_non0_exit(path=None, local_file=None): - ds = Dataset(path).create() + ds = Dataset(path).create(**common_kwargs) # plug in a proper singularity image ds.containers_add( 'mycontainer', url=get_local_file_url(op.join(local_file, 'some_container.img')), image='righthere', - call_fmt="sh -c '{cmd}'" + call_fmt="sh -c '{cmd}'", + **common_kwargs ) - ds.save() # record the effect in super-dataset + ds.save(**common_kwargs) # record the effect in super-dataset ok_clean_git(path) # now we can run stuff in the container # and because there is just one, we don't even have to name the container - ds.containers_run(['touch okfile']) + ds.containers_run(['touch okfile'], **common_kwargs) assert_repo_status(path) # Test that regular 'run' behaves as expected -- it does not proceed to save upon error with pytest.raises(IncompleteResultsError): - ds.run(['sh', '-c', 'touch nokfile && exit 1']) + ds.run(['sh', '-c', 'touch nokfile && exit 1'], **common_kwargs) assert_repo_status(path, untracked=['nokfile']) (ds.pathobj / "nokfile").unlink() # remove for the next test assert_repo_status(path) # Now test with containers-run which should behave similarly -- not save in case of error with pytest.raises(IncompleteResultsError): - ds.containers_run(['touch nokfile && exit 1']) + ds.containers_run(['touch nokfile && exit 1'], **common_kwargs) # check - must have created the file but not saved anything since failed to run! assert_repo_status(path, untracked=['nokfile']) @@ -194,8 +198,8 @@ def test_non0_exit(path=None, local_file=None): @with_tempfile @with_tree(tree={'some_container.img': "doesn't matter"}) def test_custom_call_fmt(path=None, local_file=None): - ds = Dataset(path).create() - subds = ds.create('sub') + ds = Dataset(path).create(**common_kwargs) + subds = ds.create('sub', **common_kwargs) # plug in a proper singularity image subds.containers_add( @@ -204,9 +208,10 @@ def test_custom_call_fmt(path=None, local_file=None): image='righthere', call_fmt='echo image={img} cmd={cmd} img_dspath={img_dspath} ' # and environment variable being set/propagated by default - 'name=$DATALAD_CONTAINER_NAME' + 'name=$DATALAD_CONTAINER_NAME', + **common_kwargs, ) - ds.save() # record the effect in super-dataset + ds.save(**common_kwargs) # record the effect in super-dataset # Running should work fine either within sub or within super out = WitlessRunner(cwd=subds.path).run( @@ -239,8 +244,8 @@ def test_custom_call_fmt(path=None, local_file=None): } ) def test_extra_inputs(path=None): - ds = Dataset(path).create(force=True) - subds = ds.create("sub", force=True) + ds = Dataset(path).create(force=True, **common_kwargs) + subds = ds.create("sub", force=True, **common_kwargs) subds.containers_add( "mycontainer", image="containers/container.img", @@ -250,9 +255,10 @@ def test_extra_inputs(path=None): "{img_dirpath}/../overlays/overlay2.img", "{img_dspath}/overlays/overlay3.img", ], + **common_kwargs ) - ds.save(recursive=True) # record the entire tree of files etc - ds.containers_run("XXX", container_name="sub/mycontainer") + ds.save(recursive=True, **common_kwargs) # record the entire tree of files etc + ds.containers_run("XXX", container_name="sub/mycontainer", **common_kwargs) ok_file_has_content( os.path.join(ds.repo.path, "out.log"), "image=sub/containers/container.img", @@ -273,10 +279,10 @@ def test_extra_inputs(path=None): @skip_if_no_network @with_tree(tree={"subdir": {"in": "innards"}}) def test_run_no_explicit_dataset(path=None): - ds = Dataset(path).create(force=True) - ds.save() + ds = Dataset(path).create(force=True, **common_kwargs) + ds.save(**common_kwargs) ds.containers_add("deb", url=testimg_url, - call_fmt="singularity exec {img} {cmd}") + call_fmt="singularity exec {img} {cmd}", **common_kwargs) # When no explicit dataset is given, paths are interpreted as relative to # the current working directory. @@ -285,14 +291,14 @@ def test_run_no_explicit_dataset(path=None): with chpwd(path): containers_run("cat {inputs[0]} {inputs[0]} >doubled", inputs=[op.join("subdir", "in")], - outputs=["doubled"]) + outputs=["doubled"], **common_kwargs) ok_file_has_content(op.join(path, "doubled"), "innardsinnards") # From under a subdirectory. subdir = op.join(ds.path, "subdir") with chpwd(subdir): containers_run("cat {inputs[0]} {inputs[0]} >doubled", - inputs=["in"], outputs=["doubled"]) + inputs=["in"], outputs=["doubled"], **common_kwargs) ok_file_has_content(op.join(subdir, "doubled"), "innardsinnards") @@ -316,16 +322,16 @@ def test_run_subdataset_install(path=None): # `-- d/ # `-- d2/ # `-- img - ds_src_a = ds_src.create("a") - ds_src_a2 = ds_src_a.create("a2") - ds_src_b = Dataset(ds_src.pathobj / "b").create() - ds_src_b2 = ds_src_b.create("b2") - ds_src_c = ds_src.create("c") - ds_src_c2 = ds_src_c.create("c2") - ds_src_d = Dataset(ds_src.pathobj / "d").create() - ds_src_d2 = ds_src_d.create("d2") + ds_src_a = ds_src.create("a", **common_kwargs) + ds_src_a2 = ds_src_a.create("a2", **common_kwargs) + ds_src_b = Dataset(ds_src.pathobj / "b").create(**common_kwargs) + ds_src_b2 = ds_src_b.create("b2", **common_kwargs) + ds_src_c = ds_src.create("c", **common_kwargs) + ds_src_c2 = ds_src_c.create("c2", **common_kwargs) + ds_src_d = Dataset(ds_src.pathobj / "d").create(**common_kwargs) + ds_src_d2 = ds_src_d.create("d2", **common_kwargs) - ds_src.save() + ds_src.save(**common_kwargs) add_pyscript_image(ds_src_a, "in-a", "img") add_pyscript_image(ds_src_a2, "in-a2", "img") @@ -333,9 +339,9 @@ def test_run_subdataset_install(path=None): add_pyscript_image(ds_src_c2, "in-c2", "img") add_pyscript_image(ds_src_d2, "in-d2", "img") - ds_src.save(recursive=True) + ds_src.save(recursive=True, **common_kwargs) - ds_dest = clone(ds_src.path, str(path / "dest")) + ds_dest = clone(ds_src.path, str(path / "dest"), **common_kwargs) ds_dest_a2 = Dataset(ds_dest.pathobj / "a" / "a2") ds_dest_b2 = Dataset(ds_dest.pathobj / "b" / "b2") ds_dest_c2 = Dataset(ds_dest.pathobj / "c" / "c2") @@ -346,7 +352,7 @@ def test_run_subdataset_install(path=None): assert_false(ds_dest_d2.is_installed()) # Needed subdatasets are installed if container name is given... - res = ds_dest.containers_run(["arg"], container_name="a/a2/in-a2") + res = ds_dest.containers_run(["arg"], container_name="a/a2/in-a2", **common_kwargs) assert_result_count( res, 1, action="install", status="ok", path=ds_dest_a2.path) assert_result_count( @@ -354,7 +360,7 @@ def test_run_subdataset_install(path=None): path=str(ds_dest_a2.pathobj / "img")) ok_(ds_dest_a2.is_installed()) # ... even if the name and path do not match. - res = ds_dest.containers_run(["arg"], container_name="b/b2/in-b2") + res = ds_dest.containers_run(["arg"], container_name="b/b2/in-b2", **common_kwargs) assert_result_count( res, 1, action="install", status="ok", path=ds_dest_b2.path) assert_result_count( @@ -362,15 +368,15 @@ def test_run_subdataset_install(path=None): path=str(ds_dest_b2.pathobj / "img")) ok_(ds_dest_b2.is_installed()) # Subdatasets will also be installed if given an image path... - res = ds_dest.containers_run(["arg"], container_name=str(Path("c/c2/img"))) + res = ds_dest.containers_run(["arg"], container_name=str(Path("c/c2/img")), **common_kwargs) assert_result_count( res, 1, action="install", status="ok", path=ds_dest_c2.path) assert_result_count( res, 1, action="get", status="ok", path=str(ds_dest_c2.pathobj / "img")) ok_(ds_dest_c2.is_installed()) - ds_dest.containers_run(["arg"], container_name=str(Path("d/d2/img"))) + ds_dest.containers_run(["arg"], container_name=str(Path("d/d2/img")), **common_kwargs) # There's no install record if subdataset is already present. - res = ds_dest.containers_run(["arg"], container_name="a/a2/in-a2") + res = ds_dest.containers_run(["arg"], container_name="a/a2/in-a2", **common_kwargs) assert_not_in_results(res, action="install")