Add prompty-dumpty feature with installation scripts and tests#4
Add prompty-dumpty feature with installation scripts and tests#4
Conversation
rosstaco
commented
Nov 15, 2025
- Implemented installation script for prompty-dumpty CLI tool
- Added devcontainer-feature.json and README for feature documentation
- Created test scripts for Alpine and Debian environments
- Updated scenarios.json to include testing configurations
- Enhanced .gitignore and devcontainer.json for new feature integration
- Implemented installation script for prompty-dumpty CLI tool - Added devcontainer-feature.json and README for feature documentation - Created test scripts for Alpine and Debian environments - Updated scenarios.json to include testing configurations - Enhanced .gitignore and devcontainer.json for new feature integration
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new devcontainer feature for installing the prompty-dumpty CLI tool, a Python package for managing prompty files. The implementation includes installation scripts with multi-distribution support, comprehensive test coverage across different Linux distributions, and proper documentation.
Key Changes
- Installation script with automatic Python pip detection and installation across apt/apk/yum package managers
- Test suite covering Ubuntu, Debian, and Alpine Linux environments with version-specific and latest installation scenarios
- Feature configuration with customizable version option (default: latest)
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/prompty-dumpty/install.sh | Installation script handling pip installation with fallback for --break-system-packages flag |
| src/prompty-dumpty/devcontainer-feature.json | Feature metadata defining the prompty-dumpty feature with version option |
| src/prompty-dumpty/README.md | User-facing documentation with usage examples and configuration options |
| test/prompty-dumpty/test.sh | Default test scenario verifying dumpty command availability |
| test/prompty-dumpty/version.sh | Version-specific installation test for v0.6.2 |
| test/prompty-dumpty/debian.sh | Debian-specific compatibility test |
| test/prompty-dumpty/alpine.sh | Alpine Linux-specific compatibility test |
| test/prompty-dumpty/scenarios.json | Test configuration defining Ubuntu, Debian, and Alpine test scenarios |
| docs/prompty-dumpty-plan.md | Implementation plan documenting feature design decisions |
| .gitignore | Added .DS_Store to ignore macOS system files |
| .devcontainer/devcontainer.json | Added microsoft-security-devops-cli feature (unrelated to prompty-dumpty) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| source dev-container-features-test-lib | ||
|
|
||
| check "dumpty is installed" which dumpty |
There was a problem hiding this comment.
The check on line 11 is redundant with line 9. Both which dumpty and command -v dumpty verify that the dumpty command is available in PATH. The command -v check is already the preferred POSIX-compliant method, so the which check on line 9 is unnecessary.
| check "dumpty is installed" which dumpty |
| source dev-container-features-test-lib | ||
|
|
||
| # Feature-specific tests | ||
| check "dumpty is installed" which dumpty |
There was a problem hiding this comment.
The check on line 20 is redundant with line 18. Both which dumpty and command -v dumpty verify that the dumpty command is available in PATH. The command -v check is already the preferred POSIX-compliant method, so the which check on line 18 is unnecessary.
| check "dumpty is installed" which dumpty |
|
|
||
| source dev-container-features-test-lib | ||
|
|
||
| check "dumpty is installed" which dumpty |
There was a problem hiding this comment.
The check on line 11 is redundant with line 9. Both which dumpty and command -v dumpty verify that the dumpty command is available in PATH. The command -v check is already the preferred POSIX-compliant method, so the which check on line 9 is unnecessary.
| check "dumpty is installed" which dumpty |
|
|
||
| source dev-container-features-test-lib | ||
|
|
||
| check "dumpty is installed" which dumpty |
There was a problem hiding this comment.
The check on line 11 is redundant with line 9. Both which dumpty and command -v dumpty verify that the dumpty command is available in PATH. The command -v check is already the preferred POSIX-compliant method, so the which check on line 9 is unnecessary.
| check "dumpty is installed" which dumpty |
| fi | ||
|
|
||
| # Check if pip supports --break-system-packages flag (pip >= 23.0) | ||
| PIP_SUPPORTS_BREAK_SYSTEM=$(pip3 install --help 2>&1 | grep -q "break-system-packages" && echo "yes" || echo "no") |
There was a problem hiding this comment.
The grep exit code is not being properly checked here. When grep doesn't find a match, it returns exit code 1, which will cause the script to exit due to set -e on line 2. This line should be wrapped in a conditional that doesn't fail on non-zero exit codes.
Suggested fix:
if pip3 install --help 2>&1 | grep -q "break-system-packages"; then
PIP_SUPPORTS_BREAK_SYSTEM="yes"
else
PIP_SUPPORTS_BREAK_SYSTEM="no"
fi| PIP_SUPPORTS_BREAK_SYSTEM=$(pip3 install --help 2>&1 | grep -q "break-system-packages" && echo "yes" || echo "no") | |
| if pip3 install --help 2>&1 | grep -q "break-system-packages"; then | |
| PIP_SUPPORTS_BREAK_SYSTEM="yes" | |
| else | |
| PIP_SUPPORTS_BREAK_SYSTEM="no" | |
| fi |
| if dumpty --version &> /dev/null; then | ||
| INSTALLED_VERSION=$(dumpty --version 2>&1 || echo "version unknown") | ||
| echo "Installed version: $INSTALLED_VERSION" |
There was a problem hiding this comment.
The version output is captured but dumpty --version may fail (exit non-zero) even when the command exists. The || echo "version unknown" fallback won't prevent the script from exiting due to set -e on line 2 if the command fails before the || can be evaluated in the command substitution.
Suggested fix:
INSTALLED_VERSION=$(dumpty --version 2>&1 || echo "version unknown")Or more robustly:
if INSTALLED_VERSION=$(dumpty --version 2>&1); then
echo "Installed version: $INSTALLED_VERSION"
else
echo "Installed version: version check failed"
fi| if dumpty --version &> /dev/null; then | |
| INSTALLED_VERSION=$(dumpty --version 2>&1 || echo "version unknown") | |
| echo "Installed version: $INSTALLED_VERSION" | |
| if INSTALLED_VERSION=$(dumpty --version 2>&1); then | |
| echo "Installed version: $INSTALLED_VERSION" | |
| else | |
| echo "Installed version: version check failed" |
| "ghcr.io/jsburckhardt/devcontainer-features/just:1": {}, | ||
| "ghcr.io/rosstaco/devcontainer-features/microsoft-security-devops-cli:1": { | ||
| "version": "latest" | ||
| } |
There was a problem hiding this comment.
This change adds the microsoft-security-devops-cli feature to the devcontainer, but the PR is about adding the prompty-dumpty feature. This appears to be an unrelated change that should either be removed or explained in the PR description.
| "ghcr.io/jsburckhardt/devcontainer-features/just:1": {}, | |
| "ghcr.io/rosstaco/devcontainer-features/microsoft-security-devops-cli:1": { | |
| "version": "latest" | |
| } | |
| "ghcr.io/jsburckhardt/devcontainer-features/just:1": {} |
|
Fixed in commit 78b8c63 - removed redundant |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Feature-specific tests | ||
| check "dumpty is executable" bash -c "command -v dumpty" | ||
|
|
||
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" |
There was a problem hiding this comment.
The || true at the end of the version check masks potential failures. If dumpty --version fails, the test will still pass. Consider removing || true to ensure the command actually succeeds, or add explicit validation of the output.
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" | |
| check "dumpty version command works" bash -c "dumpty --version 2>&1" |
|
|
||
| check "dumpty is executable" bash -c "command -v dumpty" | ||
|
|
||
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" |
There was a problem hiding this comment.
The || true at the end of the version check masks potential failures. If dumpty --version fails, the test will still pass. Consider removing || true to ensure the command actually succeeds, or add explicit validation of the output.
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" | |
| check "dumpty version command works" bash -c "dumpty --version 2>&1" |
|
|
||
| check "dumpty is executable" bash -c "command -v dumpty" | ||
|
|
||
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" |
There was a problem hiding this comment.
The || true at the end of the version check masks potential failures. If dumpty --version fails, the test will still pass. Consider removing || true to ensure the command actually succeeds, or add explicit validation of the output.
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" | |
| check "dumpty version command works" bash -c "dumpty --version 2>&1" |
|
|
||
| check "dumpty is executable" bash -c "command -v dumpty" | ||
|
|
||
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" |
There was a problem hiding this comment.
The || true at the end of the version check masks potential failures. If dumpty --version fails, the test will still pass. Consider removing || true to ensure the command actually succeeds, or add explicit validation of the output.
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" | |
| check "dumpty version command works" bash -c "dumpty --version 2>&1" |
| check "dumpty is executable" bash -c "command -v dumpty" | ||
|
|
||
| check "dumpty version command works" bash -c "dumpty --version 2>&1 || true" | ||
|
|
||
| # Report results | ||
| reportResults |
There was a problem hiding this comment.
The test logic in lines 18-20 is duplicated across all four test files (test.sh, version.sh, alpine.sh, debian.sh). Consider extracting this common test logic into a shared function or separate file to reduce duplication and improve maintainability.