Friday, August 21, 2015

Creating the Packager DSL - The CLI

  1. Why use a DSL?
  2. Why create your own DSL?
  3. What makes a good DSL?
  4. Creating your own DSL - Parsing
  5. Creating your own DSL - Parsing (with Ruby)
  6. Creating the Packager DSL - Initial steps
  7. Creating the Packager DSL - First feature
  8. Creating the Packager DSL - The executor
  9. Creating the Packager DSL - The CLI
Our user story:
I want to run a script, passing in the name of my DSL file. This should create an empty package by specifying the name, version, and package format. If any of them are missing, print an error message and stop. Otherwise, an empty package of the requested format should be created in the directory I am in.
Our progress:

  • We can parse the DSL into a Struct. We can handle name, version, and package format. If any of them are missing, we raise an appropriate error message.
  • We can create a package from the parsed DSL
We still need to:
  • Provide a script that executes everything
Writing this script, on its face, looks pretty easy. We need to:
  1. Receive the filename from the commandline arguments
  2. Pass the contents of that filename to Packager::DSL.parse_dsl()
  3. Pass the contents of that to Packager::Executor
A rough version (that works) could look like:

#!/usr/bin/env ruby

require 'packager/dsl'
require 'packager/executor'

filename = ARGV[0]
items = Packager::DSL.parse_dsl(IO.read(filename))
Packager::Executor.new.create_package(items)

You can create a file with a package declaration (such as the one in our spec for the DSL) and pass it to this script and you will have an empty package created. All done, right?

Not quite.

The first problem is testing executables is hard. Unlike classes and objects which live in the same process, the testing boundary of a script is a process boundary. Process boundaries are much harder to work with. Objects can be invoked and inspected over and over, in any order. Invocations of an executable are one-shot. Outside the results, there's nothing to inspect once you've invoked the script. If we could minimize the script and move most of the logic into some objects, that would make testing it so much easier. And, we can measure our code coverage of it.

The second (and bigger) problem is writing good executables is hard. Very very hard. Good executables have options, error-handling, and all sorts of other best practices. It is nearly impossible to write a good executable that handles all the things, even if you're an expert.

Again, the good Ruby opensource developers have provided a solution - Thor. With Thor, we can move all the logic into a Packager::CLI class and our executable in bin/packager becomes

#!/usr/bin/env ruby

$:.unshift File.expand_path('../../lib', __FILE__)

require 'rubygems' unless defined? Gem
require 'packager/cli'

Packager::CLI.start

Almost all of that is cribbed from other Ruby projects, meaning we can directly benefit from their experience. The executable is now 8 lines (including whitespace). We can visibly inspect this and be extremely certain of its correctness. Which is good because we really don't want to have to test it. The actual CLI functionality moves into classes and objects, things we can easily test.

First thing first - we need a test. Thor scripts tend to function very similarly to git, with invocations of the form "<script> <command> <flags> <parameters>". So, in our case, we're going to want "packager create <DSL filenames>". This translates into the #create method on the Packager::CLI class. The filenames will be passed in as the arguments to the #create method. We don't have any flags, so we'll skip that part (for now).

A note on organization - we have tests for the DSL, the Executor, and now the CLI. We can see wanting to write many more tests for each of those categories as we add more features, so let's take the time right now to reorganize our spec/ directory. RSpec will recursively descend into subdirectories, so we can create spec/dsl, spec/executor, and spec/cli directories. git mv the existing DSL and Executor specs into the appropriate directories (renaming them to be more meaningful), run RSpec to make sure everything is still found, then commit the change. You can pass rspec the name of a file or a directory, if you want to run just a subset of the tests. So, if you're adding just a DSL piece, you can run those tests to make sure they pass without having to do the entire thing.

Back to the new CLI test. The scaffolding for this looks like

describe Packager::CLI do
    subject(:cli) { Packager::CLI.new }

    describe '#create' do
    end
end

The nested describe works exactly as you'd expect. (RSpec provides many ways to organize things, letting you choose which works best for the situation at hand.)

The first test, as always, is the null test. What happens if we don't provide any filenames? Our script should probably print something and stop, ideally setting the exit code to something non-zero. In Thor, the way to do that is to raise Thor::Error, "Error string". (I wish they'd call that Thor::Hammer, but you can't have everything.) So, the first test should expect the error is raised.

    it "raises an error with no filenames" do
        expect {
            cli.create
        }.to raise_error(Thor::Error, "No filenames provided for create")
    end

Run that, see it fail, then let's create packager/cli.rb to look like

class Packager
    class CLI < Thor
        def create()
            raise Thor::Error, "No filenames passed for create"
        end
    end
end

Again, we're writing just enough code to make the tests pass. Now, let's pass in a filename to #create. Except, where do we get the file?

