Skip to content

feat(foundryup): Mutually Exclusive Argument Checks/Flags #9897

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sambacha
Copy link
Contributor

Important

Mutually Exclusive Argument Check
Added checks before processing each action argument. For example, before processing -i or --install, it checks if [[ $has_action -ne 0 ]]. If has_action is not0(meaning another action argument has already been seen), it prints an error message using the err function and exits. This prevents combinations like foundryup -i 1.0.0 -l.

Important

Required Argument Validation:
Added checks like elif [[ $has_install -eq 1 && -z "$FOUNDRYUP_VERSION" ]] to ensure that arguments like --install and --use are followed by a version value. If the version is missing, an error message is displayed.

Motivation

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

> [!NOTE]
>  Mutually Exclusive Argument Check
Added checks before processing each action argument. For example, before processing -i or --install, it checks `if [[ $has_action -ne 0 ]]`. `If has_action` is not` 0 `(meaning another action argument has already been seen), it prints an error message using the `err` function and exits. This prevents combinations like `foundryup -i 1.0.0 -l`.

> [!IMPORTANT]
>  Required Argument Validation:
 Added checks like `elif [[ $has_install -eq 1 && -z "$FOUNDRYUP_VERSION" ]]` to ensure that arguments like `--install ` and `--use` are followed by a version value. If the version is missing, an error message is displayed.
@sambacha
Copy link
Contributor Author

There ought to also be some testing for foundryup, e.g.

Bash_Unit Testing

Note

Example Test File

#!/usr/bin/env bash

# Source bash_unit.  Adjust the path if necessary.
source ./bash_unit/bash_unit

# Source the script to be tested.
source ./foundryup

# --- Test Helper Functions ---

test_usage() {
  local output
  output=$(usage 2>&1)  # Capture both stdout and stderr
  assert_contains "The installer for Foundry." "$output"
  assert_contains "USAGE:" "$output"
  assert_contains "OPTIONS:" "$output"
}

test_version() {
  local output
  output=$(version 2>&1)
  assert_equals "$FOUNDRYUP_INSTALLER_VERSION" "$output"
}

test_need_cmd_success() {
  need_cmd git  # Should not exit
  assert_true  # If we reach here, it's a success
}

test_need_cmd_failure() {
  # Expect an exit code of 1
  if need_cmd nonexistentcommand; then
    assert_fail "need_cmd should have exited"
  fi
  assert_equals 1 "$?"
}

test_check_cmd_success() {
  assert_true "$(check_cmd git)"
}

test_check_cmd_failure() {
  assert_false "$(check_cmd nonexistentcommand)"
}

test_tolower() {
  assert_equals "lowercase" "$(tolower "LOWERCASE")"
  assert_equals "mixedcase" "$(tolower "MiXeDcAsE")"
}

test_ensure_success() {
  ensure true  # Should not exit
  assert_true
}

test_ensure_failure() {
  if ensure false; then
    assert_fail "ensure should have exited"
  fi
  assert_equals 1 "$?"
}

test_check_bins_in_use_no_process() {
    # This test assumes no forge/cast/anvil/chisel processes are running.
    check_bins_in_use
    assert_true #If we reach here, it is a success.
}

test_say() {
    local output
    output=$(say "Test message" 2>&1)
    assert_equals "foundryup: Test message" "$output"
}

test_warn() {
    local output
    output=$(warn "Test message" 2>&1)
    assert_equals "foundryup: warning: Test message" "$output"
}

test_err() {
    local output
    output=$(err "Test message" 2>&1)
    assert_equals "foundryup: Test message" "$output"
    assert_equals 1 "$?"
}

# --- Test Argument Parsing (main function indirectly) ---

test_main_version_arg() {
    # Backup original values
    local original_FOUNDRYUP_INSTALLER_VERSION=$FOUNDRYUP_INSTALLER_VERSION

    # Run main with -v
    ./foundryup -v > /dev/null 2>&1
    assert_equals 0 "$?"

    # Restore original values
    FOUNDRYUP_INSTALLER_VERSION=$original_FOUNDRYUP_INSTALLER_VERSION
}

test_main_help_arg() {
  # Run main with -h
  ./foundryup -h > /dev/null 2>&1
  assert_equals 0 "$?"
}

