diff --git a/colcon_mixin/mixin/mixin_argument.py b/colcon_mixin/mixin/mixin_argument.py index 6425d75..4d41530 100644 --- a/colcon_mixin/mixin/mixin_argument.py +++ b/colcon_mixin/mixin/mixin_argument.py @@ -181,12 +181,19 @@ def collect_parsers_by_verb(root, parsers, parent_verbs=()): # update args based on selected mixins if 'mixin_verb' in args: mixins = mixins_by_verb.get(args.mixin_verb, {}) + # validate the requested mixins in the order given on the command + # line so an error reports the first unavailable mixin for mixin in args.mixin or (): if mixin not in mixins: context = '.'.join(args.mixin_verb) self._parser.error( "Mixin '{mixin}' is not available for '{context}'" .format_map(locals())) + # apply the mixins in reverse order: since _update_args() handles + # each mixin by prepending its list values, iterating in reverse + # makes the resulting list follow the order the mixins were given + # on the command line while keeping any explicit command line + for mixin in reversed(args.mixin or ()): mixin_args = mixins[mixin] logger.debug( "Using mixin '{mixin}': {mixin_args}".format_map(locals())) diff --git a/test/spell_check.words b/test/spell_check.words index 6ee44ca..9d7986f 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -4,6 +4,8 @@ argparse basenames basepath blocklist +capsys +cmake colcon completers defaultdict @@ -17,6 +19,7 @@ plugin prepending pydocstyle pytest +readouterr rtype scspell setuptools diff --git a/test/test_mixin_argument.py b/test/test_mixin_argument.py new file mode 100644 index 0000000..6118343 --- /dev/null +++ b/test/test_mixin_argument.py @@ -0,0 +1,90 @@ +# Copyright 2026 Open Source Robotics Foundation, Inc. +# Licensed under the Apache License, Version 2.0 + +import argparse +from unittest.mock import patch + +from colcon_mixin.mixin.mixin_argument import MixinArgumentDecorator +import pytest + +MIXINS = { + 'a': {'cmake-args': ['FOO=A']}, + 'b': {'cmake-args': ['FOO=B']}, + 'c': {'cmake-args': ['FOO=C']}, +} + + +SCALAR_MIXINS = { + 'a': {'build-base': 'BASE_A'}, + 'b': {'build-base': 'BASE_B'}, +} + + +def _parse(argv, mixins=MIXINS): + base = argparse.ArgumentParser(prog='colcon') + decorator = MixinArgumentDecorator(base) + subparsers = decorator.add_subparsers(dest='verb') + build = subparsers.add_parser('build') + build.add_argument('--cmake-args', nargs='*', default=[]) + build.add_argument('--build-base', default='build') + mixins_by_verb = {('build',): mixins} + with patch( + 'colcon_mixin.mixin.mixin_argument.get_mixins', + return_value=mixins_by_verb, + ): + return decorator.parse_args(argv) + + +def test_multiple_mixins_preserve_command_line_order(): + # order the mixins were given on the command line + args = _parse(['build', '--mixin', 'a', 'b']) + assert args.cmake_args == ['FOO=A', 'FOO=B'] + + +def test_multiple_mixins_follow_reversed_selection(): + args = _parse(['build', '--mixin', 'b', 'a']) + assert args.cmake_args == ['FOO=B', 'FOO=A'] + + +def test_three_mixins_preserve_order(): + args = _parse(['build', '--mixin', 'a', 'b', 'c']) + assert args.cmake_args == ['FOO=A', 'FOO=B', 'FOO=C'] + + +def test_command_line_arguments_take_precedence(): + # explicit command line arguments must come last so they win + args = _parse(['build', '--mixin', 'a', 'b', '--cmake-args=FOO=CLI']) + assert args.cmake_args == ['FOO=A', 'FOO=B', 'FOO=CLI'] + + +def test_single_mixin(): + args = _parse(['build', '--mixin', 'a']) + assert args.cmake_args == ['FOO=A'] + + +def test_no_mixin(): + args = _parse(['build']) + assert args.cmake_args == [] + + +def test_scalar_last_listed_mixin_wins(): + # a scalar value from a later mixin must override an earlier one + args = _parse(['build', '--mixin', 'a', 'b'], SCALAR_MIXINS) + assert args.build_base == 'BASE_B' + + +def test_scalar_reversed_selection(): + args = _parse(['build', '--mixin', 'b', 'a'], SCALAR_MIXINS) + assert args.build_base == 'BASE_A' + + +def test_scalar_command_line_argument_take_precedence(): + args = _parse( + ['build', '--mixin', 'a', 'b', '--build-base', 'CLI'], SCALAR_MIXINS) + assert args.build_base == 'CLI' + + +def test_unavailable_mixin_reports_first(capsys): + with pytest.raises(SystemExit): + _parse(['build', '--mixin', 'bad', 'a']) + assert "Mixin 'bad' is not available" in capsys.readouterr().err