TDD a CLI Caching Script - Part Four - GitHub Actions, Linux-compatibility, and shellcheck

2020-01-25

This is part four in a series about writing a general-purpose script to cache CLI output. Up to this point, our script has only worked in OS X. In this post, we'll add Linux compatibility, show how you can setup a GitHub action for running your bats tests, and talk about shellcheck for linting shell scripts.

GitHub Action / Linux compatibility

We've been TDD-ing our cache script development with bats tests, but they're only running locally and manually. I wanted to setup a GitHub Action to run the tests automatically on every push. I didn't know anything about GitHub Actions so this took awhile to get right.

Here's the final .github/workflows/tests.yml file:

name: CI

on: [push]

jobs:
  build:
    runs-on: ubuntu-latest

    steps:
      - name: Check out code
        uses: actions/checkout@v1

      - name: Setup bats
        run: git clone https://github.com/sstephenson/bats.git

      - name: Test
        run: CACHE_DIR="$GITHUB_WORKSPACE/" ./bats/bin/bats test

This is mostly self-explanatory, but making the tests pass required some changes to the tests and cache script for Linux compatibility (since the Action is running in Ubuntu).

Here's the full diff. The changes break down into three types:

  1. Quote every variable. I was getting [: too many arguments errors on lines like [ $output = "flounder - fish" ] because of the $output variable has spaces in it.
  2. Allow configuring the $CACHE_DIR in the tests because I wasn't able to write to $TMPDIR in the Action.
  3. GNU stat and grep behave differently than their OS X (BSD) counterparts. stat needs different arguments and grep --help exits with a different status code and has different copy.

With these small tweaks, the cache script now works fine in Linux and our tests pass in our GitHub Action (example).

shellcheck

Quoting variables made our script more compatible. What other best practices are we missing? How can we learn more? shellcheck is a fantastic utility for linting and finding bugs in your shell scripts.

Running shellcheck cache alerts us to a number of issues with our script. The most prevalent issue is SC2086: Double quote to prevent globbing and word splitting. At the bottom of the output there's a link to SC2086 on the shellcheck wiki to learn more. This is the same issue we saw above where we needed to quote variables in our test file.

Shellcheck also alerts us to two best practices $/${} is unnecessary on arithmetic variables and Don't quote rhs of =~, it'll match literally rather than as a regex.

Linters give us a common language to talk about issues/best-practices and further reading to make the potential problems clear and to explain the alternatives in a user-friendly way. Instead of internalizing the naive "quote everything, I guess" we can refine our understanding to get to the details of why.

Like most linters, you can add magic comments to ignore lines you don't want to change. In this case, we like all the suggestions, so we can use shellcheck cache -f diff | patch -p1 cache to have shellcheck generate a patch that we immediately pass to patch to apply the fixes to our script.

Not all issues can be auto-fixed. We have to clean up $/${} is unnecessary on arithmetic variables manually, but that's trivial to do.

As a neat bonus, you can run shellcheck against .bats files to find potential issues in your bats tests.

Here's the diff that applies all the shellcheck suggestions to our script and tests.


TDD a CLI Caching Script - Part Three - Help

This is part three in a series about writing a general-purpose script to cache CLI output. In part two we added support for a TTL option and specifying acceptable status codes to cache.

In this post, we'll add support for --help and improve our option parsing.

--help

We're already parsing options in our script, so responding to --help isn't much work.

What should --help do? It should give basic usage instructions and document each option our script can take. We can write a simple test to that end.

@test "documents options with --help" {
  run ./cache --help
  [ "$status" -eq 0 ]
  echo $output | grep -- --ttl
  echo $output | grep -- --cache-status
  echo $output | grep -- --help
}

We're not making any specific claims about what the help documentation tells us about each option. We're only asserting that every option is mentioned (including --help itself).

The double-dash after grep is important in these assertions. In grep -- --ttl, the -- tells grep we're not trying to pass --ttl as an option, but that it should be interpreted as a positional argument (in this case, the thing we want to search for). There's an implication for our cache script here too that we'll address shortly.

In the meantime, let's make this test pass by responding to --help.

We'll add a usage function after set -e and before we parse our options:

usage() {
    echo "usage: cache [--ttl SECONDS] [--cache-status CACHEABLE-STATUSES] [cache-key] [command] [args for command]"
    echo "  --ttl SECONDS   # Treat previously cached content as fresh if fewer than SECONDS seconds have passed"
    echo "  --cache-status  # Quoted and space-delimited exit statuses for [command] that are acceptable to cache."
    echo "  --help      # show this help documentation"
}

Within our option parsing, we'll add a case for --help to call usage and exit.

        --help)
            usage
            exit 0
            ;;

