Skip to content

fix: CI test assertions for Rich line-wrapping and scope output #510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielmeppiel merged 1 commit into from
Mar 31, 2026
Merged

fix: CI test assertions for Rich line-wrapping and scope output #510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions scripts/test-dependency-integration.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ EOF
echo "--- OUTPUT START ---"
echo "$deps_output"
echo "--- OUTPUT END ---"
if echo "$deps_output" | grep -q "No APM dependencies installed yet"; then
if echo "$deps_output" | grep -q "No APM dependencies installed"; then
log_success "Correctly shows no dependencies installed"
else
log_error "Expected 'No APM dependencies installed yet' message"
log_error "Expected 'No APM dependencies installed' message"
log_error "Got: $deps_output"
Comment on lines +67 to 71
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

This script contains non-ASCII characters (em dash in comments, and emoji symbols in log_* output). Repository guideline requires shell scripts to stay within printable ASCII to avoid Windows cp1252 encoding errors. Please replace the emoji/status glyphs with the ASCII bracket status symbols (e.g., [i], [+], [x]) and use a plain - instead of an em dash.

Copilot generated this review using guidance from repository custom instructions.
return 1
fi
Expand Down Expand Up @@ -256,7 +256,7 @@ test_dependency_cleanup() {
echo "--- OUTPUT START ---"
echo "$deps_output_after_cleanup"
echo "--- OUTPUT END ---"
if echo "$deps_output_after_cleanup" | grep -q "No APM dependencies installed yet"; then
if echo "$deps_output_after_cleanup" | grep -q "No APM dependencies installed"; then
log_success "Correctly shows no dependencies after cleanup"
else
log_error "Expected no dependencies after cleanup"
Expand Down
6 changes: 3 additions & 3 deletions scripts/windows/test-dependency-integration.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ scripts:
Write-Host "--- OUTPUT START ---"
Write-Host $depsOutput
Write-Host "--- OUTPUT END ---"
if ($depsOutput -match "No APM dependencies installed yet") {
if ($depsOutput -match "No APM dependencies installed") {
Write-DepSuccess "Correctly shows no dependencies installed"
} else {
Write-DepError "Expected 'No APM dependencies installed yet' message"
Write-DepError "Expected 'No APM dependencies installed' message"
Write-DepError "Got: $depsOutput"
return $false
}
Expand Down Expand Up @@ -282,7 +282,7 @@ function Test-DependencyCleanup {
Write-Host "--- OUTPUT START ---"
Write-Host $depsOutput
Write-Host "--- OUTPUT END ---"
if ($depsOutput -match "No APM dependencies installed yet") {
if ($depsOutput -match "No APM dependencies installed") {
Write-DepSuccess "Correctly shows no dependencies after cleanup"
} else {
Write-DepError "Expected no dependencies after cleanup"
Expand Down
8 changes: 5 additions & 3 deletions tests/unit/test_install_command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,9 @@ def test_global_rejects_local_path_package(self):
)

# Should fail with clear message about local packages
assert "local packages are not supported at user scope" in result.output
# Use shorter substring to tolerate Rich word-wrapping on
# platforms with long temp paths (Windows).
assert "not supported at user scope" in result.output
Comment on lines 820 to +823
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

The new assertion "not supported at user scope" is so broad that it can pass even if the wrong message is printed (e.g., install.py also emits "local paths are not supported at user scope", and other user-scope warnings may include the same substring). To keep the test specific while still tolerating Rich line-wrapping, consider matching the full sentence after normalizing whitespace (e.g., collapse newlines to spaces) or using a regex that allows \s+ between words.

Copilot uses AI. Check for mistakes.
# Should suggest using remote reference
assert "owner/repo" in result.output
finally:
Expand All @@ -840,7 +842,7 @@ def test_global_rejects_absolute_local_path(self):
cli, ["install", "--global", str(local_pkg)]
)

assert "local packages are not supported at user scope" in result.output
assert "not supported at user scope" in result.output
finally:
os.chdir(self.original_dir)

Expand All @@ -857,6 +859,6 @@ def test_global_rejects_tilde_local_path(self):
cli, ["install", "--global", "~/some-pkg"]
)

assert "local packages are not supported at user scope" in result.output
assert "not supported at user scope" in result.output
finally:
os.chdir(self.original_dir)
Loading