-
Notifications
You must be signed in to change notification settings - Fork 0
Fix artifact download: handle .pkg extension in artifact names and add shellcheck infrastructure #11
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
Fix artifact download: handle .pkg extension in artifact names and add shellcheck infrastructure #11
Changes from all commits
60f1bea
980a135
098c765
c143f7a
5a5e5b6
db5e363
799fc22
1c53a49
e6d4ab2
25ec8b7
67545c0
10b0c09
e202cdd
de11c40
bbcd6c0
01e356c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| --- | ||
| name: ShellCheck | ||
|
|
||
| 'on': | ||
| push: | ||
| branches: | ||
| - main | ||
| - 'copilot/**' | ||
| paths: | ||
| - 'scripts/**/*.sh' | ||
| - '.shellcheckrc' | ||
| - '.github/workflows/shellcheck.yml' | ||
| pull_request: | ||
| paths: | ||
| - 'scripts/**/*.sh' | ||
| - '.shellcheckrc' | ||
| - '.github/workflows/shellcheck.yml' | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| shellcheck: | ||
| name: ShellCheck Analysis | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Run ShellCheck | ||
| uses: ludeeus/action-shellcheck@master | ||
| with: | ||
| scandir: './scripts' | ||
| severity: warning | ||
| check_together: 'yes' | ||
| env: | ||
| SHELLCHECK_OPTS: --shell=bash | ||
|
|
||
| - name: Verify .shellcheckrc is used | ||
| run: | | ||
| echo "Verifying shellcheck configuration..." | ||
| shellcheck --version | ||
| [ -f .shellcheckrc ] && echo "✓ .shellcheckrc found" || echo "✗ .shellcheckrc not found" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| --- | ||
| name: Test Scripts | ||
|
|
||
| 'on': | ||
| push: | ||
| branches: | ||
| - main | ||
| - 'copilot/**' | ||
| paths: | ||
| - 'scripts/**' | ||
| - 'tests/**' | ||
| - '.github/workflows/test-scripts.yml' | ||
| pull_request: | ||
| paths: | ||
| - 'scripts/**' | ||
| - 'tests/**' | ||
| - '.github/workflows/test-scripts.yml' | ||
| workflow_dispatch: # Allow manual triggering for testing | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| shellcheck: | ||
| name: Lint Shell Scripts | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Run ShellCheck | ||
| uses: ludeeus/action-shellcheck@master | ||
| with: | ||
| scandir: './scripts' | ||
| severity: warning | ||
| check_together: 'yes' | ||
| env: | ||
| SHELLCHECK_OPTS: --shell=bash | ||
|
|
||
| test: | ||
| name: Run Tests | ||
| runs-on: ubuntu-latest | ||
| needs: shellcheck | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Install bats | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y bats | ||
|
|
||
| - name: Run bats tests | ||
| run: | | ||
| bats tests/download-mozc-artifacts.bats | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # ShellCheck configuration for brew_mozc | ||
| # See https://github.com/koalaman/shellcheck/wiki for more information | ||
|
|
||
| # Disable SC1091: Not following sourced files | ||
| # We don't have external dependencies to source | ||
| disable=SC1091 | ||
|
|
||
| # Set shell dialect to bash | ||
| shell=bash | ||
|
|
||
| # Don't enable all optional checks - focus on errors and warnings | ||
| # Style-level suggestions can be addressed incrementally | ||
| enable=error,warning | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -232,7 +232,7 @@ TAG_NAME=$(gh api \ | |
|
|
||
| if [ -n "$TAG_NAME" ] && [ "$TAG_NAME" != "null" ]; then | ||
| # Use the tag name as version (remove any 'v' prefix if present) | ||
| VERSION=$(echo "$TAG_NAME" | sed 's/^v//') | ||
| VERSION="${TAG_NAME#v}" | ||
| echo "Found tag: $TAG_NAME, using version: $VERSION" | ||
| else | ||
| # Fallback: extract version from commit message | ||
|
|
@@ -310,6 +310,7 @@ if [ "$DRY_RUN" = true ]; then | |
| echo "==> Processing and validating artifacts" | ||
| # Process each artifact directory to validate .pkg files | ||
| # Each Mozc_* directory should contain a Mozc.pkg file | ||
| # Note: Artifact directories from google/mozc already have .pkg in their names (e.g., Mozc_arm64.pkg/) | ||
| found_pkg=false | ||
| for artifact_dir in Mozc_*/; do | ||
| if [ -d "$artifact_dir" ]; then | ||
|
|
@@ -327,7 +328,13 @@ if [ "$DRY_RUN" = true ]; then | |
|
|
||
| # Get file size (wc -c with input redirection produces clean output) | ||
| size=$(wc -c < "$artifact_dir/Mozc.pkg" 2>/dev/null || echo "unknown") | ||
| echo " ✓ Found valid .pkg file: $artifact_name/Mozc.pkg (${size} bytes)" | ||
| # Determine expected final filename | ||
| if [[ "$artifact_name" == *.pkg ]]; then | ||
| final_name="$artifact_name" | ||
| else | ||
| final_name="${artifact_name}.pkg" | ||
| fi | ||
| echo " ✓ Found valid .pkg file: $artifact_name/Mozc.pkg -> $final_name (${size} bytes)" | ||
| found_pkg=true | ||
| else | ||
| echo "Error: Expected Mozc.pkg not found in $artifact_dir" | ||
|
|
@@ -375,17 +382,33 @@ else | |
| fi | ||
|
|
||
| # Process downloaded artifacts | ||
| # The gh run download command creates subdirectories for each artifact (e.g., Mozc_arm64/) | ||
| # The gh run download command creates subdirectories for each artifact | ||
| # Artifact names from google/mozc already include .pkg extension (e.g., Mozc_arm64.pkg/) | ||
| # We need to move the Mozc.pkg files from these subdirectories to the root with proper names | ||
| echo "==> Processing downloaded artifacts" | ||
| for artifact_dir in Mozc_*/; do | ||
| if [ -d "$artifact_dir" ]; then | ||
| artifact_name="${artifact_dir%/}" # Remove trailing slash to get clean name | ||
| if [ -f "$artifact_dir/Mozc.pkg" ]; then | ||
| echo "Processing artifact: $artifact_name" | ||
| # Move Mozc.pkg to artifact name with .pkg extension (e.g., Mozc_arm64/Mozc.pkg -> Mozc_arm64.pkg) | ||
| mv "$artifact_dir/Mozc.pkg" "${artifact_name}.pkg" | ||
| rmdir "$artifact_dir" # Remove now-empty directory | ||
| # Check if artifact name already ends with .pkg to avoid double extension | ||
| if [[ "$artifact_name" == *.pkg ]]; then | ||
| # Artifact name already has .pkg extension (e.g., Mozc_arm64.pkg) | ||
| # Since the directory name matches the target filename, we need a temporary rename: | ||
| # Use mktemp to create a unique temporary file atomically for safe handling | ||
| tmpfile=$(mktemp) | ||
| # Copy content to the temporary file using cp for better performance | ||
| cp "$artifact_dir/Mozc.pkg" "$tmpfile" | ||
| rm "$artifact_dir/Mozc.pkg" | ||
| rmdir "$artifact_dir" | ||
| # Move temp file to final name | ||
| mv "$tmpfile" "$artifact_name" | ||
| else | ||
| # Legacy case: artifact name doesn't have .pkg extension | ||
| # Add .pkg extension when moving (e.g., Mozc_arm64 -> Mozc_arm64.pkg) | ||
| mv "$artifact_dir/Mozc.pkg" "${artifact_name}.pkg" | ||
| rmdir "$artifact_dir" # Remove now-empty directory | ||
| fi | ||
| else | ||
| echo "Error: Expected Mozc.pkg not found in $artifact_dir" | ||
| echo "Contents of $artifact_dir:" | ||
|
|
@@ -403,6 +426,48 @@ else | |
| exit 1 | ||
| fi | ||
|
|
||
| # Additional check: verify specific expected files exist | ||
| echo "" | ||
| echo "==> Verifying expected artifact files" | ||
| expected_files=("Mozc_arm64.pkg" "Mozc_intel64.pkg" "Mozc_universal_binary.pkg") | ||
| missing_files=() | ||
|
|
||
| for expected_file in "${expected_files[@]}"; do | ||
| if [ ! -f "$expected_file" ]; then | ||
| missing_files+=("$expected_file") | ||
| echo " ✗ Missing: $expected_file" | ||
| else | ||
| size=$(wc -c < "$expected_file" 2>/dev/null || echo "unknown") | ||
| echo " ✓ Found: $expected_file (${size} bytes)" | ||
| fi | ||
| done | ||
|
|
||
| # Warn if any expected files are missing, but don't fail | ||
| # (google/mozc might change their artifact structure in the future) | ||
| if [ "${#missing_files[@]}" -gt 0 ]; then | ||
| echo "" | ||
| echo "Warning: Some expected artifacts are missing:" | ||
| for missing_file in "${missing_files[@]}"; do | ||
| echo " - $missing_file" | ||
|
||
| done | ||
| echo "" | ||
| echo "Available artifacts:" | ||
| if compgen -G "*.pkg" > /dev/null; then | ||
| for f in *.pkg; do | ||
| # Use portable approach to display file info | ||
| if [ -f "$f" ]; then | ||
| size=$(wc -c < "$f" 2>/dev/null || echo "unknown") | ||
| printf " - %s (%s bytes)\n" "$f" "$size" | ||
| fi | ||
| done | ||
| else | ||
| echo " No .pkg files found!" | ||
| fi | ||
| echo "" | ||
| echo "This might indicate a change in google/mozc artifact naming." | ||
| echo "Continuing with available artifacts..." | ||
| fi | ||
|
|
||
| cd .. | ||
| fi | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| # Tests for brew_mozc | ||
|
|
||
| This directory contains tests for the brew_mozc scripts. | ||
|
|
||
| ## Linting | ||
|
|
||
| We use [ShellCheck](https://github.com/koalaman/shellcheck) for linting shell scripts. | ||
|
|
||
| ### Running ShellCheck Locally | ||
|
|
||
| ```bash | ||
| # Install shellcheck | ||
| # macOS | ||
| brew install shellcheck | ||
|
|
||
| # Ubuntu/Debian | ||
| sudo apt-get install shellcheck | ||
|
|
||
| # Run shellcheck on all scripts | ||
| shellcheck scripts/*.sh | ||
|
|
||
| # ShellCheck will automatically use the .shellcheckrc configuration file | ||
| ``` | ||
|
|
||
| ### ShellCheck Configuration | ||
|
|
||
| The `.shellcheckrc` file in the repository root configures ShellCheck behavior: | ||
| - Sets shell dialect to bash | ||
| - Enables all optional checks | ||
| - Disables SC1091 (not following sourced files) | ||
|
|
||
| ## Test Framework | ||
|
|
||
| We use [Bats (Bash Automated Testing System)](https://github.com/bats-core/bats-core) for testing shell scripts. | ||
|
|
||
| ## Installation | ||
|
|
||
| ### macOS | ||
| ```bash | ||
| brew install bats-core shellcheck | ||
| ``` | ||
|
|
||
| ### Ubuntu/Debian | ||
| ```bash | ||
| sudo apt-get install bats shellcheck | ||
| ``` | ||
|
|
||
| ### Manual Installation | ||
| ```bash | ||
| git clone https://github.com/bats-core/bats-core.git | ||
| cd bats-core | ||
| ./install.sh /usr/local | ||
| ``` | ||
|
|
||
| ## Running Tests | ||
|
|
||
| Run all tests: | ||
| ```bash | ||
| bats tests/ | ||
| ``` | ||
|
|
||
| Run a specific test file: | ||
| ```bash | ||
| bats tests/download-mozc-artifacts.bats | ||
| ``` | ||
|
|
||
| Run tests with verbose output: | ||
| ```bash | ||
| bats -t tests/download-mozc-artifacts.bats | ||
| ``` | ||
|
|
||
| ## Test Coverage | ||
|
|
||
| ### download-mozc-artifacts.bats | ||
|
|
||
| Tests for the `scripts/download-mozc-artifacts.sh` script: | ||
|
|
||
| - **Artifact processing with .pkg extension**: Validates that artifact directories named `Mozc_arm64.pkg/` are correctly processed to create `Mozc_arm64.pkg` files | ||
| - **Legacy artifact processing**: Tests backward compatibility with artifact directories without `.pkg` extension (e.g., `Mozc_arm64/`) | ||
| - **Multiple artifacts**: Verifies that multiple artifacts (arm64, intel64, universal_binary) are processed correctly | ||
| - **Temporary file handling**: Tests that temporary files use mktemp's XXXXXX random suffix pattern for uniqueness and don't conflict | ||
| - **Glob pattern matching**: Validates `compgen -G` correctly detects `.pkg` files | ||
| - **File listing**: Tests the portable file listing approach | ||
| - **Version extraction**: Validates version string extraction with parameter expansion | ||
| - **Artifact validation**: Tests basic pkg file validation logic | ||
|
|
||
| ## Writing New Tests | ||
|
|
||
| When adding new tests: | ||
|
|
||
| 1. Create a new `.bats` file in the `tests/` directory | ||
| 2. Use `setup()` and `teardown()` functions for test environment preparation and cleanup | ||
| 3. Name test cases descriptively using `@test "description"` | ||
| 4. Use assertions: `[ condition ]`, `[[ pattern ]]`, `run command` | ||
| 5. Add documentation to this README | ||
|
|
||
| ## Continuous Integration | ||
|
|
||
| Both linting and tests are automatically run in GitHub Actions on every push and pull request that modifies scripts or tests. | ||
|
|
||
| ### ShellCheck Workflow | ||
|
|
||
| The `.github/workflows/shellcheck.yml` workflow provides dedicated shell script linting: | ||
|
|
||
| - **Triggers**: | ||
| - Push to `main` or `copilot/**` branches | ||
| - Pull requests that modify `scripts/`, `.shellcheckrc`, or the workflow file | ||
| - Manual workflow dispatch | ||
|
|
||
| - **Features**: | ||
| - Uses the `ludeeus/action-shellcheck` GitHub Action | ||
| - Scans all scripts in `scripts/` directory | ||
| - Respects `.shellcheckrc` configuration | ||
| - Checks for style issues and best practices | ||
|
|
||
| ### Test Scripts Workflow | ||
|
|
||
| The `.github/workflows/test-scripts.yml` workflow runs linting and tests: | ||
|
|
||
| - **Triggers**: | ||
| - Push to `main` or `copilot/**` branches | ||
| - Pull requests that modify `scripts/`, `tests/`, or the workflow file itself | ||
| - Manual workflow dispatch | ||
|
|
||
| - **Steps**: | ||
| 1. **Lint Shell Scripts** (separate job): | ||
| - Checkout repository | ||
| - Run ShellCheck using GitHub Action | ||
| - Verify `.shellcheckrc` configuration | ||
| 2. **Run Tests** (depends on linting passing): | ||
| - Checkout repository | ||
| - Install bats | ||
| - Run all bats tests | ||
|
|
||
| ### Manual Workflow Execution | ||
|
|
||
| You can manually trigger the test workflow from the GitHub Actions tab or using the GitHub CLI: | ||
|
|
||
| ```bash | ||
| gh workflow run test-scripts.yml | ||
| ``` | ||
|
|
||
| ### Adding Tests to Other Workflows | ||
|
|
||
| To integrate tests into other workflows: | ||
|
|
||
| ```yaml | ||
| - name: Install Bats | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y bats | ||
|
|
||
| - name: Run tests | ||
| run: | | ||
| bats tests/download-mozc-artifacts.bats | ||
| ``` |
Uh oh!
There was an error while loading. Please reload this page.