From b02dd2233294f37789cd45993124c421f1ee22d2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 23:56:40 +0000 Subject: [PATCH 01/19] Initial plan From 7f29321538359c1a31c5321b2396f7b29c8b455e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 00:05:35 +0000 Subject: [PATCH 02/19] Add InsertParallel instruction handling and test cases Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- .../_integrations/ci/bitbucket/pipeweld.py | 78 ++++++++++- .../ci/bitbucket/test_pipeweld.py | 122 ++++++++++++++++++ 2 files changed, 197 insertions(+), 3 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index ec5f9e6b..80d53d6d 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -15,6 +15,7 @@ from usethis._integrations.ci.bitbucket.schema import ( ImportPipeline, Items, + Parallel, ParallelExpanded, ParallelItem, ParallelSteps, @@ -25,7 +26,7 @@ ) from usethis._integrations.ci.bitbucket.schema_utils import step1tostep from usethis._integrations.file.yaml.update import update_ruamel_yaml_map -from usethis._pipeweld.ops import Instruction +from usethis._pipeweld.ops import InsertParallel, InsertSuccessor, Instruction if TYPE_CHECKING: from usethis._integrations.ci.bitbucket.io_ import ( @@ -151,21 +152,92 @@ def apply_pipeweld_instruction_via_doc( items = default.root.root if instruction.after is None: + # Insert at the beginning - always as a simple step items.insert(0, StepItem(step=new_step)) - else: + elif isinstance(instruction, InsertSuccessor): + # Insert in series after the specified step for item in items: if _is_insertion_necessary(item, instruction=instruction): - # N.B. This doesn't currently handle InsertParallel properly items.insert( items.index(item) + 1, StepItem(step=new_step), ) break + elif isinstance(instruction, InsertParallel): + # Insert in parallel with the specified step + for idx, item in enumerate(items): + if _is_insertion_necessary(item, instruction=instruction): + _insert_parallel_step(items, idx, item, new_step) + break if default is None and items: pipelines.default = Pipeline(Items(items)) +@singledispatch +def _insert_parallel_step( + items: list[StepItem | ParallelItem | StageItem], + idx: int, + item: StepItem | ParallelItem | StageItem, + new_step: "Step", +) -> None: + """Insert a step in parallel with an existing item. + + This function handles the logic of converting a single step to a parallel block + or adding to an existing parallel block. + """ + raise NotImplementedError + + +@_insert_parallel_step.register +def _(items, idx, item: StepItem, new_step): + """Convert a single StepItem to a ParallelItem with both steps.""" + # Replace the single step with a parallel block containing both steps + parallel_item = ParallelItem( + parallel=Parallel( + ParallelSteps([ + StepItem(step=item.step), + StepItem(step=new_step), + ]) + ) + ) + items[idx] = parallel_item + + +@_insert_parallel_step.register +def _(items, idx, item: ParallelItem, new_step): + """Add a new step to an existing ParallelItem.""" + if item.parallel is not None: + if isinstance(item.parallel.root, ParallelSteps): + # Add to the existing list of parallel steps + item.parallel.root.root.append(StepItem(step=new_step)) + elif isinstance(item.parallel.root, ParallelExpanded): + # Add to the expanded parallel steps + item.parallel.root.steps.root.append(StepItem(step=new_step)) + else: + assert_never(item.parallel.root) + + +@_insert_parallel_step.register +def _(items, idx, item: StageItem, new_step): + """Insert parallel step after a stage item. + + Since we found the target step within a stage, we can't insert in parallel + with it (stages don't support parallelism within them). Instead, we insert + a new step in parallel with the entire stage. + """ + # Create a parallel block with the stage and the new step + parallel_item = ParallelItem( + parallel=Parallel( + ParallelSteps([ + item, # The entire stage + StepItem(step=new_step), + ]) + ) + ) + items[idx] = parallel_item + + @singledispatch def _is_insertion_necessary( item: StepItem | ParallelItem | StageItem, diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index 6651b417..04d6eebd 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -304,6 +304,128 @@ def test_stage_item(self, tmp_path: Path): """ ) + class TestInsertParallel: + def test_simple_step(self, tmp_path: Path): + # Arrange: Pipeline with a single step + (tmp_path / "bitbucket-pipelines.yml").write_text( + """\ +image: atlassian/default-image:3 +pipelines: + default: + - step: + name: bar + script: + - echo bar +""" + ) + + # Act: Insert foo in parallel to bar + with change_cwd(tmp_path): + apply_pipeweld_instruction( + InsertParallel(step="foo", after="bar"), + new_step=Step(name="foo", script=Script(["echo foo"])), + ) + + # Assert: Should create a parallel block with both steps + content = (tmp_path / "bitbucket-pipelines.yml").read_text() + assert ( + content + == """\ +image: atlassian/default-image:3 +pipelines: + default: + - parallel: + - step: + name: bar + script: + - echo bar + - step: + name: foo + script: + - echo foo +""" + ) + + def test_add_to_existing_parallel(self, tmp_path: Path): + # Arrange: Pipeline with an existing parallel block + (tmp_path / "bitbucket-pipelines.yml").write_text( + """\ +image: atlassian/default-image:3 +pipelines: + default: + - parallel: + - step: + name: bar + script: + - echo bar + - step: + name: baz + script: + - echo baz +""" + ) + + # Act: Insert foo in parallel to bar (which is already in a parallel block) + with change_cwd(tmp_path): + apply_pipeweld_instruction( + InsertParallel(step="foo", after="bar"), + new_step=Step(name="foo", script=Script(["echo foo"])), + ) + + # Assert: Should add to the existing parallel block + content = (tmp_path / "bitbucket-pipelines.yml").read_text() + assert ( + content + == """\ +image: atlassian/default-image:3 +pipelines: + default: + - parallel: + - step: + name: bar + script: + - echo bar + - step: + name: baz + script: + - echo baz + - step: + name: foo + script: + - echo foo +""" + ) + + def test_parallel_after_none(self, tmp_path: Path): + # Arrange: Empty pipeline + (tmp_path / "bitbucket-pipelines.yml").write_text( + """\ +image: atlassian/default-image:3 +""" + ) + + # Act: Insert foo in parallel at the beginning (after=None) + with change_cwd(tmp_path): + apply_pipeweld_instruction( + InsertParallel(step="foo", after=None), + new_step=Step(name="foo", script=Script(["echo foo"])), + ) + + # Assert: Should just add the step (parallel with no other steps is just a step) + content = (tmp_path / "bitbucket-pipelines.yml").read_text() + assert ( + content + == """\ +image: atlassian/default-image:3 +pipelines: + default: + - step: + name: foo + script: + - echo foo +""" + ) + class TestGetInstructionsForInsertion: class TestStr: From 42dacbf021b5a665896c739990949053d63e700a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 00:07:43 +0000 Subject: [PATCH 03/19] Fix singledispatch signature for _insert_parallel_step Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- src/usethis/_integrations/ci/bitbucket/pipeweld.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index 80d53d6d..7aec7864 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -167,7 +167,7 @@ def apply_pipeweld_instruction_via_doc( # Insert in parallel with the specified step for idx, item in enumerate(items): if _is_insertion_necessary(item, instruction=instruction): - _insert_parallel_step(items, idx, item, new_step) + _insert_parallel_step(item, items=items, idx=idx, new_step=new_step) break if default is None and items: @@ -176,9 +176,10 @@ def apply_pipeweld_instruction_via_doc( @singledispatch def _insert_parallel_step( + item: StepItem | ParallelItem | StageItem, + *, items: list[StepItem | ParallelItem | StageItem], idx: int, - item: StepItem | ParallelItem | StageItem, new_step: "Step", ) -> None: """Insert a step in parallel with an existing item. @@ -190,7 +191,7 @@ def _insert_parallel_step( @_insert_parallel_step.register -def _(items, idx, item: StepItem, new_step): +def _(item: StepItem, *, items, idx, new_step): """Convert a single StepItem to a ParallelItem with both steps.""" # Replace the single step with a parallel block containing both steps parallel_item = ParallelItem( @@ -205,7 +206,7 @@ def _(items, idx, item: StepItem, new_step): @_insert_parallel_step.register -def _(items, idx, item: ParallelItem, new_step): +def _(item: ParallelItem, *, items, idx, new_step): """Add a new step to an existing ParallelItem.""" if item.parallel is not None: if isinstance(item.parallel.root, ParallelSteps): @@ -219,7 +220,7 @@ def _(items, idx, item: ParallelItem, new_step): @_insert_parallel_step.register -def _(items, idx, item: StageItem, new_step): +def _(item: StageItem, *, items, idx, new_step): """Insert parallel step after a stage item. Since we found the target step within a stage, we can't insert in parallel From 81bb3afff225ebf0058b42faacf2549979d8be52 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 00:12:26 +0000 Subject: [PATCH 04/19] Add support for breaking up parallelism by moving existing steps Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- .../_integrations/ci/bitbucket/pipeweld.py | 101 ++++++++++++++-- .../ci/bitbucket/test_pipeweld.py | 111 ++++++++++++++++++ 2 files changed, 204 insertions(+), 8 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index 7aec7864..8ec3d420 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -130,10 +130,6 @@ def apply_pipeweld_instruction_via_doc( new_step: Step, doc: BitbucketPipelinesYAMLDocument, ) -> None: - if get_pipeweld_step(new_step) != instruction.step: - # N.B. This doesn't currently handle moving existing steps - return - if doc.model.pipelines is None: doc.model.pipelines = Pipelines() @@ -144,36 +140,125 @@ def apply_pipeweld_instruction_via_doc( items = [] elif isinstance(default.root, ImportPipeline): msg = ( - f"Cannot add step '{new_step.name}' to default pipeline in " + f"Cannot add step to default pipeline in " f"'bitbucket-pipelines.yml' because it is an import pipeline." ) raise UnexpectedImportPipelineError(msg) else: items = default.root.root + # Check if this instruction is for a new step or an existing step + is_new_step = get_pipeweld_step(new_step) == instruction.step + + if is_new_step: + step_to_insert = new_step + else: + # This is an instruction to move an existing step + # Find and extract the step from the pipeline + extracted_step = _extract_step_from_items(items, instruction.step) + if extracted_step is None: + # Step not found in pipeline, skip this instruction + return + step_to_insert = extracted_step + if instruction.after is None: # Insert at the beginning - always as a simple step - items.insert(0, StepItem(step=new_step)) + items.insert(0, StepItem(step=step_to_insert)) elif isinstance(instruction, InsertSuccessor): # Insert in series after the specified step for item in items: if _is_insertion_necessary(item, instruction=instruction): items.insert( items.index(item) + 1, - StepItem(step=new_step), + StepItem(step=step_to_insert), ) break elif isinstance(instruction, InsertParallel): # Insert in parallel with the specified step for idx, item in enumerate(items): if _is_insertion_necessary(item, instruction=instruction): - _insert_parallel_step(item, items=items, idx=idx, new_step=new_step) + _insert_parallel_step(item, items=items, idx=idx, new_step=step_to_insert) break if default is None and items: pipelines.default = Pipeline(Items(items)) +def _extract_step_from_items( + items: list[StepItem | ParallelItem | StageItem], step_name: str +) -> "Step | None": + """Find and remove a step from the items list. + + This function searches for a step with the given name, removes it from the + items list, and returns the step. If the step is found in a parallel block + with other steps, only that step is removed from the parallel block. + """ + for idx, item in enumerate(items): + extracted = _extract_step_from_item(item, step_name, items, idx) + if extracted is not None: + return extracted + return None + + +@singledispatch +def _extract_step_from_item( + item: StepItem | ParallelItem | StageItem, + step_name: str, + items: list[StepItem | ParallelItem | StageItem], + idx: int, +) -> "Step | None": + """Extract a step from an item, potentially modifying the items list.""" + raise NotImplementedError + + +@_extract_step_from_item.register +def _(item: StepItem, step_name, items, idx): + """Extract from a StepItem.""" + if get_pipeweld_step(item.step) == step_name: + # Remove this item from the list + items.pop(idx) + return item.step + return None + + +@_extract_step_from_item.register +def _(item: ParallelItem, step_name, items, idx): + """Extract from a ParallelItem.""" + if item.parallel is not None: + if isinstance(item.parallel.root, ParallelSteps): + step_items = item.parallel.root.root + elif isinstance(item.parallel.root, ParallelExpanded): + step_items = item.parallel.root.steps.root + else: + assert_never(item.parallel.root) + + for step_idx, step_item in enumerate(step_items): + if get_pipeweld_step(step_item.step) == step_name: + # Found it - remove from the parallel block + extracted_step = step_item.step + step_items.pop(step_idx) + + # If only one step remains in the parallel, convert to a simple step + if len(step_items) == 1: + items[idx] = step_items[0] + elif len(step_items) == 0: + # No steps left, remove the parallel item + items.pop(idx) + + return extracted_step + return None + + +@_extract_step_from_item.register +def _(item: StageItem, step_name, items, idx): + """Extract from a StageItem. + + We don't extract steps from within stages as they represent deployment + stages and their internal structure should be preserved. + """ + return None + + @singledispatch def _insert_parallel_step( item: StepItem | ParallelItem | StageItem, diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index 04d6eebd..d5cd9ca6 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -427,6 +427,117 @@ def test_parallel_after_none(self, tmp_path: Path): ) +class TestBreakUpParallelism: + """Test breaking up an existing parallel block to satisfy dependencies.""" + + def test_extract_step_from_parallel(self, tmp_path: Path): + # Arrange: Parallel block with two steps + (tmp_path / "bitbucket-pipelines.yml").write_text( + """\ +image: atlassian/default-image:3 +pipelines: + default: + - parallel: + - step: + name: A + script: + - echo A + - step: + name: B + script: + - echo B +""" + ) + + # Act: Move A to the beginning (simulating what pipeweld does) + with change_cwd(tmp_path): + apply_pipeweld_instruction( + InsertSuccessor(step="A", after=None), + new_step=Step(name="C", script=Script(["echo C"])), + ) + + # Assert: A should be extracted and B should remain as a single step + content = (tmp_path / "bitbucket-pipelines.yml").read_text() + assert ( + content + == """\ +image: atlassian/default-image:3 +pipelines: + default: + - step: + name: A + script: + - echo A + - step: + name: B + script: + - echo B +""" + ) + + def test_full_parallel_breakup_sequence(self, tmp_path: Path): + # Arrange: Start with parallel(A, B), want to insert C after A + (tmp_path / "bitbucket-pipelines.yml").write_text( + """\ +image: atlassian/default-image:3 +pipelines: + default: + - parallel: + - step: + name: A + script: + - echo A + - step: + name: B + script: + - echo B +""" + ) + + with change_cwd(tmp_path): + # Simulate pipeweld instructions for: series("A", "C", "B") + # Step 1: Move A to beginning + apply_pipeweld_instruction( + InsertSuccessor(step="A", after=None), + new_step=Step(name="C", script=Script(["echo C"])), + ) + + # Step 2: Insert C after A + apply_pipeweld_instruction( + InsertSuccessor(step="C", after="A"), + new_step=Step(name="C", script=Script(["echo C"])), + ) + + # Step 3: Move B after A + apply_pipeweld_instruction( + InsertSuccessor(step="B", after="A"), + new_step=Step(name="C", script=Script(["echo C"])), + ) + + # Assert: Should result in A, then B, then C in series + content = (tmp_path / "bitbucket-pipelines.yml").read_text() + assert ( + content + == """\ +image: atlassian/default-image:3 +pipelines: + default: + - step: + name: A + script: + - echo A + - step: + name: B + script: + - echo B + - step: + name: C + script: + - echo C +""" + ) + + class TestGetInstructionsForInsertion: class TestStr: def test_after_str(self): From 650021631effac14ff1d0488f7d0a1b79c5884e5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 00:15:51 +0000 Subject: [PATCH 05/19] Fix test to use correct instruction order for A, C, B sequence Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- .../ci/bitbucket/test_pipeweld.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index d5cd9ca6..0d43bf16 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -502,19 +502,19 @@ def test_full_parallel_breakup_sequence(self, tmp_path: Path): new_step=Step(name="C", script=Script(["echo C"])), ) - # Step 2: Insert C after A + # Step 2: Move B after A apply_pipeweld_instruction( - InsertSuccessor(step="C", after="A"), + InsertSuccessor(step="B", after="A"), new_step=Step(name="C", script=Script(["echo C"])), ) - # Step 3: Move B after A + # Step 3: Insert C after A (C will go between A and B) apply_pipeweld_instruction( - InsertSuccessor(step="B", after="A"), + InsertSuccessor(step="C", after="A"), new_step=Step(name="C", script=Script(["echo C"])), ) - # Assert: Should result in A, then B, then C in series + # Assert: Should result in A, then C, then B in series content = (tmp_path / "bitbucket-pipelines.yml").read_text() assert ( content @@ -526,14 +526,14 @@ def test_full_parallel_breakup_sequence(self, tmp_path: Path): name: A script: - echo A - - step: - name: B - script: - - echo B - step: name: C script: - echo C + - step: + name: B + script: + - echo B """ ) From 147b856a55f1ae0ac4b95b6f98c06b9a941ccf5e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 00:17:19 +0000 Subject: [PATCH 06/19] Add clarifying comments to test for instruction/new_step relationship Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index 0d43bf16..83dd533a 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -496,6 +496,10 @@ def test_full_parallel_breakup_sequence(self, tmp_path: Path): with change_cwd(tmp_path): # Simulate pipeweld instructions for: series("A", "C", "B") + # Note: new_step is always C (the actual new step being added) + # but instruction.step varies (A, B, or C) to indicate which step + # the instruction is about (existing or new) + # Step 1: Move A to beginning apply_pipeweld_instruction( InsertSuccessor(step="A", after=None), From e35688efd7e0000e882d10e64259c91fb782046e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 00:39:44 +0000 Subject: [PATCH 07/19] Restore new_step.name in import pipeline error message Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- src/usethis/_integrations/ci/bitbucket/pipeweld.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index 8ec3d420..aab25e29 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -140,7 +140,7 @@ def apply_pipeweld_instruction_via_doc( items = [] elif isinstance(default.root, ImportPipeline): msg = ( - f"Cannot add step to default pipeline in " + f"Cannot add step '{new_step.name}' to default pipeline in " f"'bitbucket-pipelines.yml' because it is an import pipeline." ) raise UnexpectedImportPipelineError(msg) From 6eda12db673bae72ef73c1514a06619f0e2d31b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 00:45:55 +0000 Subject: [PATCH 08/19] Remove test_extract_step_from_parallel test Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- .../ci/bitbucket/test_pipeweld.py | 45 ------------------- 1 file changed, 45 deletions(-) diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index 83dd533a..8d1ebd5e 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -430,51 +430,6 @@ def test_parallel_after_none(self, tmp_path: Path): class TestBreakUpParallelism: """Test breaking up an existing parallel block to satisfy dependencies.""" - def test_extract_step_from_parallel(self, tmp_path: Path): - # Arrange: Parallel block with two steps - (tmp_path / "bitbucket-pipelines.yml").write_text( - """\ -image: atlassian/default-image:3 -pipelines: - default: - - parallel: - - step: - name: A - script: - - echo A - - step: - name: B - script: - - echo B -""" - ) - - # Act: Move A to the beginning (simulating what pipeweld does) - with change_cwd(tmp_path): - apply_pipeweld_instruction( - InsertSuccessor(step="A", after=None), - new_step=Step(name="C", script=Script(["echo C"])), - ) - - # Assert: A should be extracted and B should remain as a single step - content = (tmp_path / "bitbucket-pipelines.yml").read_text() - assert ( - content - == """\ -image: atlassian/default-image:3 -pipelines: - default: - - step: - name: A - script: - - echo A - - step: - name: B - script: - - echo B -""" - ) - def test_full_parallel_breakup_sequence(self, tmp_path: Path): # Arrange: Start with parallel(A, B), want to insert C after A (tmp_path / "bitbucket-pipelines.yml").write_text( From cb6b258cc024779d958be1a3a653ff7742fc1132 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Mon, 27 Oct 2025 14:24:45 +1300 Subject: [PATCH 09/19] Improve type compliance --- .../_integrations/ci/bitbucket/pipeweld.py | 90 ++++++++++--------- .../ci/bitbucket/test_pipeweld.py | 2 +- 2 files changed, 48 insertions(+), 44 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index aab25e29..d4ae1c63 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -22,6 +22,7 @@ Pipeline, Pipelines, StageItem, + Step, StepItem, ) from usethis._integrations.ci.bitbucket.schema_utils import step1tostep @@ -29,13 +30,8 @@ from usethis._pipeweld.ops import InsertParallel, InsertSuccessor, Instruction if TYPE_CHECKING: - from usethis._integrations.ci.bitbucket.io_ import ( - BitbucketPipelinesYAMLDocument, - ) - from usethis._integrations.ci.bitbucket.schema import ( - PipelinesConfiguration, - Step, - ) + from usethis._integrations.ci.bitbucket.io_ import BitbucketPipelinesYAMLDocument + from usethis._integrations.ci.bitbucket.schema import PipelinesConfiguration def get_pipeweld_step(step: Step) -> str: @@ -124,7 +120,7 @@ def apply_pipeweld_instruction(instruction: Instruction, *, new_step: Step) -> N update_ruamel_yaml_map(doc.content, dump, preserve_comments=True) -def apply_pipeweld_instruction_via_doc( +def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 instruction: Instruction, *, new_step: Step, @@ -149,7 +145,7 @@ def apply_pipeweld_instruction_via_doc( # Check if this instruction is for a new step or an existing step is_new_step = get_pipeweld_step(new_step) == instruction.step - + if is_new_step: step_to_insert = new_step else: @@ -177,7 +173,9 @@ def apply_pipeweld_instruction_via_doc( # Insert in parallel with the specified step for idx, item in enumerate(items): if _is_insertion_necessary(item, instruction=instruction): - _insert_parallel_step(item, items=items, idx=idx, new_step=step_to_insert) + _insert_parallel_step( + item, items=items, idx=idx, new_step=step_to_insert + ) break if default is None and items: @@ -186,9 +184,9 @@ def apply_pipeweld_instruction_via_doc( def _extract_step_from_items( items: list[StepItem | ParallelItem | StageItem], step_name: str -) -> "Step | None": +) -> Step | None: """Find and remove a step from the items list. - + This function searches for a step with the given name, removes it from the items list, and returns the step. If the step is found in a parallel block with other steps, only that step is removed from the parallel block. @@ -206,7 +204,7 @@ def _extract_step_from_item( step_name: str, items: list[StepItem | ParallelItem | StageItem], idx: int, -) -> "Step | None": +) -> Step | None: """Extract a step from an item, potentially modifying the items list.""" raise NotImplementedError @@ -231,28 +229,28 @@ def _(item: ParallelItem, step_name, items, idx): step_items = item.parallel.root.steps.root else: assert_never(item.parallel.root) - + for step_idx, step_item in enumerate(step_items): if get_pipeweld_step(step_item.step) == step_name: # Found it - remove from the parallel block extracted_step = step_item.step step_items.pop(step_idx) - + # If only one step remains in the parallel, convert to a simple step if len(step_items) == 1: items[idx] = step_items[0] elif len(step_items) == 0: # No steps left, remove the parallel item items.pop(idx) - + return extracted_step return None @_extract_step_from_item.register -def _(item: StageItem, step_name, items, idx): +def _(item: StageItem): # noqa: ARG001 """Extract from a StageItem. - + We don't extract steps from within stages as they represent deployment stages and their internal structure should be preserved. """ @@ -265,10 +263,10 @@ def _insert_parallel_step( *, items: list[StepItem | ParallelItem | StageItem], idx: int, - new_step: "Step", + new_step: Step, ) -> None: """Insert a step in parallel with an existing item. - + This function handles the logic of converting a single step to a parallel block or adding to an existing parallel block. """ @@ -276,22 +274,34 @@ def _insert_parallel_step( @_insert_parallel_step.register -def _(item: StepItem, *, items, idx, new_step): +def _( + item: StepItem, + *, + items: list[StepItem | ParallelItem | StageItem], + idx: int, + new_step: Step, +) -> None: """Convert a single StepItem to a ParallelItem with both steps.""" # Replace the single step with a parallel block containing both steps parallel_item = ParallelItem( parallel=Parallel( - ParallelSteps([ - StepItem(step=item.step), - StepItem(step=new_step), - ]) + ParallelSteps( + [ + StepItem(step=item.step), + StepItem(step=new_step), + ] + ) ) ) items[idx] = parallel_item @_insert_parallel_step.register -def _(item: ParallelItem, *, items, idx, new_step): +def _( + item: ParallelItem, + *, + new_step: Step, +) -> None: """Add a new step to an existing ParallelItem.""" if item.parallel is not None: if isinstance(item.parallel.root, ParallelSteps): @@ -305,23 +315,17 @@ def _(item: ParallelItem, *, items, idx, new_step): @_insert_parallel_step.register -def _(item: StageItem, *, items, idx, new_step): - """Insert parallel step after a stage item. - - Since we found the target step within a stage, we can't insert in parallel - with it (stages don't support parallelism within them). Instead, we insert - a new step in parallel with the entire stage. - """ - # Create a parallel block with the stage and the new step - parallel_item = ParallelItem( - parallel=Parallel( - ParallelSteps([ - item, # The entire stage - StepItem(step=new_step), - ]) - ) - ) - items[idx] = parallel_item +def _( + item: StageItem, + *, + items: list[StepItem | ParallelItem | StageItem], + idx: int, + new_step: Step, +) -> None: + # StageItems are trickier since they aren't supported in ParallelSteps. But we + # never need to add them in practice anyway. The only reason this is really here + # is for type safety. + raise NotImplementedError @singledispatch diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index 8d1ebd5e..54065f67 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -454,7 +454,7 @@ def test_full_parallel_breakup_sequence(self, tmp_path: Path): # Note: new_step is always C (the actual new step being added) # but instruction.step varies (A, B, or C) to indicate which step # the instruction is about (existing or new) - + # Step 1: Move A to beginning apply_pipeweld_instruction( InsertSuccessor(step="A", after=None), From 228fd3dd20dd830b7ef1a35c9fb8088df41aeef5 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Mon, 27 Oct 2025 15:01:21 +1300 Subject: [PATCH 10/19] Improve type compliance --- .../_integrations/ci/bitbucket/pipeweld.py | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index d4ae1c63..c5bc5bc5 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -210,7 +210,12 @@ def _extract_step_from_item( @_extract_step_from_item.register -def _(item: StepItem, step_name, items, idx): +def _( + item: StepItem, + step_name: str, + items: list[StepItem | ParallelItem | StageItem], + idx: int, +) -> Step | None: """Extract from a StepItem.""" if get_pipeweld_step(item.step) == step_name: # Remove this item from the list @@ -220,7 +225,12 @@ def _(item: StepItem, step_name, items, idx): @_extract_step_from_item.register -def _(item: ParallelItem, step_name, items, idx): +def _( + item: ParallelItem, + step_name: str, + items: list[StepItem | ParallelItem | StageItem], + idx: int, +) -> Step | None: """Extract from a ParallelItem.""" if item.parallel is not None: if isinstance(item.parallel.root, ParallelSteps): @@ -248,7 +258,13 @@ def _(item: ParallelItem, step_name, items, idx): @_extract_step_from_item.register -def _(item: StageItem): # noqa: ARG001 +def _( + # https://github.com/astral-sh/ruff/issues/18654 + item: StageItem, # noqa: ARG001 + step_name: str, # noqa: ARG001 + items: list[StepItem | ParallelItem | StageItem], # noqa: ARG001 + idx: int, # noqa: ARG001 +) -> Step | None: """Extract from a StageItem. We don't extract steps from within stages as they represent deployment @@ -300,6 +316,9 @@ def _( def _( item: ParallelItem, *, + # https://github.com/astral-sh/ruff/issues/18654 + items: list[StepItem | ParallelItem | StageItem], # noqa: ARG001 + idx: int, # noqa: ARG001 new_step: Step, ) -> None: """Add a new step to an existing ParallelItem.""" From 7410b3698f2fbfa9b86165387cb4d99ad448b9e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 02:20:51 +0000 Subject: [PATCH 11/19] Clarify comment and raise AssertionError when step not found Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- src/usethis/_integrations/ci/bitbucket/pipeweld.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index c5bc5bc5..7dff0ef9 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -149,12 +149,12 @@ def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 if is_new_step: step_to_insert = new_step else: - # This is an instruction to move an existing step + # This instruction applies to an existing step (e.g., to break up parallelism) # Find and extract the step from the pipeline extracted_step = _extract_step_from_items(items, instruction.step) if extracted_step is None: - # Step not found in pipeline, skip this instruction - return + msg = f"Step '{instruction.step}' not found in pipeline" + raise AssertionError(msg) step_to_insert = extracted_step if instruction.after is None: From 6758f8cc8fc97806d7b93561d08ce562384f6ac4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 02:40:31 +0000 Subject: [PATCH 12/19] Add detailed comments explaining new vs existing step logic and remove docstrings from register functions Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- .../_integrations/ci/bitbucket/pipeweld.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index 7dff0ef9..90a04773 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -144,6 +144,13 @@ def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 items = default.root.root # Check if this instruction is for a new step or an existing step + # When pipeweld determines we need to add a step (e.g., adding "lint" step to a pipeline), + # it generates instructions for how to insert it. If the pipeline already has steps in + # parallel that need to be rearranged to satisfy dependencies, pipeweld also generates + # instructions to move those existing steps. So: + # - is_new_step=True: inserting the actual new step being added (e.g., "lint") + # - is_new_step=False: rearranging an existing step (e.g., moving "build" from a parallel + # block so "lint" can be inserted between "build" and "test") is_new_step = get_pipeweld_step(new_step) == instruction.step if is_new_step: @@ -216,7 +223,6 @@ def _( items: list[StepItem | ParallelItem | StageItem], idx: int, ) -> Step | None: - """Extract from a StepItem.""" if get_pipeweld_step(item.step) == step_name: # Remove this item from the list items.pop(idx) @@ -231,7 +237,6 @@ def _( items: list[StepItem | ParallelItem | StageItem], idx: int, ) -> Step | None: - """Extract from a ParallelItem.""" if item.parallel is not None: if isinstance(item.parallel.root, ParallelSteps): step_items = item.parallel.root.root @@ -265,11 +270,8 @@ def _( items: list[StepItem | ParallelItem | StageItem], # noqa: ARG001 idx: int, # noqa: ARG001 ) -> Step | None: - """Extract from a StageItem. - - We don't extract steps from within stages as they represent deployment - stages and their internal structure should be preserved. - """ + # We don't extract steps from within stages as they represent deployment + # stages and their internal structure should be preserved. return None @@ -297,7 +299,6 @@ def _( idx: int, new_step: Step, ) -> None: - """Convert a single StepItem to a ParallelItem with both steps.""" # Replace the single step with a parallel block containing both steps parallel_item = ParallelItem( parallel=Parallel( @@ -321,7 +322,6 @@ def _( idx: int, # noqa: ARG001 new_step: Step, ) -> None: - """Add a new step to an existing ParallelItem.""" if item.parallel is not None: if isinstance(item.parallel.root, ParallelSteps): # Add to the existing list of parallel steps From 25f4c99e978aca02b50ed767b7493c4054e3fb0e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 02:48:55 +0000 Subject: [PATCH 13/19] Refactor to use MissingStepError exception instead of None checking Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- .../_integrations/ci/bitbucket/errors.py | 4 +++ .../_integrations/ci/bitbucket/pipeweld.py | 33 ++++++++++--------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/errors.py b/src/usethis/_integrations/ci/bitbucket/errors.py index 1c26abb2..4a463421 100644 --- a/src/usethis/_integrations/ci/bitbucket/errors.py +++ b/src/usethis/_integrations/ci/bitbucket/errors.py @@ -5,3 +5,7 @@ class UnexpectedImportPipelineError(UsethisError): """Raised when an import pipeline is unexpectedly encountered.""" + + +class MissingStepError(ValueError): + """Raised when a step cannot be found in the pipeline.""" diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index 90a04773..83f02827 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -8,7 +8,10 @@ import usethis._pipeweld.containers from usethis._integrations.ci.bitbucket.dump import bitbucket_fancy_dump -from usethis._integrations.ci.bitbucket.errors import UnexpectedImportPipelineError +from usethis._integrations.ci.bitbucket.errors import ( + MissingStepError, + UnexpectedImportPipelineError, +) from usethis._integrations.ci.bitbucket.io_ import ( edit_bitbucket_pipelines_yaml, ) @@ -148,21 +151,15 @@ def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 # it generates instructions for how to insert it. If the pipeline already has steps in # parallel that need to be rearranged to satisfy dependencies, pipeweld also generates # instructions to move those existing steps. So: - # - is_new_step=True: inserting the actual new step being added (e.g., "lint") - # - is_new_step=False: rearranging an existing step (e.g., moving "build" from a parallel + # - New step: inserting the actual new step being added (e.g., "lint") + # - Existing step: rearranging an existing step (e.g., moving "build" from a parallel # block so "lint" can be inserted between "build" and "test") - is_new_step = get_pipeweld_step(new_step) == instruction.step - - if is_new_step: + try: + # Try to extract an existing step with this name + step_to_insert = _extract_step_from_items(items, instruction.step) + except MissingStepError: + # Step not found in pipeline, so this must be the new step being added step_to_insert = new_step - else: - # This instruction applies to an existing step (e.g., to break up parallelism) - # Find and extract the step from the pipeline - extracted_step = _extract_step_from_items(items, instruction.step) - if extracted_step is None: - msg = f"Step '{instruction.step}' not found in pipeline" - raise AssertionError(msg) - step_to_insert = extracted_step if instruction.after is None: # Insert at the beginning - always as a simple step @@ -191,18 +188,22 @@ def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 def _extract_step_from_items( items: list[StepItem | ParallelItem | StageItem], step_name: str -) -> Step | None: +) -> "Step": """Find and remove a step from the items list. This function searches for a step with the given name, removes it from the items list, and returns the step. If the step is found in a parallel block with other steps, only that step is removed from the parallel block. + + Raises: + MissingStepError: If the step cannot be found in the pipeline. """ for idx, item in enumerate(items): extracted = _extract_step_from_item(item, step_name, items, idx) if extracted is not None: return extracted - return None + msg = f"Step '{step_name}' not found in pipeline" + raise MissingStepError(msg) @singledispatch From f2e552ea918f8db6959f0acaac224f2506bbfa38 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Mon, 27 Oct 2025 15:59:55 +1300 Subject: [PATCH 14/19] Pass Ruff --- src/usethis/_integrations/ci/bitbucket/pipeweld.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index 83f02827..a7cf33f8 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -188,13 +188,13 @@ def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 def _extract_step_from_items( items: list[StepItem | ParallelItem | StageItem], step_name: str -) -> "Step": +) -> Step: """Find and remove a step from the items list. This function searches for a step with the given name, removes it from the items list, and returns the step. If the step is found in a parallel block with other steps, only that step is removed from the parallel block. - + Raises: MissingStepError: If the step cannot be found in the pipeline. """ From 095e3f80ac03decf2f54faeae4d17689aa767f46 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 03:06:16 +0000 Subject: [PATCH 15/19] Extract instruction application logic and rename new_step to step_to_insert Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- .../_integrations/ci/bitbucket/pipeweld.py | 57 +++++++++++++------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index a7cf33f8..85a0f947 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -116,17 +116,21 @@ def _(item: StageItem): ) -def apply_pipeweld_instruction(instruction: Instruction, *, new_step: Step) -> None: +def apply_pipeweld_instruction( + instruction: Instruction, *, step_to_insert: Step +) -> None: with edit_bitbucket_pipelines_yaml() as doc: - apply_pipeweld_instruction_via_doc(instruction, doc=doc, new_step=new_step) + apply_pipeweld_instruction_via_doc( + instruction, doc=doc, step_to_insert=step_to_insert + ) dump = bitbucket_fancy_dump(doc.model, reference=doc.content) update_ruamel_yaml_map(doc.content, dump, preserve_comments=True) -def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 +def apply_pipeweld_instruction_via_doc( instruction: Instruction, *, - new_step: Step, + step_to_insert: Step, doc: BitbucketPipelinesYAMLDocument, ) -> None: if doc.model.pipelines is None: @@ -139,7 +143,7 @@ def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 items = [] elif isinstance(default.root, ImportPipeline): msg = ( - f"Cannot add step '{new_step.name}' to default pipeline in " + f"Cannot add step '{step_to_insert.name}' to default pipeline in " f"'bitbucket-pipelines.yml' because it is an import pipeline." ) raise UnexpectedImportPipelineError(msg) @@ -156,11 +160,29 @@ def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 # block so "lint" can be inserted between "build" and "test") try: # Try to extract an existing step with this name - step_to_insert = _extract_step_from_items(items, instruction.step) + step = _extract_step_from_items(items, instruction.step) except MissingStepError: # Step not found in pipeline, so this must be the new step being added - step_to_insert = new_step + step = step_to_insert + + _apply_instruction_to_items(instruction, items, step) + + if default is None and items: + pipelines.default = Pipeline(Items(items)) + +def _apply_instruction_to_items( + instruction: Instruction, + items: list[StepItem | ParallelItem | StageItem], + step_to_insert: Step, +) -> None: + """Apply an instruction to insert a step into the items list. + + Args: + instruction: The instruction specifying how to insert the step. + items: The list of pipeline items to modify. + step_to_insert: The step to insert. + """ if instruction.after is None: # Insert at the beginning - always as a simple step items.insert(0, StepItem(step=step_to_insert)) @@ -178,12 +200,11 @@ def apply_pipeweld_instruction_via_doc( # noqa: PLR0912 for idx, item in enumerate(items): if _is_insertion_necessary(item, instruction=instruction): _insert_parallel_step( - item, items=items, idx=idx, new_step=step_to_insert + item, items=items, idx=idx, step_to_insert=step_to_insert ) break - - if default is None and items: - pipelines.default = Pipeline(Items(items)) + else: + assert_never(instruction) def _extract_step_from_items( @@ -282,7 +303,7 @@ def _insert_parallel_step( *, items: list[StepItem | ParallelItem | StageItem], idx: int, - new_step: Step, + step_to_insert: Step, ) -> None: """Insert a step in parallel with an existing item. @@ -298,7 +319,7 @@ def _( *, items: list[StepItem | ParallelItem | StageItem], idx: int, - new_step: Step, + step_to_insert: Step, ) -> None: # Replace the single step with a parallel block containing both steps parallel_item = ParallelItem( @@ -306,7 +327,7 @@ def _( ParallelSteps( [ StepItem(step=item.step), - StepItem(step=new_step), + StepItem(step=step_to_insert), ] ) ) @@ -321,15 +342,15 @@ def _( # https://github.com/astral-sh/ruff/issues/18654 items: list[StepItem | ParallelItem | StageItem], # noqa: ARG001 idx: int, # noqa: ARG001 - new_step: Step, + step_to_insert: Step, ) -> None: if item.parallel is not None: if isinstance(item.parallel.root, ParallelSteps): # Add to the existing list of parallel steps - item.parallel.root.root.append(StepItem(step=new_step)) + item.parallel.root.root.append(StepItem(step=step_to_insert)) elif isinstance(item.parallel.root, ParallelExpanded): # Add to the expanded parallel steps - item.parallel.root.steps.root.append(StepItem(step=new_step)) + item.parallel.root.steps.root.append(StepItem(step=step_to_insert)) else: assert_never(item.parallel.root) @@ -340,7 +361,7 @@ def _( *, items: list[StepItem | ParallelItem | StageItem], idx: int, - new_step: Step, + step_to_insert: Step, ) -> None: # StageItems are trickier since they aren't supported in ParallelSteps. But we # never need to add them in practice anyway. The only reason this is really here From acb48b64053922ddf5b085725b091ffdddcde109 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Mon, 27 Oct 2025 16:10:56 +1300 Subject: [PATCH 16/19] Make various arguments kwarg-only --- .../_integrations/ci/bitbucket/pipeweld.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/pipeweld.py b/src/usethis/_integrations/ci/bitbucket/pipeweld.py index 85a0f947..f681f04d 100644 --- a/src/usethis/_integrations/ci/bitbucket/pipeweld.py +++ b/src/usethis/_integrations/ci/bitbucket/pipeweld.py @@ -160,24 +160,27 @@ def apply_pipeweld_instruction_via_doc( # block so "lint" can be inserted between "build" and "test") try: # Try to extract an existing step with this name - step = _extract_step_from_items(items, instruction.step) + step = _extract_step_from_items(items, step_name=instruction.step) except MissingStepError: # Step not found in pipeline, so this must be the new step being added step = step_to_insert - _apply_instruction_to_items(instruction, items, step) + _apply_instruction_to_items( + instruction=instruction, items=items, step_to_insert=step + ) if default is None and items: pipelines.default = Pipeline(Items(items)) def _apply_instruction_to_items( + *, instruction: Instruction, items: list[StepItem | ParallelItem | StageItem], step_to_insert: Step, ) -> None: """Apply an instruction to insert a step into the items list. - + Args: instruction: The instruction specifying how to insert the step. items: The list of pipeline items to modify. @@ -208,7 +211,7 @@ def _apply_instruction_to_items( def _extract_step_from_items( - items: list[StepItem | ParallelItem | StageItem], step_name: str + items: list[StepItem | ParallelItem | StageItem], *, step_name: str ) -> Step: """Find and remove a step from the items list. @@ -220,7 +223,9 @@ def _extract_step_from_items( MissingStepError: If the step cannot be found in the pipeline. """ for idx, item in enumerate(items): - extracted = _extract_step_from_item(item, step_name, items, idx) + extracted = _extract_step_from_item( + item, step_name=step_name, items=items, idx=idx + ) if extracted is not None: return extracted msg = f"Step '{step_name}' not found in pipeline" @@ -230,6 +235,7 @@ def _extract_step_from_items( @singledispatch def _extract_step_from_item( item: StepItem | ParallelItem | StageItem, + *, step_name: str, items: list[StepItem | ParallelItem | StageItem], idx: int, @@ -241,6 +247,7 @@ def _extract_step_from_item( @_extract_step_from_item.register def _( item: StepItem, + *, step_name: str, items: list[StepItem | ParallelItem | StageItem], idx: int, @@ -255,6 +262,7 @@ def _( @_extract_step_from_item.register def _( item: ParallelItem, + *, step_name: str, items: list[StepItem | ParallelItem | StageItem], idx: int, @@ -288,6 +296,7 @@ def _( def _( # https://github.com/astral-sh/ruff/issues/18654 item: StageItem, # noqa: ARG001 + *, step_name: str, # noqa: ARG001 items: list[StepItem | ParallelItem | StageItem], # noqa: ARG001 idx: int, # noqa: ARG001 From a7a1dd1576c8aa59dbf83ee12583466fe4c857f2 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Mon, 27 Oct 2025 16:19:07 +1300 Subject: [PATCH 17/19] Apply variable renaming consistently (step_to_insert) --- .../_integrations/ci/bitbucket/steps.py | 2 +- .../ci/bitbucket/test_pipeweld.py | 26 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/usethis/_integrations/ci/bitbucket/steps.py b/src/usethis/_integrations/ci/bitbucket/steps.py index 8b0396f3..878717ec 100644 --- a/src/usethis/_integrations/ci/bitbucket/steps.py +++ b/src/usethis/_integrations/ci/bitbucket/steps.py @@ -195,7 +195,7 @@ def _add_step_in_default_via_doc( ).add() for instruction in weld_result.instructions: apply_pipeweld_instruction_via_doc( - instruction=instruction, new_step=step, doc=doc + instruction=instruction, step_to_insert=step, doc=doc ) diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index 54065f67..f35f5f8c 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -41,7 +41,7 @@ def test_add_to_brand_new_pipeline(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertSuccessor(step="foo", after=None), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert @@ -74,7 +74,7 @@ def test_import_pipeline_fails(self, tmp_path: Path): with change_cwd(tmp_path), pytest.raises(UnexpectedImportPipelineError): apply_pipeweld_instruction( InsertSuccessor(step="foo", after=None), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) def test_existing_pipeline(self, tmp_path: Path): @@ -95,7 +95,7 @@ def test_existing_pipeline(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertSuccessor(step="foo", after=None), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert @@ -136,7 +136,7 @@ def test_step_item(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertSuccessor(step="foo", after="bar"), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert @@ -181,7 +181,7 @@ def test_parallel_item(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertSuccessor(step="foo", after="qux"), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert @@ -232,7 +232,7 @@ def test_parallel_expanded(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertSuccessor(step="foo", after="qux"), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert @@ -280,7 +280,7 @@ def test_stage_item(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertSuccessor(step="foo", after="baz"), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert @@ -323,7 +323,7 @@ def test_simple_step(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertParallel(step="foo", after="bar"), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert: Should create a parallel block with both steps @@ -369,7 +369,7 @@ def test_add_to_existing_parallel(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertParallel(step="foo", after="bar"), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert: Should add to the existing parallel block @@ -408,7 +408,7 @@ def test_parallel_after_none(self, tmp_path: Path): with change_cwd(tmp_path): apply_pipeweld_instruction( InsertParallel(step="foo", after=None), - new_step=Step(name="foo", script=Script(["echo foo"])), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), ) # Assert: Should just add the step (parallel with no other steps is just a step) @@ -458,19 +458,19 @@ def test_full_parallel_breakup_sequence(self, tmp_path: Path): # Step 1: Move A to beginning apply_pipeweld_instruction( InsertSuccessor(step="A", after=None), - new_step=Step(name="C", script=Script(["echo C"])), + step_to_insert=Step(name="C", script=Script(["echo C"])), ) # Step 2: Move B after A apply_pipeweld_instruction( InsertSuccessor(step="B", after="A"), - new_step=Step(name="C", script=Script(["echo C"])), + step_to_insert=Step(name="C", script=Script(["echo C"])), ) # Step 3: Insert C after A (C will go between A and B) apply_pipeweld_instruction( InsertSuccessor(step="C", after="A"), - new_step=Step(name="C", script=Script(["echo C"])), + step_to_insert=Step(name="C", script=Script(["echo C"])), ) # Assert: Should result in A, then C, then B in series From 7a28d39be4cc0b33b2109d27e69e954a4b314eca Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Oct 2025 03:29:48 +0000 Subject: [PATCH 18/19] Add tests for ParallelExpanded format and empty parallel block extraction Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --- .../ci/bitbucket/test_pipeweld.py | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index f35f5f8c..f66d7441 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -426,6 +426,58 @@ def test_parallel_after_none(self, tmp_path: Path): """ ) + def test_add_to_existing_parallel_expanded_format(self, tmp_path: Path): + # Arrange: Pipeline with an existing parallel block using expanded format + (tmp_path / "bitbucket-pipelines.yml").write_text( + """\ +image: atlassian/default-image:3 +pipelines: + default: + - parallel: + steps: + - step: + name: bar + script: + - echo bar + - step: + name: baz + script: + - echo baz +""" + ) + + # Act: Insert foo in parallel to bar (expanded format) + with change_cwd(tmp_path): + apply_pipeweld_instruction( + InsertParallel(step="foo", after="bar"), + step_to_insert=Step(name="foo", script=Script(["echo foo"])), + ) + + # Assert: Should add to the existing parallel block + content = (tmp_path / "bitbucket-pipelines.yml").read_text() + assert ( + content + == """\ +image: atlassian/default-image:3 +pipelines: + default: + - parallel: + steps: + - step: + name: bar + script: + - echo bar + - step: + name: baz + script: + - echo baz + - step: + name: foo + script: + - echo foo +""" + ) + class TestBreakUpParallelism: """Test breaking up an existing parallel block to satisfy dependencies.""" @@ -496,6 +548,51 @@ def test_full_parallel_breakup_sequence(self, tmp_path: Path): """ ) + def test_extract_last_step_from_parallel(self, tmp_path: Path): + # Arrange: Parallel block with a single step + (tmp_path / "bitbucket-pipelines.yml").write_text( + """\ +image: atlassian/default-image:3 +pipelines: + default: + - parallel: + - step: + name: A + script: + - echo A + - step: + name: B + script: + - echo B +""" + ) + + # Act: Extract the only step from the parallel block + with change_cwd(tmp_path): + apply_pipeweld_instruction( + InsertSuccessor(step="A", after=None), + step_to_insert=Step(name="C", script=Script(["echo C"])), + ) + + # Assert: Parallel block should be removed entirely, leaving A and B + content = (tmp_path / "bitbucket-pipelines.yml").read_text() + assert ( + content + == """\ +image: atlassian/default-image:3 +pipelines: + default: + - step: + name: A + script: + - echo A + - step: + name: B + script: + - echo B +""" + ) + class TestGetInstructionsForInsertion: class TestStr: From fa35be176ff7dd68b8e03d33e2c48f853d8251c8 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Mon, 27 Oct 2025 16:43:44 +1300 Subject: [PATCH 19/19] Fix outdated variable name in comment Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py index f66d7441..97095cc2 100644 --- a/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py +++ b/tests/usethis/_integrations/ci/bitbucket/test_pipeweld.py @@ -503,7 +503,7 @@ def test_full_parallel_breakup_sequence(self, tmp_path: Path): with change_cwd(tmp_path): # Simulate pipeweld instructions for: series("A", "C", "B") - # Note: new_step is always C (the actual new step being added) + # Note: step_to_insert is always C (the actual new step being added) # but instruction.step varies (A, B, or C) to indicate which step # the instruction is about (existing or new)