Skip to content
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
eb85d97
test for copy_examples
cadenmyers13 Oct 13, 2025
918c559
news
cadenmyers13 Oct 13, 2025
6a26c9e
rm comment
cadenmyers13 Oct 13, 2025
3ab2efb
rm old tests
cadenmyers13 Oct 13, 2025
678d0b9
add user_input as param for copy_examples
cadenmyers13 Oct 14, 2025
15fd82a
add user inputs to test
cadenmyers13 Oct 14, 2025
d57e7fa
add cwd and user_target at same level as cases in tmp dir
cadenmyers13 Oct 14, 2025
79fa679
modify test for new tmp dir structure
cadenmyers13 Oct 14, 2025
a1fcf3b
fix testing matrix
cadenmyers13 Oct 14, 2025
fc1d0b0
add test for bad copy cases
cadenmyers13 Oct 14, 2025
a50c442
rm docstring
cadenmyers13 Oct 14, 2025
a04de5a
Add FileExistError test case
cadenmyers13 Oct 14, 2025
aa453b7
reverse logic for target_dir
cadenmyers13 Oct 14, 2025
e75a339
add additional dirs to tmp dir
cadenmyers13 Oct 15, 2025
f750453
move copy_examples inside PacksManager
cadenmyers13 Oct 15, 2025
49e745e
move copy_examples tests inside pm test
cadenmyers13 Oct 15, 2025
14fac59
rm tests from test_cli
cadenmyers13 Oct 15, 2025
909da70
add bad_target test
cadenmyers13 Oct 15, 2025
e66eb5f
rm duplicate copy_examples() in cli.py
cadenmyers13 Oct 16, 2025
42553c1
update docstring and param name
cadenmyers13 Oct 16, 2025
ef792d2
wrap str as list, add 4) to bad input test
cadenmyers13 Oct 16, 2025
4dc1538
rm param name from func signature
cadenmyers13 Oct 17, 2025
5820dd3
change scope from session to function
cadenmyers13 Oct 17, 2025
84e54a8
rglob examples after they are copied
cadenmyers13 Oct 17, 2025
6c38885
write text to the files and make check that the copied files have the…
cadenmyers13 Oct 17, 2025
3405e17
check exception message rather than the exception type
cadenmyers13 Oct 17, 2025
b8e4336
copy_examples returns None
cadenmyers13 Oct 17, 2025
1324185
sort list of paths, rm bad target test
cadenmyers13 Oct 17, 2025
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
23 changes: 23 additions & 0 deletions news/copy-func2.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* Add test for ``copy_examples`` function.

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
6 changes: 5 additions & 1 deletion src/diffpy/cmi/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,18 @@


def copy_examples(
examples_dict: Dict[str, List[Tuple[str, Path]]], target_dir: Path = None
examples_dict: Dict[str, List[Tuple[str, Path]]],
user_input: List[str],
target_dir: Path = None,
) -> None:
"""Copy an example into the the target or current working directory.
Parameters
----------
examples_dict : dict
Dictionary mapping pack name -> list of (example, path) tuples.
user_input : list of str
List of user-specified pack(s) or example(s) to copy.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

please review. Think this would be useful for parsing examples_dict

target_dir : pathlib.Path, optional
Target directory to copy examples into. Defaults to current
working directory.
Expand Down
18 changes: 18 additions & 0 deletions src/diffpy/cmi/packsmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,24 @@ def available_examples(self) -> dict[str, List[tuple[str, Path]]]:
)
return examples_dict

def copy_examples(
self,
user_input: Union[str, List[str]],
target_dir: Path = None,
) -> List[Path]:
"""Copy examples or packs into the target or current working
directory.

Parameters
----------
user_input : str or list of str
User-specified pack(s), example(s), or "all" to copy all.
target_dir : pathlib.Path, optional
Target directory to copy examples into. Defaults to current
working directory.
"""
return

def _resolve_pack_file(self, identifier: Union[str, Path]) -> Path:
"""Resolve a pack identifier to an absolute .txt path.

