Fix mixin arg merge order for lists and scalars (fixes #40)

[PiousCrossten]

Fixes #40

When using --mixin a b, two bugs existed in how the selected mixins
were applied in _update_args:

• Lists were concatenated in reverse of the given order (b's values
  appeared before a's). Each mixin prepends its list values so that
  explicit command line arguments stay last and win — but the mixins
  were iterated in forward order, so each prepend stacked the previous
  one further back, producing [b, a, CLI] instead of [a, b, CLI].

• Scalars from a later mixin were silently skipped.** Once the first
  mixin set a scalar it was no longer a default, so subsequent mixins
  couldn't override it.

Fix: Split the single apply loop into two:

1. Validate the requested mixins in the given order, so an error
   still reports the first unavailable mixin.
2. Apply the mixins in reverse with
   for mixin in reversed(args.mixin or ()):. Because _update_args
   prepends each mixin's list values, reverse iteration unwinds them
   back into the order the mixins were given on the command line, while
   any explicit command line arguments remain last and win.

This is a minimal change — one new executable line plus comments — and
fixes both bugs at once:

• Lists: --mixin a b → [A, B, CLI] (CLI still wins).
• Scalars: the last-listed mixin wins; an explicit CLI value still
  overrides all.

Added 10 regression tests (test/test_mixin_argument.py) covering list
ordering, reversed selection, three-mixin order, scalar override,
command line precedence, and the first-unavailable-mixin error.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.37%. Comparing base (ebedd27) to head (19cc725).

Additional details and impacted files

@@             Coverage Diff             @@
##           master      #59       +/-   ##
===========================================
+ Coverage   15.30%   37.37%   +22.07%     
===========================================
  Files          11       12        +1     
  Lines         562      602       +40     
  Branches       94      102        +8     
===========================================
+ Hits           86      225      +139     
+ Misses        474      365      -109     
- Partials        2       12       +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[knmcguire]

I also did a test locally where I ran the tests made on the current master and this particular branch:

With PR 46 all 10 tests passes (as expected)

test/test_mixin_argument.py::test_multiple_mixins_preserve_command_line_order PASSED [ 10%]
test/test_mixin_argument.py::test_multiple_mixins_follow_reversed_selection PASSED [ 20%]
test/test_mixin_argument.py::test_three_mixins_preserve_order PASSED     [ 30%]
test/test_mixin_argument.py::test_command_line_arguments_take_precedence PASSED [ 40%]
test/test_mixin_argument.py::test_single_mixin PASSED                    [ 50%]
test/test_mixin_argument.py::test_no_mixin PASSED                        [ 60%]
test/test_mixin_argument.py::test_scalar_last_listed_mixin_wins PASSED   [ 70%]
test/test_mixin_argument.py::test_scalar_reversed_selection PASSED       [ 80%]
test/test_mixin_argument.py::test_scalar_command_line_argument_take_precedence PASSED [ 90%]
test/test_mixin_argument.py::test_unavailable_mixin_reports_first PASSED [100%]
============================= 10 passed in 0.12s ==============================

On the latest master with ONLY the test_mixin_arguments.py copied

test/test_mixin_argument.py::test_multiple_mixins_preserve_command_line_order FAILED [ 10%]
test/test_mixin_argument.py::test_multiple_mixins_follow_reversed_selection FAILED [ 20%]
test/test_mixin_argument.py::test_three_mixins_preserve_order FAILED     [ 30%]
test/test_mixin_argument.py::test_command_line_arguments_take_precedence FAILED [ 40%]
test/test_mixin_argument.py::test_single_mixin PASSED                    [ 50%]
test/test_mixin_argument.py::test_no_mixin PASSED                        [ 60%]
test/test_mixin_argument.py::test_scalar_last_listed_mixin_wins FAILED   [ 70%]
test/test_mixin_argument.py::test_scalar_reversed_selection FAILED       [ 80%]
test/test_mixin_argument.py::test_scalar_command_line_argument_take_precedence PASSED [ 90%]
test/test_mixin_argument.py::test_unavailable_mixin_reports_first PASSED [100%]
E       AssertionError: assert ['FOO=B', 'FOO=A'] == ['FOO=A', 'FOO=B']
test\test_mixin_argument.py:43: AssertionError
E       AssertionError: assert ['FOO=A', 'FOO=B'] == ['FOO=B', 'FOO=A']
test\test_mixin_argument.py:48: AssertionError
E       AssertionError: assert ['FOO=C', 'FOO=B', 'FOO=A'] == ['FOO=A', 'FOO=B', 'FOO=C']
test\test_mixin_argument.py:53: AssertionError
E       AssertionError: assert ['FOO=B', 'FOO=A', 'FOO=CLI'] == ['FOO=A', 'FOO=B', 'FOO=CLI']
test\test_mixin_argument.py:59: AssertionError
E       AssertionError: assert 'BASE_A' == 'BASE_B'
test\test_mixin_argument.py:75: AssertionError
colcon.colcon_mixin.mixin.mixin_argument DEBUG Skipping mixin key 'build-base' which was passed explicitly as a command line 
argument
DEBUG    colcon.colcon_mixin.mixin.mixin_argument:mixin_argument.py:278 Skipping mixin key 'build-base' which was passed 
explicitly as a command line argument
E       AssertionError: assert 'BASE_B' == 'BASE_A'
test\test_mixin_argument.py:80: AssertionError
colcon.colcon_mixin.mixin.mixin_argument DEBUG Skipping mixin key 'build-base' which was passed explicitly as a command line 
argument
DEBUG    colcon.colcon_mixin.mixin.mixin_argument:mixin_argument.py:278 Skipping mixin key 'build-base' which was passed 
explicitly as a command line argument
FAILED test/test_mixin_argument.py::test_multiple_mixins_preserve_command_line_order
FAILED test/test_mixin_argument.py::test_multiple_mixins_follow_reversed_selection
FAILED test/test_mixin_argument.py::test_three_mixins_preserve_order - Assert...
FAILED test/test_mixin_argument.py::test_command_line_arguments_take_precedence
FAILED test/test_mixin_argument.py::test_scalar_last_listed_mixin_wins - Asse...
FAILED test/test_mixin_argument.py::test_scalar_reversed_selection - Assertio...
========================= 6 failed, 4 passed in 0.25s =========================

So 4 tests already passed with the current master. test_single_mixin, test_no_mixin, test_unavailable_mixin_reports_first I would expect to pass, but interestingly test_scalar_command_line_argument_take_precedence() seems to pass on current master as well. So if there is a cmake argument, this means that for some reason.... the order of mixin A and B is in the right order?

I'm not sure if this is relevant for this PR though!

[cottsay]

Thanks for the PR. I'm on board here.

It should be noted that while this PR does appear to align the behavior with official documentation, it does change the existing behavior for both scalar and list arguments when multiple mixins change the same values. For that reason, we should probably make some noise about this change and possibly announce it on ROS Discourse.