Skip to content

Commit

Permalink
do not store base at nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
bernt-matthias committed Feb 14, 2022
1 parent 267ff21 commit 1204647
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 21 deletions.
4 changes: 0 additions & 4 deletions lib/galaxy/tool_util/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,13 @@ class XMLLintMessageLine(LintMessage):
def __init__(self, level: str, message: str, node: Optional[etree.Element] = None):
super().__init__(level, message)
self.line = None
self.fname = None
if node is not None:
self.line = node.sourceline
self.fname = node.base

def __str__(self) -> str:
rval = super().__str__()
if self.line is not None:
rval += " ("
if self.fname:
rval += f"{self.fname}:"
rval += str(self.line)
rval += ")"
return rval
Expand Down
9 changes: 0 additions & 9 deletions lib/galaxy/util/xml_macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,15 +283,6 @@ def _imported_macro_paths_from_el(macros_el):

def _load_macro_file(path, xml_base_dir):
tree = parse_xml(path, strip_whitespace=False)
for node in tree.iter():
# little hack: node.base contains that path to the file which is used in
# the linter. it is a property that apparently is determined from the
# xmltree, so if the macro is inserted into the main xml node.base will
# give the path of the main xml file.
# luckily lxml allows to set the property by adding xml:base to the node
# which will be returned if the property is read
# https://github.com/lxml/lxml/blob/5a5c7fb01d15af58def4bab2ba7b15c937042835/src/lxml/etree.pyx#L1106
node.base = node.base
root = tree.getroot()
return _load_macros(root, xml_base_dir)

Expand Down
20 changes: 14 additions & 6 deletions test/unit/tool_util/test_tool_linters.py
Original file line number Diff line number Diff line change
Expand Up @@ -1285,7 +1285,7 @@ def test_xml_order(lint_ctx):
"""


def test_tool_and_macro_xml(lint_ctx_xpath):
def test_tool_and_macro_xml(lint_ctx_xpath, lint_ctx):
"""
test linters (all of them via lint_tool_source_with) on a tool and macro xml file
checking a list of asserts, where each assert is a 4-tuple:
Expand All @@ -1304,21 +1304,29 @@ def test_tool_and_macro_xml(lint_ctx_xpath):
tool_xml, _ = load_with_references(tool_path)

tool_source = XmlToolSource(tool_xml)
# lint once with the lint context using XMLLintMessageXPath and XMLLintMessageLine
lint_tool_source_with(lint_ctx_xpath, tool_source)
lint_tool_source_with(lint_ctx, tool_source)

asserts = (
("Select parameter [select] has multiple options with the same value", "tool.xml", 5, "/tool/inputs/param[1]"),
("Found param input with no name specified.", "tool.xml", 13, "/tool/inputs/param[2]"),
("Param input [No_type] input with no type specified.", "macros.xml", 3, "/tool/inputs/param[3]")
("Select parameter [select] has multiple options with the same value", 5, "/tool/inputs/param[1]"),
("Found param input with no name specified.", 13, "/tool/inputs/param[2]"),
("Param input [No_type] input with no type specified.", 3, "/tool/inputs/param[3]")
)
for a in asserts:
message, fname, line, xpath = a
message, line, xpath = a
found = False
for lint_message in lint_ctx_xpath.message_list:
if lint_message.message != message:
continue
found = True
assert lint_message.xpath == xpath, f"Assumed xpath {xpath} xpath {lint_message.xpath} for: {message}"
assert lint_message.xpath == xpath, f"Assumed xpath {xpath}; found xpath {lint_message.xpath} for: {message}"
assert found, f"Did not find {message}"
for lint_message in lint_ctx.message_list:
if lint_message.message != message:
continue
found = True
assert lint_message.line == line, f"Assumed line {line}; found line {lint_message.line} for: {message}"
assert found, f"Did not find {message}"


Expand Down
5 changes: 3 additions & 2 deletions test/unit/tool_util/test_tool_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,11 +642,12 @@ def test_loader_specify_nested_macro_by_token():
# test with re because loading from external macros
# adds a xml:base property (containing the source path)
# to the node which is printed
print(f"{xml_to_string(xml, pretty=True)}")
assert re.match(r"""<\?xml version="1\.0" \?>
<tool>
<macros/>
<A xml:base=".*/external.xml"/>
<B xml:base=".*/external.xml"/>
<A/>
<B/>
</tool>""", xml_to_string(xml, pretty=True), re.MULTILINE)


Expand Down

0 comments on commit 1204647

Please sign in to comment.