Here's what the output of cache --help looks like:

usage: cache [--ttl SECONDS] [--cache-status CACHEABLE-STATUSES] [cache-key] [command] [args for command]
        --ttl SECONDS   # Treat previously cached content as fresh if fewer than SECONDS seconds have passed
        --cache-status  # Quoted and space-delimited exit statuses for [command] that are acceptable to cache.
        --help          # show this help documentation

Not bad.

Better positional argument parsing

Remember that bit about -- and positional arguments in the grep usage? Anything after -- is treated as a positional argument even if it looks like an option. Let's think about why that matters. Imagine we're trying to cache the --help output of a command. That may sound silly, but it'll illustrate our point. Consider cache some-cache-key grep --help. Can you guess what the output will be?

If you guessed the --help content for our cache script, you're right. Our cache script sees --help anywhere in the command and says "Ah, I know how to do that!" That's not ideal.

We should support the -- like we see in this updated command: cache -- some-cache-key grep --help

We'll write a test:

@test "stops parsing arguments after --" {
  run ./cache -- $TEST_KEY grep --help
  [ "$status" -eq 2 ]
  echo $output | grep -- "usage: grep"
}

Note the expectation that the status will be 2. That's the code grep --help exists with. It is a little strange, perhaps, since the command didn't exactly fail. But it didn't exactly succeed either. git --help exits with 0. I'm fine with cache --help exiting with 0, but this is an idiosyncrasy worth knowing about.

This test fails as every good initial test should. Our $status is 0 and our output is cache --help.

Fortunately our option-parsing code lends itself well to handling --. We'll add the following case:

        --)
            shift
            positional+=("$@")
            break
            ;;

And we're green. And now we're a better citizen of the command line.

Flexible Parsing and Confusion

Handling the double-dash is the right thing to do. But cache some-cache-key grep --help without the double-dash seems pretty reasonable too. We really only have two types of positional arguments: the cache key, and the command and its arguments.

Passing options before the cache key positional argument like cache --ttl 1 some-cache-key grep --help seems valid. So does passing options after the cache key positional argument like cache some-cache-key --ttl 1 grep --help.

What doesn't make sense is providing options after the command starts. Something like cache some-cache-key grep --ttl 1 --help shouldn't be interpreted as providing a TTL of 1 to our cache script.

We could explicitly stop parsing options after our cache key. We'd essentially be treating this as an implicit --. But is this worth the effort or should we trust the user to do the right thing? That's a classic question, right? You need to weigh the value of protecting against weird input against the complexity the protection adds to the code and the difficulty of implementation. Also there's the nebulous question of how much effort it'll take to support this feature as time marches on.

In this case, I'm learning in my free time, so I'll go the extra mile to see how it pans out.

First let's add a test illustrating how we already parse options before or after the cache key. This test will keep us from accidentally breaking things:

@test "parses options before and after the cache key" {
  # fails because the status isn't allowed by our options
  run ./cache --cache-status "2" $TEST_KEY exit 0
  [ "$status" -eq 0 ]
  [ ! -f "$TMPDIR$TEST_KEY" ]

  # succeeds because the status is allowed by our option before the
  # cache key
  run ./cache --cache-status "0 2" $TEST_KEY exit 2
  [ "$status" -eq 2 ]
  [ -f "$TMPDIR$TEST_KEY" ]

  rm "$TMPDIR$TEST_KEY"

  # succeeds because the status is allowed by our option after the
  # cache key
  run ./cache $TEST_KEY --cache-status "0 2" exit 2
  [ "$status" -eq 2 ]
  [ -f "$TMPDIR$TEST_KEY" ]
}

That already passes. Now let's add a failing test.