One possibility is to create a file with what we want, save it somewhere inside spec/, add it to the project, then reference that filename as needed.

There are a few subtle problems with that approach.

  1. The file contents are separated from the test(s) using them.
  2. These files have to be packaged with the gem in order for client-side tests to work.
  3. There could be permission issues with writing to files inside the installation directory.
  4. Developers end up wanting to keep the number of these files small, so shoehorn as many possible cases within each file as possible.
Fortunately, there's a much better approach. Ruby, like most languages, has a library for creating and managing tempfiles. Ruby's is called Tempfile. Adding this to our test file results in

require 'tempfile'

describe Packager::CLI do
    subject(:cli) { Packager::CLI.new }
    let(:package_file) { Tempfile.new('foo').path }

    def append_to_file(filename, contents)
        File.open(filename, 'a+') do |f|
            f.write(content)
            f.flush
        end
    end

    describe '#create' do
        it "raises an error with no filenames" do
            expect {
                cli.create
            }.to raise_error(Thor::Error, "No filenames provided for create")
        end

        it "creates a package with a filename" do
            append_to_file(package_file, "
                package {
                    name 'foo'
                    version '0.0.1'
                    type 'dir'
                }
            ")

            cli.create(package_file)
        end
    end
end

We create a tempfile and the filename stored in the package_file 'let' variable. That's just an empty file, though. We then want to put some stuff in it, so we create the append_to_file helper method. (This highlights something important - we can add methods as needed to our tests.) Then, we use it to fill the file with stuff and pass the filename to Packager::CLI#create.

Note: We have to flush to disk to ensure that when we read from the file, the contents are actually in the file instead of the output buffer.

We have our input filename (and its contents) figured out. What should we expect to happen? We could look at whether a package was created in the directory we invoked the CLI. That is what our user story requires. And, we will want to have that sort of integration test, making sure everything actually functions the way a user will expect it to function. But, that's not this test. (Not to mention the Executor doesn't actually invoke FPM yet!)

These tests are meant to exercise each class in isolation - these are unit tests. Unit tests exercise the insides of one and only one class. If we were to see if a package is created, we're actually testing three classes - the CLI as well as the DSL and Executor classes. That's too many moving parts to quickly figure out what's gone wrong when something fails. By having tests which only focus on the internals of the CLI, DSL, and Executor classes by themselves as well as the integration of all the parts, we can easily see which part of our system is in error when tests start to fail. Is it the integration and CLI tests? Is it just the integration tests? Is it just the DSL? All of these scenarios immediately point out the culprit (or culprits).

Given that the CLI is going to invoke the DSL and Executor, we want to catch the invocation of the #parse_dsl and #create_package methods. We don't want to actually do what those methods do as part of this test. Frankly, the CLI object doesn't care what those methods do. It only cares that the methods function, whatever that means.

RSpec has a concept called stubbing. (This is part of a larger concept in testing called "mocking". RSpec provides mocks, doubles, and stubs, as do many other libraries like mocha.) For our purposes, what we can do is say "The next time method X is called on an instance of class Y, do <this> instead." Stubs (and mocks and doubles) will be uninstalled at the end of every spec, so there's no danger of it leaking or affecting anything else. With stubs, our happy-day test now looks like

        it "creates a package with a filename" do
            contents = "
                package {
                    name 'foo'
                    version '0.0.1'
                    type 'dir'
                }
            "
            append_to_file(package_file, contents)

            expect(Packager::DSL).to receive(:parse_dsl).with(contents).and_return(:stuff)
            expect_any_instance_of(Packager::Executor).to receive(:create_package).with(:stuff).and_return(true)

            cli.create(package_file)
        end

This looks like an awful mouthful. And, it may seem odd to create expectations before we call cli.create. But, if you think about it for a second and read the two expectations out loud, it can make sense. All of our tests so far have been "We expect X is true." What we're now saying is "We expect X will be true." Which works out.

As for the formatting, you can do the following:

      expect(Packager::DSL).to(
          receive(:parse_dsl).
          with(contents).
          and_return(:stuff)
      )

Note the new parentheses for the .to() method and the periods at the end of one line (instead of the beginning of the next). These are required for how Ruby parses. You could also use backslashes, but I find those too ugly. This, to me, is a good compromise. Please feel free to experiment - the goal is to make it readable for you and your maintainers, not me or anyone else in the world.

Our #create method now changes to

        def create(filename=nil)
            raise Thor::Error, "No filenames passed for create" unless filename
            items = Packager::DSL.parse_dsl(IO.read(filename))
            Packager::Executor.new.create_package(items)            
        end

and we're done. Remember to do a git status to make sure you're adding all the new files we've been creating to source control.

prev