fix: close if-block before starting a new one in sort_vinca_lists

vinca/sort_vinca_lists.py

@@ -97,9 +97,21 @@ def sort_vinca_lists(path: Path) -> bool:
 
  # Start of conditional block: " - if: ..."
  if RE_IF_BLOCK.match(line):
- if current_if_block is None:
- current_if_block = []
- current_if_block.append(line)
+ # If a previous if-block is already open (no blank line or
+ # comment separated it from this one), close it first.
+ # Otherwise this new block's "then:" would be treated as a
+ # continuation of the previous one's, and sorting would
+ # pool both blocks' items together and redistribute them
+ # across the block boundary.
+ if current_if_block is not None and any(
+ RE_IF_BLOCK.match(bl) for bl in current_if_block
+ ):
+ if_blocks.append(current_if_block)
+ current_if_block = [line]
+ else:
+ if current_if_block is None:
+ current_if_block = []
+ current_if_block.append(line)
  i += 1
  continue
 

vinca/test_sort_vinca_lists.py

@@ -0,0 +1,98 @@
+"""Tests for the vinca.yaml list sorter."""
+
+from vinca.sort_vinca_lists import sort_vinca_lists
+
+BASE = """packages_select_by_deps:
+ - alpha
+ - charlie
+ - bravo
+"""
+
+
+def test_sorts_simple_top_level_items(tmp_path):
+ path = tmp_path / "vinca.yaml"
+ path.write_text(BASE)
+
+ changed = sort_vinca_lists(path)
+
+ assert changed is True
+ items = [
+ line.strip()[2:]
+ for line in path.read_text().splitlines()
+ if line.startswith(" - ")
+ ]
+ assert items == ["alpha", "bravo", "charlie"]
+
+
+def test_leaves_already_sorted_file_unchanged(tmp_path):
+ path = tmp_path / "vinca.yaml"
+ sorted_content = "packages_select_by_deps:\n - alpha\n - bravo\n - charlie\n\n"
+ path.write_text(sorted_content)
+
+ changed = sort_vinca_lists(path)
+
+ assert changed is False
+ assert path.read_text() == sorted_content
+
+
+def test_adjacent_if_blocks_without_separator_stay_isolated(tmp_path):
+ # Regression test: two "- if:" blocks back-to-back with no blank line or
+ # comment between them used to be parsed as a single block, so sorting
+ # pooled both blocks' then-items together and could redistribute an item
+ # from one block's platform condition into the other's.
+ content = """packages_select_by_deps:
+ - if: not wasm32 and not win
+ then:
+ - web_video_server
+ - webots_ros2
+ - yasmin
+ - yasmin_ros
+ - if: linux and not aarch64
+ then:
+ - somepkg
+ - zed_msgs
+
+patch_dir: patch
+"""
+ path = tmp_path / "vinca.yaml"
+ path.write_text(content)
+
+ sort_vinca_lists(path)
+
+ result = path.read_text()
+ first_block, second_block = result.split("- if: not wasm32 and not win")[1].split(
+ "- if: linux and not aarch64"
+ )
+
+ assert "webots_ros2" in first_block
+ assert "yasmin_ros" in first_block
+ assert "webots_ros2" not in second_block
+ assert "somepkg" in second_block
+ assert "zed_msgs" in second_block
+
+
+def test_adjacent_if_blocks_separated_by_comment_stay_isolated(tmp_path):
+ # The comment-separator workaround must keep working alongside the fix.
+ content = """packages_select_by_deps:
+ - if: not wasm32 and not win
+ then:
+ - web_video_server
+ - yasmin
+ - yasmin_ros
+
+ # webots_ros2 is linux-only
+ - if: linux and not aarch64
+ then:
+ - webots_ros2
+ - zed_msgs
+
+patch_dir: patch
+"""
+ path = tmp_path / "vinca.yaml"
+ path.write_text(content)
+
+ sort_vinca_lists(path)
+
+ first_block, second_block = path.read_text().split("- if: linux and not aarch64")
+ assert "- webots_ros2" not in first_block
+ assert "- webots_ros2" in second_block