@test "stops parsing options after the command starts" {
  run ./cache $TEST_KEY echo --ttl 1 --help
  [ "$status" -eq 0 ]
  [ $output = "--ttl 1 --help" ]
}

This fails because we're still parsing --ttl 1 and --help after the command (echo) starts. That means that $output is the result of cache --help.

We'll update the default branch of our case statement.

        *) # default
            if [ -z "$cache_key" ]; then
                cache_key=$1
                shift
            else
                break;
            fi
            ;;

If the $cache_key isn't set, we set it and remove it from the argument list. If it is set, we break because we only want to parse one positional argument (for the cache key) and treat all remaining arguments as the provided command and its args.

Next we'll replace the -- branch of the case statement with

        --)
            cache_key=$2
            shift # drop the --
            shift # drop the cache key
            break
            ;;

We can now remove these lines after the case statement

set -- "${positional[@]}" # restore positional parameters
cache_key=$1
shift

and also remove any other references the positional variable since we are no longer using it.

 ✓ initial run is uncached
 ✓ works for quoted arguments
 ✓ preserves the status code of the original command
 ✓ subsequent runs are cached
 ✓ respects a TTL
 ✓ only caches 0 exit status by default
 ✓ allows specifying exit statuses to cache
 ✓ allows specifying * to allow caching all statuses
 ✓ returns the cached exit status
 ✓ documents options with --help
 ✓ stops parsing arguments after --
 ✓ parses options before and after the cache key
 ✓ stops parsing options after the command starts

13 tests, 0 failures

All this work might have you questioning if supporting positional arguments is worth it. We could require the cache key to be provided as --key. This is an ergonomics versus aesthetic argument that ultimately comes down to opinion. Do whatever you feel is best and document accordingly.

Closing

You might be asking: --help is nice, but what about writing a man page? I haven't taken the plunge here yet, but if I were going to try it, I'd probably lean on (the always wonderful) pandoc. Search for "Man page" on their demos.

Here's the diff for adding --help and -- support and the diff for not parsing options after the command starts.

Stay tuned for our next post where we'll take a quick look at a shell script linter that can help us avoid errors and improve code quality.


TDD a CLI Caching Script - Part Two - Option Parsing, TTL, and Exit Codes

This is part two in a series about writing a general-purpose script to cache CLI output. In part one we wrote our first iteration of the script. It works for permanently caching content.

In this post, we'll add support for a TTL to specify when the content is no longer fresh. We'll also update the script to only cache successful runs of the provided command.

Let's parse some options

Our cache script uses positional arguments. Positional arguments can be great when they're few but as the number of arguments grows, they can lead to ambiguity in parsing and confusion for the user.

We'll support the TTL as an option specified by --ttl NUMBER_OF_SECONDS. Example usage:

cache --ttl 90 cache-key ping -c 3 google.com

After the first run, this will return cached content for any subsequent runs in 90 seconds.

We write a naive test for the TTL cache.

@test "respects a TTL" {
  run ./cache --ttl 1 $TEST_KEY echo initial-value
  [ "$status" -eq 0 ]
  [ $output = "initial-value" ]

  run ./cache --ttl 1 $TEST_KEY echo new-value
  [ "$status" -eq 0 ]
  [ $output = "initial-value" ]

  sleep 1

  run ./cache --ttl 1 $TEST_KEY echo third-value
  [ "$status" -eq 0 ]
  [ $output = "third-value" ]
}

This caches an initial-value with a TTL of 1 second and then immediately tries to run for the same cache-key within that window. Because the TTL hasn't expired, the cached initial-value is returned. Next we sleep 1 second and the TTL has expired. The cached content is no longer valid so we execute our echo command, third-value is returned, and third-value is now cached.

