Merge branch 'release_24.0' into dev

client/src/components/Citation/CitationsList.vue

@@ -18,7 +18,7 @@ const outputFormats = Object.freeze({
 interface Props {
     id: string;
     source: string;
-    simple: boolean;
+    simple?: boolean;
 }
 
 const props = withDefaults(defineProps<Props>(), {
@@ -29,7 +29,7 @@ const { config } = useConfig(true);
 
 const emit = defineEmits(["rendered", "show", "shown", "hide", "hidden"]);
 
-const outputFormat = ref(outputFormats.CITATION);
+const outputFormat = ref<string>(outputFormats.CITATION);
 const citations = ref<Citation[]>([]);
 
 onUpdated(() => {
@@ -65,7 +65,7 @@ onMounted(async () => {
             </template>
 
             <div v-if="source === 'histories'" class="infomessage">
-                <div v-html="config.citations_export_message_html"></div>
+                <div v-html="config?.citations_export_message_html"></div>
             </div>
 
             <div class="citations-formatted">

client/src/components/History/HistoryScrollList.vue

@@ -148,6 +148,10 @@ const filtered = computed<HistorySummary[]>(() => {
             return -1;
         } else if (!isMultiviewPanel.value && b.id == currentHistoryId.value) {
             return 1;
+        } else if (isMultiviewPanel.value && isPinned(a.id) && !isPinned(b.id)) {
+            return -1;
+        } else if (isMultiviewPanel.value && !isPinned(a.id) && isPinned(b.id)) {
+            return 1;
         } else if (a.update_time < b.update_time) {
             return 1;
         } else {
@@ -161,16 +165,20 @@ const isMultiviewPanel = computed(() => !props.inModal && props.multiple);
 
 function isActiveItem(history: HistorySummary) {
     if (isMultiviewPanel.value) {
-        return pinnedHistories.value.some((item: PinnedHistory) => item.id == history.id);
+        return isPinned(history.id);
     } else {
         return props.selectedHistories.some((item: PinnedHistory) => item.id == history.id);
     }
 }
 
+function isPinned(historyId: string) {
+    return pinnedHistories.value.some((item: PinnedHistory) => item.id == historyId);
+}
+
 function historyClicked(history: HistorySummary) {
     emit("selectHistory", history);
     if (isMultiviewPanel.value) {
-        if (pinnedHistories.value.some((item: PinnedHistory) => item.id == history.id)) {
+        if (isPinned(history.id)) {
             historyStore.unpinHistories([history.id]);
         } else {
             openInMulti(history);
@@ -263,18 +271,14 @@ async function loadMore(noScroll = false) {
                                         <i v-if="history.id === currentHistoryId">(Current)</i>
                                     </Heading>
                                     <i
-                                        v-if="props.multiple && pinnedHistories.some((h) => h.id === history.id)"
-                                        v-b-tooltip.noninteractive.hover
+                                        v-if="props.multiple && isPinned(history.id)"
                                         title="This history is currently pinned in the multi-history view">
                                         (currently pinned)
                                     </i>
                                 </div>
                                 <TextSummary v-else component="h4" :description="history.name" one-line-summary />
                                 <div class="d-flex align-items-center flex-gapx-1">
-                                    <BBadge
-                                        v-b-tooltip.noninteractive.hover
-                                        pill
-                                        :title="localize('Amount of items in history')">
+                                    <BBadge pill :title="localize('Amount of items in history')">
                                         {{ history.count }} {{ localize("items") }}
                                     </BBadge>
                                     <BBadge

lib/galaxy/tool_util/lint.py

@@ -212,11 +212,19 @@ def found_errors(self) -> bool:
     def found_warns(self) -> bool:
         return len(self.warn_messages) > 0
 
-    def lint(self, name: str, lint_func: Callable[[LintTargetType, "LintContext"], None], lint_target: LintTargetType):
+    def lint(
+        self,
+        name: str,
+        lint_func: Callable[[LintTargetType, "LintContext"], None],
+        lint_target: LintTargetType,
+        module_name: Optional[str] = None,
+    ):
         if name.startswith("lint_"):
             name = name[len("lint_") :]
         if name in self.skip_types:
             return
+        if module_name and module_name in self.skip_types:
+            return
 
         if self.level < LintLevel.SILENT:
             # this is a relict from the past where the lint context
@@ -355,6 +363,7 @@ def lint_tool_source_with_modules(lint_context: LintContext, tool_source, linter
     tool_type = tool_source.parse_tool_type() or "default"
 
     for module in linter_modules:
+        module_name = module.__name__.split(".")[-1]
         lint_tool_types = getattr(module, "lint_tool_types", ["default", "manage_data"])
         if not ("*" in lint_tool_types or tool_type in lint_tool_types):
             continue
@@ -374,7 +383,7 @@ def lint_tool_source_with_modules(lint_context: LintContext, tool_source, linter
                 else:
                     lint_context.lint(name, value, tool_source)
             elif inspect.isclass(value) and issubclass(value, Linter) and not inspect.isabstract(value):
-                lint_context.lint(name, value.lint, tool_source)
+                lint_context.lint(name, value.lint, tool_source, module_name=module_name)
     return lint_context
 
 

lib/galaxy/tool_util/linters/outputs.py → lib/galaxy/tool_util/linters/output.py

@@ -86,7 +86,7 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
         for output in tool_xml.findall("./outputs/data") + tool_xml.findall("./outputs/collection"):
             name = output.attrib.get("name", "")
             label = output.attrib.get("label", "${tool.name} on ${on_string}")
-            if label in labels and output.find(".//filter") is not None:
+            if label in labels and output.find("./filter") is not None:
                 lint_ctx.warn(
                     f"Tool output [{name}] uses duplicated label '{label}', double check if filters imply disjoint cases",
                     linter=cls.name(),
@@ -105,7 +105,7 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
         for output in tool_xml.findall("./outputs/data[@name]") + tool_xml.findall("./outputs/collection[@name]"):
             name = output.attrib.get("name", "")
             label = output.attrib.get("label", "${tool.name} on ${on_string}")
-            if label in labels and output.find(".//filter") is None:
+            if label in labels and output.find("./filter") is None:
                 lint_ctx.warn(f"Tool output [{name}] uses duplicated label '{label}'", linter=cls.name(), node=output)
             labels.add(label)
 

lib/galaxy/tool_util/linters/tests.py

@@ -144,11 +144,11 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
         for test_idx, test in enumerate(tests, start=1):
             # check if expect_num_outputs is set if there are outputs with filters
             # (except for tests with expect_failure .. which can't have test outputs)
-            filter = tool_xml.find("./outputs//filter")
+            has_no_filter = (
+                tool_xml.find("./outputs/data/filter") is None and tool_xml.find("./outputs/collection/filter") is None
+            )
             if not (
-                filter is None
-                or "expect_num_outputs" in test.attrib
-                or asbool(test.attrib.get("expect_failure", False))
+                has_no_filter or "expect_num_outputs" in test.attrib or asbool(test.attrib.get("expect_failure", False))
             ):
                 lint_ctx.warn(
                     f"Test {test_idx}: should specify 'expect_num_outputs' if outputs have filters",

test/unit/tool_util/test_tool_linters.py

@@ -1,8 +1,10 @@
+import inspect
 import os
 import tempfile
 
 import pytest
 
+import galaxy.tool_util.linters
 from galaxy.tool_util.lint import (
     lint_tool_source_with,
     lint_tool_source_with_modules,
@@ -18,7 +20,7 @@
     general,
     help,
     inputs,
-    outputs,
+    output,
     stdio,
     tests,
     xml_order,
@@ -29,6 +31,7 @@
 from galaxy.util import (
     ElementTree,
     parse_xml,
+    submodules,
 )
 from galaxy.util.unittest_utils import skip_if_site_down
 from galaxy.util.xml_macros import load_with_references
@@ -1555,7 +1558,7 @@ def test_inputs_repeats(lint_ctx):
 
 def test_outputs_missing(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_MISSING)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "Tool contains no outputs section, most tools should produce outputs." in lint_ctx.warn_messages
     assert not lint_ctx.info_messages
     assert not lint_ctx.valid_messages
@@ -1565,7 +1568,7 @@ def test_outputs_missing(lint_ctx):
 
 def test_outputs_multiple(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_MULTIPLE)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "0 outputs found." in lint_ctx.info_messages
     assert "Invalid XML: Element 'outputs': This element is not expected." in lint_ctx.error_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1582,7 +1585,7 @@ def test_outputs_unknown_tag(lint_ctx):
     probably be avoided.
     """
     tool_source = get_xml_tool_source(OUTPUTS_UNKNOWN_TAG)
-    lint_tool_source_with_modules(lint_ctx, tool_source, [outputs, xsd])
+    lint_tool_source_with_modules(lint_ctx, tool_source, [output, xsd])
     assert "0 outputs found." in lint_ctx.info_messages
     assert "Avoid the use of 'output' and replace by 'data' or 'collection'" in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1593,7 +1596,7 @@ def test_outputs_unknown_tag(lint_ctx):
 
 def test_outputs_unnamed_invalid_name(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_UNNAMED_INVALID_NAME)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "3 outputs found." in lint_ctx.info_messages
     assert "Invalid XML: Element 'data': The attribute 'name' is required but missing." in lint_ctx.error_messages
     assert "Invalid XML: Element 'collection': The attribute 'name' is required but missing." in lint_ctx.error_messages
@@ -1609,7 +1612,7 @@ def test_outputs_unnamed_invalid_name(lint_ctx):
 
 def test_outputs_format_input_legacy(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT_LEGACY)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "1 outputs found." in lint_ctx.info_messages
     assert "Using format='input' on data is deprecated. Use the format_source attribute." in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1620,7 +1623,7 @@ def test_outputs_format_input_legacy(lint_ctx):
 
 def test_outputs_format_input(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "1 outputs found." in lint_ctx.info_messages
     assert "Using format='input' on data is deprecated. Use the format_source attribute." in lint_ctx.error_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1631,7 +1634,7 @@ def test_outputs_format_input(lint_ctx):
 
 def test_outputs_collection_format_source(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_COLLECTION_FORMAT_SOURCE)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "Tool data output 'reverse' should use either format_source or format/ext" in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
@@ -1641,7 +1644,7 @@ def test_outputs_collection_format_source(lint_ctx):
 
 def test_outputs_format_action(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_FORMAT_ACTION)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
     assert not lint_ctx.warn_messages
@@ -1650,7 +1653,7 @@ def test_outputs_format_action(lint_ctx):
 
 def test_outputs_discover_tool_provided_metadata(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_DISCOVER_TOOL_PROVIDED_METADATA)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "1 outputs found." in lint_ctx.info_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
@@ -1661,7 +1664,7 @@ def test_outputs_discover_tool_provided_metadata(lint_ctx):
 def test_outputs_duplicated_name_label(lint_ctx):
     """ """
     tool_source = get_xml_tool_source(OUTPUTS_DUPLICATED_NAME_LABEL)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "4 outputs found." in lint_ctx.info_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
@@ -2122,6 +2125,30 @@ def test_xml_comments_are_ignored(lint_ctx: LintContext):
         assert "Comment" not in lint_message.message
 
 
+def test_skip_by_name(lint_ctx):
+    # add a linter class name to the skip list
+    lint_ctx.skip_types = ["CitationsMissing"]
+
+    tool_source = get_xml_tool_source(CITATIONS_ABSENT)
+    run_lint_module(lint_ctx, citations, tool_source)
+    assert not lint_ctx.warn_messages
+    assert not lint_ctx.info_messages
+    assert not lint_ctx.valid_messages
+    assert not lint_ctx.error_messages
+
+
+def test_skip_by_module(lint_ctx):
+    # add a module name to the skip list -> all linters in this module are skipped
+    lint_ctx.skip_types = ["citations"]
+
+    tool_source = get_xml_tool_source(CITATIONS_ABSENT)
+    run_lint_module(lint_ctx, citations, tool_source)
+    assert not lint_ctx.warn_messages
+    assert not lint_ctx.info_messages
+    assert not lint_ctx.valid_messages
+    assert not lint_ctx.error_messages
+
+
 def test_list_linters():
     linter_names = Linter.list_listers()
     # make sure to add/remove a test for new/removed linters if this number changes
@@ -2142,3 +2169,45 @@ def test_list_linters():
         "XSD",
     ]:
         assert len([x for x in linter_names if x.startswith(prefix)])
+
+
+def test_linter_module_list():
+    linter_modules = submodules.import_submodules(galaxy.tool_util.linters)
+    linter_module_names = [m.__name__.split(".")[-1] for m in linter_modules]
+    linter_module_names = [n for n in linter_module_names if not n.startswith("_")]
+    assert len(linter_module_names) >= 11
+
+    # until 23.2 the linters were implemented as functions lint_xyz contained in a module named xyz
+    # with 24.0 the functions were split in multiple classes (1 per linter message)
+    # in order to keep the skip functionality of lint contexts working (which is used eg in planemo)
+    # with the old names, we now also check for module name if a linter should be skipped
+    # therefore we test here that the module names are not changed
+    # the keys of the following dict represent the linter names before the switch and the values give
+    # the number of linter classes when we switched
+    # so adding new functionality to a linter module is fine. But splitting one or removing functionality
+    # should raise an error here to point to the possible consequences of renaming
+    old_linters = {
+        "citations": 4,
+        "command": 5,
+        "cwl": 9,
+        "general": 17,
+        "help": 6,
+        "inputs": 52,
+        "output": 11,
+        "stdio": 3,
+        "tests": 21,
+        "xml_order": 1,
+    }
+    assert len(set(linter_module_names).intersection(set(old_linters.keys()))) == len(old_linters.keys())
+
+    for module in linter_modules:
+        module_name = module.__name__.split(".")[-1]
+        if module_name not in old_linters:
+            continue
+        linter_cnt = 0
+        for name, value in inspect.getmembers(module):
+            if callable(value) and name.startswith("lint_"):
+                continue
+            elif inspect.isclass(value) and issubclass(value, Linter) and not inspect.isabstract(value):
+                linter_cnt += 1
+        assert linter_cnt >= old_linters[module_name]