test_main_invalid_arg_combination() {
    # Run main with an invalid combination (-v and -i)
    ./foundryup -v -i stable > /dev/null 2>&1
    assert_equals 1 "$?"
}

test_main_install_arg() {
    # Backup original values
    local original_FOUNDRYUP_VERSION=$FOUNDRYUP_VERSION

    # Run main with -i
    ./foundryup -i "v1.2.3" > /dev/null 2>&1
    assert_equals 0 "$?"

    # Restore original values
    FOUNDRYUP_VERSION=$original_FOUNDRYUP_VERSION
}

test_main_install_missing_version() {
  ./foundryup -i > /dev/null 2>&1
  assert_equals 1 "$?"
}

test_main_use_arg() {
    # Backup original values
    local original_FOUNDRYUP_VERSION=$FOUNDRYUP_VERSION

    # Run main with -u
     ./foundryup -u "v1.2.3" > /dev/null 2>&1
    assert_equals 0 "$?"

    # Restore original values
    FOUNDRYUP_VERSION=$original_FOUNDRYUP_VERSION
}

test_main_use_missing_version() {
  ./foundryup -u > /dev/null 2>&1
  assert_equals 1 "$?"
}

test_main_update() {
    ./foundryup -U > /dev/null 2>&1
    assert_equals 0 "$?"
}

test_main_local_install() {
    # Create a dummy directory structure for the local install test
    mkdir -p tmp_test_repo/target/release
    touch tmp_test_repo/target/release/forge
    touch tmp_test_repo/target/release/cast
    touch tmp_test_repo/target/release/anvil
    touch tmp_test_repo/target/release/chisel

    # Run with -p pointing to the dummy repo
    ./foundryup -p tmp_test_repo > /dev/null 2>&1
    assert_equals 0 "$?"

    # Cleanup
    rm -rf tmp_test_repo
    rm -f "$FOUNDRY_BIN_DIR/forge"
    rm -f "$FOUNDRY_BIN_DIR/cast"
    rm -f "$FOUNDRY_BIN_DIR/anvil"
    rm -f "$FOUNDRY_BIN_DIR/chisel"
}

test_main_branch_install() {
    # Backup original values
    local original_FOUNDRYUP_BRANCH=$FOUNDRYUP_BRANCH

    # Run with -b
    ./foundryup -b my-branch > /dev/null 2>&1
    assert_equals 0 "$?"

    # Restore original values
    FOUNDRYUP_BRANCH=$original_FOUNDRYUP_BRANCH
}

# Run the tests
bash_unit "$0"

Workflow CI

jobs:
  test:
    runs-on: ubuntu-latest

    steps:
    - name: Checkout code
      uses: actions/checkout@v3

    - name: Install bash_unit
      run: |
        git clone https://github.com/pgrange/bash_unit.git
        # No need to add to PATH, as the test script sources it directly

    - name: Install kcov
      uses: shivammathur/setup-kcov@v1

    - name: Run tests with coverage
      run: |
        kcov --include-pattern=new.sh --exclude-pattern=new_test.sh coverage new_test.sh

    - name: Upload coverage report
      uses: actions/upload-artifact@v3
      with:
        name: coverage-report
        path: coverage

    - name: Upload coverage to Codecov
      uses: codecov/codecov-action@v3
      with:
        token: ${{ secrets.CODECOV_TOKEN }} # Optional: Add this if you use Codecov
        directory: ./coverage
        fail_ci_if_error: true
        verbose: true

Copy link
Contributor Author

@sambacha sambacha left a comment

Choose a reason for hiding this comment

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

🍒

@grandizzy
Copy link
Collaborator

@sambacha I am testing it locally and I'd expect foundryup -i -l to fail with invalid combination err message, but it doesn't and tries to install -l version instead, is that something that should have been prevented by the PR? thank you

@jenpaff jenpaff assigned sambacha and unassigned grandizzy and zerosnacks Apr 9, 2025
@grandizzy
Copy link
Collaborator

hey @sambacha ping on comment above. thank you!

@jenpaff jenpaff removed this from the v1.1.0 milestone Apr 15, 2025
@jenpaff
Copy link
Collaborator

jenpaff commented Apr 15, 2025

@sambacha will you be able to pick up this PR or do you want us to take over?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready For Review
Development

Successfully merging this pull request may close these issues.

4 participants