(I don't love sleeping in tests, but this is the easiest way to test this.)

The test fails because our exit status is 127. That's "command not found." The problem here is that our positional arguments mean we're parsing --ttl as the cache key. Then we're trying to execute 1 cache-tests-key echo initial-value as our command. That won't work.

Worse, our invalid argument parsing means we've left behind a temporary file our script doesn't know how to clean up. rm $TMPDIR/--ttl puts us back in a clean state.

There's a number of ways to parse options in bash scripts. getopts is compelling but it only allows for single character flags and that feels a little restrictive. I could brew install gnu-getopt to get a getopt that supports long flag names, but the usage is a little opaque.

We'll go with a straightforward while [[ $# -gt 0 ]] with a case statement as suggested here.

We modify our cache script to look like this

#!/usr/bin/env bash

set -e

positional=()
while [[ $# -gt 0 ]]
do
    key="$1"

    case $key in
        --ttl)
            ttl="$2"
            shift # drop the key
            shift # drop the value
            ;;
        *)    # default
            positional+=("$1") # save it in an array for later
            shift # drop the argument
            ;;
    esac
done

set -- "${positional[@]}" # restore positional parameters
cache_key=$1
shift

cache_dir=${CACHE_DIR:-$TMPDIR}
cache_file="$cache_dir$cache_key"

if test -f $cache_file; then
    cat $cache_file
else
    "$@" | tee $cache_file
    exit ${PIPESTATUS[0]}
fi

We're draining our arguments (with shift) until none remain. If we find the --ttl we extract it. Otherwise we keep the positional arguments and set those as our argument list ($@).

bats test now shows the test now fails because the initial-value remains even after the content should have expired. That's because we haven't implemented cache expiration yet.

Cache expiration

Let's list the circumstances where a cache hit is valid:

  1. The cache file exists and no TTL is specified
  2. The cache file exists and the TTL has not expired

How do we tell if the TTL is expired? When we write the cache file, we're implicitly keeping track of the timestamp when the content was cached in the modified timestamp attribute of the cache file. If the current time is less than the modified timestamp + our TTL seconds, then the cache is still valid. Otherwise it is expired.

We can use stat to get the modified time of a file and date to get the system time. stat -f %m <FILENAME> and date +%s both return an epoch time which makes for easy integer math.

Let's update our concept of freshness to consider the TTL:

We'll replace

if test -f $cache_file; then

with

fresh () {
    # if the $cache_file doesn't exist, it can't be fresh
    if [ ! -f $cache_file ]; then
        return 1
    fi

    # if we don't have a ttl specifed, our $cache_file is
    # fresh-enough
    if [ -z "$ttl" ]; then
        return 0
    fi

    # if a ttl is specified, we need to check the last modified
    # timestamp on the $cache_file
    mtime=$(stat -f %m $cache_file)
    now=$(date +%s)
    remaining_time=$(($now - $mtime))

    if [ $remaining_time -lt $ttl ]; then
        return 0
    fi

    return 1
}

if fresh; then

In some languages, zero is falsy and non-zero numbers are truthy. But in shell scripts, as you've seen in exit statuses, zero indicates success and non-zero indicates failure. Writing our function to be consistent with other expected values means we can easily swap in our function for the previous test conditional.

Our fresh function returns a truthy value if the cache file exists and either no TTL is specified or the TTL hasn't passed.

Running bats test shows everything passing:

 ✓ initial run is uncached
 ✓ works for quoted arguments
 ✓ preserves the status code of the original command
 ✓ subsequent runs are cached
 ✓ respects a TTL

5 tests, 0 failures

Caching successful runs only

Our current script caches regardless of the exit status of the provided command. In some cases, this is what you want because the status won't change on subsequent runs. But if the command can fail intermittently or the status can change over time, then you might only want to cache certain statuses.

hub ci-status, for example, can return an exit status of 2 while a GitHub Check is pending. Neither the hub ci-status command nor the GitHub check has failed. A subsequent run could exit with another 2 if it is still pending or exit with a 0 or a 1 for the final outcome.

It feels like a good default is to only cache on zero exit statuses. This default is least likely to unintentionally cache bad content. We can then add an option for specifying other acceptable statuses to cache.

We'll add an initial test:

@test "only caches 0 exit status by default" {
  run ./cache $TEST_KEY exit 1
  [ "$status" -eq 1 ]
  [ ! -f "$TMPDIR$TEST_KEY" ]

  run ./cache $TEST_KEY exit 0
  [ "$status" -eq 0 ]
  [ -f "$TMPDIR$TEST_KEY" ]
}

This fails since we cache everything. We'll update our script to make this pass by replacing

    "$@" | tee $cache_file
    exit ${PIPESTATUS[0]}

with

    "$@" | tee $cache_file
    status=${PIPESTATUS[0]}

    if [[ $status -ne 0 ]]; then
        rm $cache_file
    fi

    exit $status

We're actually still writing the cache file, we're just deleting it immediately if the exit status wasn't valid. This could be optimized for a conditional write, but the ease of using tee here makes this conditional rm acceptable to me.

That test passes, so let's add a new test for specifying acceptable status codes. We're going to pass these in with --cache-status followed by space-separated statuses we want to allow to be cached. We'll need to quote the statuses if there's more than one.

@test "allows specifying exit statuses to cache" {
  run ./cache --cache-status "1 2" $TEST_KEY exit 0
  [ "$status" -eq 0 ]
  [ ! -f "$TMPDIR$TEST_KEY" ]

  run ./cache --cache-status "1 2" $TEST_KEY exit 1
  [ "$status" -eq 1 ]
  [ -f "$TMPDIR$TEST_KEY" ]

  rm "$TMPDIR$TEST_KEY"

  run ./cache --cache-status "1 2" $TEST_KEY exit 2
  [ "$status" -eq 2 ]
  [ -f "$TMPDIR$TEST_KEY" ]
}

This initially fails because we aren't parsing our option yet. We'll update our case statement to look like this:

    case $key in
        --ttl)
            ttl="$2"
            shift # drop the key
            shift # drop the value
            ;;
        --cache-status)
            acceptable_statuses="$2"
            shift # drop the key
            shift # drop the value
            ;;
        *)    # default
            positional+=("$1") # save it in an array for later
            shift # drop the argument
            ;;
    esac