Expand Down
14 changes: 13 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ def example_cases(tmp_path_factory):
Returns the path to that copy.
"""
root_temp_dir = tmp_path_factory.mktemp("temp")

cwd = root_temp_dir / "cwd"
cwd.mkdir(parents=True, exist_ok=True)
user_target = root_temp_dir / "user_target"
user_target.mkdir(parents=True, exist_ok=True)
# case 1: pack with no examples
case1ex_dir = root_temp_dir / "case1" / "docs" / "examples"
case1 = case1ex_dir / "empty_pack" # empty_pack
Expand Down Expand Up @@ -86,6 +89,15 @@ def example_cases(tmp_path_factory):
case5b = case5ex_dir / "packB" / "ex1" / "path2" # packB, ex1
case5b.mkdir(parents=True, exist_ok=True)
(case5b / "script2.py").touch()
case5c = case5ex_dir / "packA" / "ex2" # packB, ex2
case5c.mkdir(parents=True, exist_ok=True)
(case5c / "script3.py").touch()
case5d = case5ex_dir / "packB" / "ex3"
case5d.mkdir(parents=True, exist_ok=True)
(case5d / "script4.py").touch()
case5e = case5ex_dir / "packB" / "ex4"
case5e.mkdir(parents=True, exist_ok=True)
(case5e / "script5.py").touch()
case5req_dir = root_temp_dir / "case5" / "requirements" / "packs"
case5req_dir.mkdir(parents=True, exist_ok=True)

Expand Down
65 changes: 0 additions & 65 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,65 +0,0 @@
import os
from pathlib import Path

import pytest

from diffpy.cmi import cli


def test_map_pack_to_examples_structure():
"""Test that map_pack_to_examples returns the right shape of
data."""
actual = cli.map_pack_to_examples()
assert isinstance(actual, dict)
for pack, exdirs in actual.items():
assert isinstance(pack, str)
assert isinstance(exdirs, list)
for ex in exdirs:
assert isinstance(ex, str)
# Check for known packs
assert "core" in actual.keys()
assert "pdf" in actual.keys()
# Check for known examples
assert ["linefit"] in actual.values()


@pytest.mark.parametrize(
"input_valid_str",
[
"core/linefit",
"pdf/ch03NiModelling",
],
)
def test_copy_example_success(tmp_path, input_valid_str):
"""Given a valid example format (<pack>/<ex>), test that its copied
to the temp dir."""
os.chdir(tmp_path)
actual = cli.copy_example(input_valid_str)
expected = tmp_path / Path(input_valid_str).name
assert expected.exists() and expected.is_dir()
assert actual == expected


def test_copy_example_fnferror():
"""Test that FileNotFoundError is raised when the example does not
exist."""
with pytest.raises(FileNotFoundError):
cli.copy_example("pack/example1")


@pytest.mark.parametrize(
"input_bad_str",
[
"", # empty string
"/", # missing pack and example
"corelinefit", # missing slash
"linefit", # missing pack and slash
"core/", # missing example
"/linefit", # missing pack
"core/linefit/extra", # too many slashes
],
)
def test_copy_example_valueerror(input_bad_str):
"""Test that ValueError is raised when the format is invalid."""
with pytest.raises(ValueError):
cli.copy_example(input_bad_str)
192 changes: 192 additions & 0 deletions tests/test_packsmanager.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import os
from pathlib import Path

import pytest

from diffpy.cmi.packsmanager import PacksManager
Expand Down Expand Up @@ -63,9 +66,12 @@ def paths_and_names_match(expected, actual, root):
{
"packA": [
("ex1", "case5/docs/examples/packA/ex1"),
("ex2", "case5/docs/examples/packA/ex2"),
],
"packB": [
("ex1", "case5/docs/examples/packB/ex1"),
("ex3", "case5/docs/examples/packB/ex3"),
("ex4", "case5/docs/examples/packB/ex4"),
],
},
),
Expand All @@ -92,3 +98,189 @@ def test_tmp_file_structure(input, expected, example_cases):
assert path.is_file()
else:
assert path.is_dir()


copy_params = [
# Test various use cases to copy_examples on case5
# 1) copy one example (ambiguous)
# 2) copy list of examples from same pack (ambiguous)
# 3) copy list of examples from different pack (ambiguous)
# 4) copy one example (unambiguous)
# 5) copy list of examples from same pack (unambiguous)
# 6) copy list of examples from different pack (unambiguous)
# 7) copy all examples from a pack
# 8) copy all examples from list of packs
# 9) copy all examples from all packs
( # 1) copy one example, (ambiguous)
"ex1",
[
Path("packA/ex1/path1/script1.py"),
Path("packB/ex1/path2/script2.py"),
],
),
( # 2) copy list of examples from same pack (ambiguous)
["ex1", "ex2"],
[
Path("packA/ex1/path1/script1.py"),
Path("packB/ex1/path2/script2.py"),
Path("packA/ex2/script3.py"),
],
),
( # 3) copy list of examples from different packs (ambiguous)
["ex1", "ex1"],
[
Path("packA/ex1/path1/script1.py"),
Path("packB/ex1/path2/script2.py"),
],
),
( # 4) copy one example (unambiguous)
"ex2",
[
Path("packA/ex2/script3.py"),
],
),
( # 5) copy list of examples from same pack (unambiguous)
["ex3", "ex4"],
[
Path("packB/ex3/script4.py"),
Path("packB/ex4/script5.py"),
],
),
( # 6) copy list of examples from different packs (unambiguous)
["ex2", "ex3"],
[
Path("packA/ex2/script3.py"),
Path("packB/ex3/script4.py"),
],
),
( # 7) copy all examples from a pack
"packA",
[
Path("packA/ex1/path1/script1.py"),
Path("packA/ex2/script3.py"),
],
),
( # 8) copy all examples from list of packs
["packA", "packB"],
[
Path("packA/ex1/path1/script1.py"),
Path("packA/ex2/script3.py"),
Path("packB/ex1/path2/script2.py"),
Path("packB/ex3/script4.py"),
Path("packB/ex4/script5.py"),
],
),
( # 9) copy all examples from all packs
"all",
[
Path("packA/ex1/path1/script1.py"),
Path("packA/ex2/script3.py"),
Path("packB/ex1/path2/script2.py"),
Path("packB/ex3/script4.py"),
Path("packB/ex4/script5.py"),
],
),
]


# input: list of str - cli input(s) to copy_examples
# expected_paths: list of Path - expected relative paths to copied examples
@pytest.mark.parametrize("input,expected_paths", copy_params)
def test_copy_examples(input, expected_paths, example_cases):
examples_dir = example_cases / "case5"
pm = PacksManager(root_path=examples_dir)
target_dir = example_cases / "user_target"
actual = pm.copy_examples(user_input=input, target_dir=target_dir)
expected = []
for path in expected_paths:
root_path = target_dir / path
Copy link
Contributor

Choose a reason for hiding this comment

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

root path is a bad name here. In what sense is this a root_path?

How about this for this work:

expected = [target_dir / path for path in expected_paths]

Then to test the files were copied correctly we would need something like

actual_gen = target_dir.rglob('*')

and syntax that iterates through the lists and (a) checks that they contain the same elements and (b) that the files exist and (c) optionally, but not necessarily, check that the files contain the right contents. I think (c) may be overkill, but strictly if we don't we could write the function that passes the test just by touching those files and not copying them. It will be some work though because I think we are creating those files empty so we would have to write something in them. I think there is a way to create and write them in one line though in conftest, so it could be quick to do. Let's just think about it.

expected.append(root_path)
assert actual == expected


# Test default and targeted copy_example location on case5
# input: str or None - path arg to copy_examples
# expected: Path - expected relative path to copied example
@pytest.mark.parametrize(
"input,expected_path",
[
(None, Path("cwd/packA/ex1/path1/script1.py")),
("user_target", Path("user_target/packA/ex1/path1/script1.py")),
],
)
def test_copy_examples_location(input, expected_path, example_cases):
examples_dir = example_cases / "case5"
os.chdir(example_cases / "cwd")
pm = PacksManager(root_path=examples_dir)
paths = pm.copy_examples(user_input="packA", target_dir=input)
actual = paths[0]
expected = example_cases / expected_path
assert actual == expected


# Test bad inputs to copy_examples on case3
# These include:
# 1) input not found (example or pack)
# 2) mixed good and bad inputs
# 3) Path to directory already exists
@pytest.mark.parametrize(
"bad_inputs,expected,path",
[
(
"bad_example",
ValueError,
Copy link
Contributor

Choose a reason for hiding this comment

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

normally we would check the error message. In this case we use the tests to define good (helpful) error messages.

None,
), # input not found (example or pack)
(
["ex1", "bad_example"],
ValueError,
None,
), # mixed good ex and bad inputs
(
["packA", "bad_example"],
ValueError,
None,
), # mixed good pack and bad inputs
(
"ex1",
FileExistsError,
Path("docs/examples/"),
), # path to dir already exists
],
)
def test_copy_examples_bad(bad_inputs, expected, path, example_cases):
examples_dir = example_cases / "case3"
pm = PacksManager(root_path=examples_dir)
with pytest.raises(expected):
pm.copy_examples(
user_input=bad_inputs,
target_dir=examples_dir / path if path is not None else None,
)


# Test bad target_dir to copy_examples on case3
# 1) target doesn't exist
# 2) target is a file
# 3) target is nested in a file
@pytest.mark.parametrize(
"bad_inputs,expected",
[
(Path("nonexistent/path/target"), FileNotFoundError), # doesn't exist
Copy link
Contributor

Choose a reason for hiding this comment

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

please test error messages here too.

(
Path("docs/examples/packA/ex1/script4.py"),
NotADirectoryError,
), # target is a file
(
Path("docs/examples/packA/ex1/script4.py/nested"),
NotADirectoryError,
), # nested in file
],
)
def test_copy_examples_bad_target(bad_inputs, expected, example_cases):
examples_dir = example_cases / "case3"
pm = PacksManager(root_path=examples_dir)
with pytest.raises(expected):
pm.copy_examples(
user_input="packA",
target_dir=bad_inputs,
)
Loading