Skip to content

Add prompty-dumpty feature with installation scripts and tests#4

Merged
rosstaco merged 3 commits intomainfrom
feat/prompty-dumpty
Nov 15, 2025
Merged

Add prompty-dumpty feature with installation scripts and tests#4
rosstaco merged 3 commits intomainfrom
feat/prompty-dumpty

Conversation

@rosstaco
Copy link
Copy Markdown
Owner

  • 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
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment thread test/prompty-dumpty/version.sh Outdated

source dev-container-features-test-lib

check "dumpty is installed" which dumpty
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
check "dumpty is installed" which dumpty

Copilot uses AI. Check for mistakes.
Comment thread test/prompty-dumpty/test.sh Outdated
source dev-container-features-test-lib

# Feature-specific tests
check "dumpty is installed" which dumpty
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
check "dumpty is installed" which dumpty

Copilot uses AI. Check for mistakes.
Comment thread test/prompty-dumpty/debian.sh Outdated

source dev-container-features-test-lib

check "dumpty is installed" which dumpty
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
check "dumpty is installed" which dumpty

Copilot uses AI. Check for mistakes.
Comment thread test/prompty-dumpty/alpine.sh Outdated

source dev-container-features-test-lib

check "dumpty is installed" which dumpty
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
check "dumpty is installed" which dumpty

Copilot uses AI. Check for mistakes.
Comment thread src/prompty-dumpty/install.sh Outdated
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")
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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
Suggested change
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

Copilot uses AI. Check for mistakes.
Comment thread src/prompty-dumpty/install.sh Outdated
Comment on lines +70 to +72
if dumpty --version &> /dev/null; then
INSTALLED_VERSION=$(dumpty --version 2>&1 || echo "version unknown")
echo "Installed version: $INSTALLED_VERSION"
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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
Suggested change
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"

Copilot uses AI. Check for mistakes.
Comment thread .devcontainer/devcontainer.json Outdated
Comment on lines +22 to +25
"ghcr.io/jsburckhardt/devcontainer-features/just:1": {},
"ghcr.io/rosstaco/devcontainer-features/microsoft-security-devops-cli:1": {
"version": "latest"
}
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
"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": {}

Copilot uses AI. Check for mistakes.
@rosstaco
Copy link
Copy Markdown
Owner Author

Fixed in commit 78b8c63 - removed redundant which checks from all test files, fixed set -e compatibility in install.sh, and removed the unrelated devcontainer.json change. All tests passing.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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"
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
check "dumpty version command works" bash -c "dumpty --version 2>&1 || true"
check "dumpty version command works" bash -c "dumpty --version 2>&1"

Copilot uses AI. Check for mistakes.

check "dumpty is executable" bash -c "command -v dumpty"

check "dumpty version command works" bash -c "dumpty --version 2>&1 || true"
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
check "dumpty version command works" bash -c "dumpty --version 2>&1 || true"
check "dumpty version command works" bash -c "dumpty --version 2>&1"

Copilot uses AI. Check for mistakes.

check "dumpty is executable" bash -c "command -v dumpty"

check "dumpty version command works" bash -c "dumpty --version 2>&1 || true"
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
check "dumpty version command works" bash -c "dumpty --version 2>&1 || true"
check "dumpty version command works" bash -c "dumpty --version 2>&1"

Copilot uses AI. Check for mistakes.

check "dumpty is executable" bash -c "command -v dumpty"

check "dumpty version command works" bash -c "dumpty --version 2>&1 || true"
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
check "dumpty version command works" bash -c "dumpty --version 2>&1 || true"
check "dumpty version command works" bash -c "dumpty --version 2>&1"

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +23
check "dumpty is executable" bash -c "command -v dumpty"

check "dumpty version command works" bash -c "dumpty --version 2>&1 || true"

# Report results
reportResults
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
@rosstaco rosstaco merged commit f0e406f into main Nov 15, 2025
20 checks passed
@rosstaco rosstaco deleted the feat/prompty-dumpty branch November 15, 2025 12:26
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.

2 participants