Now our test fails because we're not considering the acceptable_statuses. We can replace the line

if [[ $status -ne 0 ]]; then

with

    acceptable_statuses=${acceptable_statuses:-0}
    if [[ ! " $acceptable_statuses " =~ " $status " ]]; then

to make this pass. We're checking whether the actual exit status is in our list of acceptable_statuses (which is 0 by default). If not, we remove the $cache_file.

Finally, it would be nice to allow caching any status with --cache-status "*" so let's support that.

The test:

@test "allows specifying * to allow caching all statuses" {
  run ./cache --cache-status "*" $TEST_KEY exit 3
  [ "$status" -eq 3 ]
  [ -f "$TMPDIR$TEST_KEY" ]
}

And we'll tweak the line

    if [[ ! " $acceptable_statuses " =~ " $status " ]]; then

to be

    if [[ $acceptable_statuses != "*" ]] && [[ ! " $acceptable_statuses " =~ " $status " ]]; then

And the test passes.

Returning the exit status of the cached command

That all works, but it is a little unfortunate that if we have a cache hit, we're always exiting with a status of 0. If we cache a command that exited with status of 2 and that 2 is meaningful, it would be better to return that original exit status.

We write a test:

@test "returns the cached exit status" {
  run ./cache --cache-status "*" $TEST_KEY exit 3
  [ "$status" -eq 3 ]

  run ./cache --cache-status "*" $TEST_KEY exit 9
  [ "$status" -eq 3 ]
}

That fails because we exit with the zero status on cache hits. Let's update the code. We'll add an else clause to our acceptable status check to persist the status code of the cached run:

    if [[ $acceptable_statuses != "*" ]] && [[ ! " $acceptable_statuses " =~ " $status " ]]; then
        rm $cache_file
    else
        echo $status > "$cache_file.cache-status"
    fi

Next we'll update the if fresh check to read that cached status:

if fresh; then
    status=$(cat "$cache_file.cache-status")
    cat $cache_file

Finally, we'll move our existing exit $status line to the bottom of the file since we always are exiting with a specific status now.

Closing

We've added a TTL and made our script smarter about what exit statuses are able to be cached. We're also returning the cached exit status now.

Here's the diff for adding the TTL and the diff for better handling status codes.

Because the command to run is only executed if the TTL has expired, you can use the cache script as a throttle/rate-limiter. The command cache --ttl 90 cache-key say "90 seconds have passed" run in a tight loop will only speak every 90 seconds.

Now that we're accepting options, we should probably respond to --help. We'll address that and more in part three.

My goofy face

Hi, I'm Jeffrey Chupp.
I solve problems, often with code.