Skip to content

Commit 89674cb

Browse files
jmchiltonjmchilton
committed
Improved comments and tests for shed related stuff.
With spelling fixes from @nsoranzo.
1 parent c0b2883 commit 89674cb

File tree

6 files changed

+117
-43
lines changed

6 files changed

+117
-43
lines changed

‎planemo/shed/__init__.py‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
"""Abstractions for shed related interactions used by the rest of planemo."""
12
from collections import namedtuple
23
import contextlib
34
import copy
@@ -63,6 +64,8 @@
6364
"repositories.")
6465
INCORRECT_OWNER_MESSAGE = ("Attempting to create a repository with configured "
6566
"owner [%s] that does not matche API user [%s].")
67+
PROBLEM_PROCESSING_REPOSITORY_MESSAGE = "Problem processing repositories, exiting."
68+
6669
# Planemo generated or consumed files that do not need to be uploaded to the
6770
# tool shed.
6871
PLANEMO_FILES = [
@@ -153,6 +156,7 @@ def _shed_context_owner(self):
153156

154157

155158
def shed_init(ctx, path, **kwds):
159+
"""Intialize a new shed repository."""
156160
if not os.path.exists(path):
157161
os.makedirs(path)
158162
shed_config_path = os.path.join(path, SHED_CONFIG_NAME)
@@ -185,8 +189,7 @@ def shed_init(ctx, path, **kwds):
185189

186190

187191
def install_arg_lists(ctx, paths, **kwds):
188-
""" Build a list of install args for resolved repositories.
189-
"""
192+
"""Build a list of install args for resolved repositories."""
190193
shed_context = get_shed_context(ctx, **kwds)
191194
install_args_list = []
192195

@@ -196,7 +199,7 @@ def process_repo(realized_repository):
196199

197200
exit_code = for_each_repository(ctx, process_repo, paths, **kwds)
198201
if exit_code:
199-
raise RuntimeError("Problem processing repositories, exiting.")
202+
raise RuntimeError(PROBLEM_PROCESSING_REPOSITORY_MESSAGE)
200203

201204
return install_args_list
202205

@@ -241,8 +244,7 @@ def report_non_existent_repository(realized_repository):
241244

242245

243246
def upload_repository(ctx, realized_repository, **kwds):
244-
"""Upload a tool directory as a tarball to a tool shed.
245-
"""
247+
"""Upload a tool directory as a tarball to a tool shed."""
246248
path = realized_repository.path
247249
tar_path = kwds.get("tar", None)
248250
if not tar_path:
@@ -261,7 +263,7 @@ def upload_repository(ctx, realized_repository, **kwds):
261263
return report_non_existent_repository(realized_repository)
262264

263265
if kwds.get("check_diff", False):
264-
is_diff = diff_repo(ctx, realized_repository, **kwds)
266+
is_diff = diff_repo(ctx, realized_repository, **kwds) != 0
265267
if not is_diff:
266268
name = realized_repository.name
267269
info("Repository [%s] not different, skipping upload." % name)
@@ -295,6 +297,11 @@ def _update_commit_message(ctx, realized_repository, update_kwds, **kwds):
295297

296298

297299
def diff_repo(ctx, realized_repository, **kwds):
300+
"""Compare two repositories (local or remote) and check for differences.
301+
302+
Returns 0 if and only the repositories are effectively the same
303+
given supplied kwds for comparison description.
304+
"""
298305
with temp_directory("tool_shed_diff_") as working:
299306
return _diff_in(ctx, working, realized_repository, **kwds)
300307

@@ -321,7 +328,7 @@ def _diff_in(ctx, working, realized_repository, **kwds):
321328
# $ diff README.rst not_a_file 2&>1 /dev/null; echo $?
322329
# 2
323330
return 2
324-
info("Diffing repository %s " % realized_repository.name)
331+
info("Diffing repository [%s]" % realized_repository.name)
325332
download_tarball(
326333
ctx,
327334
shed_context,

‎planemo/shed/diff.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
"""Utilities for calculating effective repository diffs.
2+
3+
Some intelligence is required because the tool shed updates attributes that it
4+
is beneficial to ignore.
5+
"""
16
from __future__ import print_function
27

38
import os
@@ -8,6 +13,11 @@
813

914

1015
def diff_and_remove(working, label_a, label_b, f):
16+
"""Remove tool shed XML files and use a smart XML diff on them.
17+
18+
Return 0 if and only if the XML content is the sam after stripping
19+
attirbutes the tool shed updates.
20+
"""
1121
assert label_a != label_b
1222
special = ["tool_dependencies.xml", "repository_dependencies.xml"]
1323
deps_diff = 0
@@ -26,6 +36,11 @@ def diff_and_remove(working, label_a, label_b, f):
2636

2737

2838
def _shed_diff(file_a, file_b, f=sys.stdout):
39+
"""Strip attributes the tool shed writes and do smart XML diff.
40+
41+
Returns 0 if and only if the XML content is the same after stripping
42+
``tool_shed`` and ``changeset_revision`` attributes.
43+
"""
2944
xml_a = ElementTree.parse(file_a).getroot()
3045
xml_b = ElementTree.parse(file_b).getroot()
3146
_strip_shed_attributes(xml_a)
@@ -46,3 +61,6 @@ def _remove_attribs(xml_element):
4661
for attrib in ["changeset_revision", "toolshed"]:
4762
if attrib in xml_element.attrib:
4863
del xml_element.attrib[attrib]
64+
65+
66+
__all__ = ["diff_and_remove"]

‎planemo/xml/diff.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11

22
def diff(x1, x2, reporter=None):
3+
"""Return 0 if and only if the XML has the same content."""
34
compare = xml_compare(x1, x2, reporter)
45
return_val = 0 if compare else 1
56
return return_val

‎tests/test_shed_diff.py‎

Lines changed: 12 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,16 @@
44
from os.path import join
55
from .test_utils import (
66
CliShedTestCase,
7+
TEST_REPOS_DIR,
78
)
89
from planemo import io
910
import tempfile
10-
import shutil
1111
import sys
1212
import os
1313
from xml.etree import ElementTree
1414
from planemo.xml.diff import diff
15-
from .test_utils import TEST_REPOS_DIR
15+
16+
from .test_shed_upload import update_package_1
1617

1718
DIFF_LINES = [
1819
"diff -r _workingdir_/related_file _custom_shed_/related_file",
@@ -43,11 +44,7 @@ def test_diff_doesnt_exist(self):
4344

4445
def test_diff_recursive(self):
4546
with self._isolate_repo("multi_repos_nested") as f:
46-
upload_command = [
47-
"shed_upload", "-r", "--force_repository_creation"
48-
]
49-
upload_command.extend(self._shed_args())
50-
self._check_exit_code(upload_command)
47+
self._shed_create(recursive=True)
5148

5249
diff_command = ["shed_diff", "-r"]
5350
diff_command.extend(self._shed_args(read_only=True))
@@ -61,11 +58,7 @@ def test_diff_recursive(self):
6158

6259
def test_shed_diff_raw(self):
6360
with self._isolate_repo("suite_auto"):
64-
upload_command = [
65-
"shed_upload", "--force_repository_creation",
66-
]
67-
upload_command.extend(self._shed_args())
68-
self._check_exit_code(upload_command)
61+
self._shed_create()
6962

7063
diff_command = [
7164
"shed_diff", "-o", "diff"
@@ -82,37 +75,25 @@ def test_shed_diff_raw(self):
8275

8376
def test_shed_diff_xml_no_diff(self):
8477
with self._isolate_repo("package_1"):
85-
upload_command = [
86-
"shed_upload", "--force_repository_creation",
87-
]
88-
upload_command.extend(self._shed_args())
89-
self._check_exit_code(upload_command)
78+
self._shed_create()
79+
9080
diff_command = ["shed_diff"]
9181
diff_command.extend(self._shed_args(read_only=True))
9282
self._check_exit_code(diff_command, exit_code=0)
9383

9484
def test_shed_diff_xml_diff(self):
9585
with self._isolate_repo("package_1") as f:
96-
upload_command = [
97-
"shed_upload", "--force_repository_creation",
98-
]
99-
upload_command.extend(self._shed_args())
100-
self._check_exit_code(upload_command)
101-
changed_xml = os.path.join(TEST_REPOS_DIR,
102-
"package_1_changed",
103-
"tool_dependencies.xml")
104-
shutil.copyfile(changed_xml, join(f, "tool_dependencies.xml"))
86+
self._shed_create()
87+
88+
update_package_1(f)
89+
10590
diff_command = ["shed_diff"]
10691
diff_command.extend(self._shed_args(read_only=True))
10792
self._check_exit_code(diff_command, exit_code=1)
10893

10994
def test_diff_xunit(self):
11095
with self._isolate_repo("multi_repos_nested") as f:
111-
upload_command = [
112-
"shed_upload", "-r", "--force_repository_creation"
113-
]
114-
upload_command.extend(self._shed_args())
115-
self._check_exit_code(upload_command)
96+
self._shed_create(recursive=True)
11697

11798
xunit_report = tempfile.NamedTemporaryFile(delete=False)
11899
xunit_report.flush()

‎tests/test_shed_upload.py‎

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
1-
""" Integration tests for shed_upload, shed_download, and shed_create
1+
"""Integration tests for shed contents commands.
2+
3+
Specifically, tests for shed_upload, shed_download, and shed_create.
24
commands.
35
"""
46
from os.path import exists, join
57
import os
68
import tarfile
79
import shutil
810

9-
from .test_utils import CliShedTestCase
11+
from .test_utils import (
12+
CliShedTestCase,
13+
TEST_REPOS_DIR,
14+
)
1015
from planemo.io import shell
1116
from planemo import git
1217

@@ -40,12 +45,15 @@ def test_update_not_exists_update_only(self):
4045

4146
def test_update_with_check_diff(self):
4247
with self._isolate_repo("single_tool") as f:
48+
self._shed_create()
49+
50+
self._assert_shed_diff(diff=0)
51+
4352
upload_command = [
4453
"shed_update", "--force_repository_creation", "--check_diff"
4554
]
4655
upload_command.extend(self._shed_args())
4756
self._check_exit_code(upload_command)
48-
upload_command.append("--check_diff")
4957

5058
# First time no difference.
5159
r = self._check_exit_code(upload_command)
@@ -55,15 +63,44 @@ def test_update_with_check_diff(self):
5563
with open(join(f, "related_file"), "w") as rf:
5664
rf.write("new_contents")
5765

66+
self._assert_shed_diff(diff=1)
67+
5868
# No assert there is no difference again.
5969
r = self._check_exit_code(upload_command)
6070
assert "not different, skipping upload." not in r.output
6171

62-
def test_update_metadata_only(self):
63-
with self._isolate_repo("single_tool"):
72+
self._assert_shed_diff(diff=0)
73+
74+
def test_update_with_check_diff_package(self):
75+
with self._isolate_repo("package_1") as f:
76+
self._shed_create()
77+
78+
self._assert_shed_diff(diff=0)
79+
upload_command = [
80+
"shed_update", "--force_repository_creation", "--check_diff"
81+
]
82+
upload_command.extend(self._shed_args())
83+
self._check_exit_code(upload_command)
84+
85+
# First time no difference.
86+
r = self._check_exit_code(upload_command)
87+
assert "not different, skipping upload." in r.output
88+
89+
update_package_1(f)
90+
self._assert_shed_diff(diff=1)
91+
92+
# No assert there is no difference again.
93+
r = self._check_exit_code(upload_command)
94+
assert "not different, skipping upload." not in r.output
95+
96+
self._assert_shed_diff(diff=0)
97+
98+
def test_update_with_force_create_metadata_only(self):
99+
with self._isolate_repo("single_tool") as f:
64100
upload_command = ["shed_update", "--force_repository_creation", "--skip_upload"]
65101
upload_command.extend(self._shed_args())
66102
self._check_exit_code(upload_command)
103+
self._verify_empty_repository(f)
67104

68105
def test_update_with_force_create(self):
69106
with self._isolate_repo("single_tool") as f:
@@ -290,6 +327,11 @@ def test_upload_with_double_dot(self):
290327
],
291328
not_contains=[])
292329

330+
def _assert_shed_diff(self, diff=1):
331+
shed_diff_command = ["shed_diff"]
332+
shed_diff_command.extend(self._shed_args())
333+
self._check_exit_code(shed_diff_command, exit_code=diff)
334+
293335
def _verify_expansion(self, f, name=None):
294336
upload_command = ["shed_upload", "--tar_only"]
295337
upload_command.extend(self._shed_args())
@@ -320,6 +362,10 @@ def _verify_single_uploaded(self, f, download_args=[]):
320362
f, ["cat.xml", "related_file", "test-data/1.bed"], download_args
321363
)
322364

365+
def _verify_empty_repository(self, f, download_args=[]):
366+
target = self._download_repo(f, download_args)
367+
assert len(os.listdir(target)) == 0
368+
323369
def _verify_upload(self, f, download_files=[], download_args=[]):
324370
target = self._download_repo(f, download_args)
325371
for download_file in download_files:
@@ -371,7 +417,21 @@ def _untar(self, f, path, tarbomb=True):
371417
return target
372418

373419

420+
def update_package_1(f):
421+
"""Update tool dependencies file for package_1."""
422+
changed_xml = join(
423+
TEST_REPOS_DIR,
424+
"package_1_changed",
425+
"tool_dependencies.xml"
426+
)
427+
shutil.copyfile(changed_xml, join(f, "tool_dependencies.xml"))
428+
429+
374430
def assert_exists(path):
431+
"""Assert supplied ``path`` exists.
432+
433+
Produces an informative AssertionError if it is does not.
434+
"""
375435
dir_path = os.path.dirname(path)
376436
msg = None
377437
if not exists(dir_path):

‎tests/test_utils.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,13 @@ def tearDown(self): # noqa
123123
super(CliShedTestCase, self).tearDown()
124124
self.mock_shed.shutdown()
125125

126+
def _shed_create(self, recursive=False):
127+
create_command = ["shed_create"]
128+
if recursive:
129+
create_command.append("-r")
130+
create_command.extend(self._shed_args())
131+
self._check_exit_code(create_command)
132+
126133
def _shed_args(self, read_only=False):
127134
args = [
128135
"--shed_target", self.mock_shed.url,

0 commit comments

Comments
 (0)