Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove some local-dev test detritus #3393

Merged
merged 1 commit into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions parsl/tests/test_bash_apps/test_memoize_ignore_args.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import os

import pytest

import parsl
from parsl.app.app import bash_app

Expand All @@ -23,24 +21,18 @@ def no_checkpoint_stdout_app_ignore_args(stdout=None):
return "echo X"


def test_memo_stdout():
def test_memo_stdout(tmpd_cwd):
path_x = tmpd_cwd / "test.memo.stdout.x"

# this should run and create a file named after path_x
path_x = "test.memo.stdout.x"
if os.path.exists(path_x):
os.remove(path_x)

no_checkpoint_stdout_app_ignore_args(stdout=path_x).result()
assert os.path.exists(path_x)

# this should be memoized, so not create benc.test.y
path_y = "test.memo.stdout.y"
no_checkpoint_stdout_app_ignore_args(stdout=str(path_x)).result()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see other comment about hopefully-redundant str

assert path_x.exists()

if os.path.exists(path_y):
os.remove(path_y)
# this should be memoized, so should not get created
path_y = tmpd_cwd / "test.memo.stdout.y"

no_checkpoint_stdout_app_ignore_args(stdout=path_y).result()
assert not os.path.exists(path_y)
assert not path_y.exists(), "For memoization, expected NO file written"

# this should also be memoized, so not create an arbitrary name
z_fut = no_checkpoint_stdout_app_ignore_args(stdout=parsl.AUTO_LOGNAME)
Expand Down
17 changes: 6 additions & 11 deletions parsl/tests/test_bash_apps/test_memoize_ignore_args_regr.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import copy
import os
from typing import List

import pytest
Expand Down Expand Up @@ -30,21 +29,17 @@ def no_checkpoint_stdout_app(stdout=None):
return "echo X"


def test_memo_stdout():

def test_memo_stdout(tmpd_cwd):
assert const_list_x == const_list_x_arg

path_x = "test.memo.stdout.x"
if os.path.exists(path_x):
os.remove(path_x)
path_x = tmpd_cwd / "test.memo.stdout.x"

# this should run and create a file named after path_x
no_checkpoint_stdout_app(stdout=path_x).result()
assert os.path.exists(path_x)
no_checkpoint_stdout_app(stdout=str(path_x)).result()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recent work should have this still work with path_x being an os.PathLike, which I think is the case here - for example, see the monitoring tests in parsl/tests/test_monitoring/test_stdouterr.py (although those tests only check that the path goes through to monitoring correctly - not that the path really is used by the executing app)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(i.e. this str should be unnecessary)
(and maybe should be os.fspath)
(at which point you might get a bytes)
(and have to deal with that)

path_x.unlink(missing_ok=False)

os.remove(path_x)
no_checkpoint_stdout_app(stdout=path_x).result()
assert not os.path.exists(path_x)
no_checkpoint_stdout_app(stdout=str(path_x)).result()
assert not path_x.exists(), "For memoization, expected NO file written"

# this should also be memoized, so not create an arbitrary name
z_fut = no_checkpoint_stdout_app(stdout=parsl.AUTO_LOGNAME)
Expand Down
19 changes: 4 additions & 15 deletions parsl/tests/test_error_handling/test_retries.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import argparse
import os

import pytest

import parsl
from parsl import bash_app, python_app
from parsl.tests.configs.local_threads import fresh_config

Expand Down Expand Up @@ -68,8 +66,6 @@ def test_fail_nowait(numtasks=10):
assert isinstance(
e, TypeError), "Expected a TypeError, got {}".format(e)

print("Done")


@pytest.mark.local
def test_fail_delayed(numtasks=10):
Expand All @@ -94,19 +90,12 @@ def test_fail_delayed(numtasks=10):
assert isinstance(
e, TypeError), "Expected a TypeError, got {}".format(e)

print("Done")


@pytest.mark.local
def test_retry():
def test_retry(tmpd_cwd):
"""Test retries via app that succeeds on the Nth retry.
"""

fname = "retry.out"
try:
os.remove(fname)
except OSError:
pass
fu = succeed_on_retry(fname)

fu.result()
fpath = tmpd_cwd / "retry.out"
sout = str(tmpd_cwd / "stdout")
succeed_on_retry(str(fpath), stdout=sout).result()
12 changes: 6 additions & 6 deletions parsl/tests/test_staging/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ def test_files():


@pytest.mark.local
def test_open():
with open('test-open.txt', 'w') as tfile:
tfile.write('Hello')
def test_open(tmpd_cwd):
fpath = tmpd_cwd / 'test-open.txt'
fpath.write_text('Hello')

pfile = File('test-open.txt')
pfile = File(fpath)

with open(str(pfile), 'r') as opfile:
assert (opfile.readlines()[0] == 'Hello')
with open(pfile) as opfile:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is changing what's being tested: before, it tested that whatever a File returned as its str could be opened. Now it's testing that whatever comes back as its fspath can be opened.

That str implementation is a bit horrible (it raises an exception for File objects which don't have a local representation) but I think it's still needed as part of usability: for example, people writing bash_apps would traditionally write f"cat {myfile}" when forming their bash command lines rather than `f"cat {os.fspath(myfile)}"

So maybe this should be testing both behaviours?

assert (opfile.read() == 'Hello')
Loading