Skip to content

Parameter Expansion (2/2) - #285

Merged
UdeshyaDhungana merged 2 commits into
parameter-expansionfrom
parameter-expansion-2
Apr 22, 2026
Merged

UdeshyaDhungana merged 2 commits into
parameter-expansionfrom
parameter-expansion-2

Conversation

@UdeshyaDhungana

@UdeshyaDhungana UdeshyaDhungana commented Apr 16, 2026 •

Copy link
Copy Markdown
Contributor

Note

Low Risk
Changes are confined to test stages and golden fixtures; primary risk is increased flakiness if randomized names/values ever collide or produce edge-case tokens.

Overview
Adds concrete implementations for testPEX5, testPEX6, and testPEX7, replacing stubs with end-to-end tests that set variables via declare and then verify argument expansion when invoking a generated signature_printer executable.

Stage 5 asserts basic $VAR expansion into argv, stage 6 asserts brace form ${VAR} including embedding within surrounding literals, and stage 7 asserts missing variables expand to an empty string (including inside a larger token). Updates the bash fixture parameter_expansion/pass to include the new expected transcripts (PATH setup, executable listing, and multiline argv output).

Reviewed by Cursor Bugbot for commit 79a29ac. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: No-op strings.Join wrapping a single string variable
    • Replaced the no-op strings.Join([]string{command}, " ") with just command in all three files, and removed the unused strings import from stage_pex7.go.

Create PR

Or push these changes by commenting:

@cursor push 8e86eea1c7
Preview (8e86eea1c7)
diff --git a/internal/stage_pex5.go b/internal/stage_pex5.go
--- a/internal/stage_pex5.go
+++ b/internal/stage_pex5.go
@@ -67,7 +67,7 @@
 	}
 
 	testCase := test_cases.CommandWithMultilineResponseTestCase{
-		Command:            strings.Join([]string{command}, " "),
+		Command:            command,
 		MultiLineAssertion: assertions.NewMultiLineAssertion(expectedLines),
 		SuccessMessage:     "✓ Received expected response",
 	}

diff --git a/internal/stage_pex6.go b/internal/stage_pex6.go
--- a/internal/stage_pex6.go
+++ b/internal/stage_pex6.go
@@ -76,7 +76,7 @@
 	}
 
 	testCase := test_cases.CommandWithMultilineResponseTestCase{
-		Command:            strings.Join([]string{command}, " "),
+		Command:            command,
 		MultiLineAssertion: assertions.NewMultiLineAssertion(expectedLines),
 		SuccessMessage:     "✓ Received expected response",
 	}

diff --git a/internal/stage_pex7.go b/internal/stage_pex7.go
--- a/internal/stage_pex7.go
+++ b/internal/stage_pex7.go
@@ -3,7 +3,6 @@
 import (
 	"fmt"
 	"strconv"
-	"strings"
 
 	"github.com/codecrafters-io/shell-tester/internal/assertions"
 	"github.com/codecrafters-io/shell-tester/internal/logged_shell_asserter"
@@ -55,7 +54,7 @@
 	}
 
 	testCase := test_cases.CommandWithMultilineResponseTestCase{
-		Command:            strings.Join([]string{command}, " "),
+		Command:            command,
 		MultiLineAssertion: assertions.NewMultiLineAssertion(expectedLines),
 		SuccessMessage:     "✓ Received expected response",
 	}

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit f556693. Configure here.

Comment thread internal/stage_pex5.go Outdated
@UdeshyaDhungana UdeshyaDhungana changed the title Parameter Expansion (Stage 5-7) Parameter Expansion (2/2) Apr 22, 2026
@UdeshyaDhungana
UdeshyaDhungana merged commit 21d5782 into parameter-expansion Apr 22, 2026
7 checks passed
@UdeshyaDhungana
UdeshyaDhungana deleted the parameter-expansion-2 branch April 22, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant