ENTR-451

The definition of done, and tests

Ben Block -

Code-along 3 ended with a Company model that could save, find and change rows without a line of code in app/models/company.rb. The migration gave it all of that. What it couldn't do was refuse anything: a company with no name saved as happily as Apple. This post is the second half of that code-along and the whole of code-along 4: how a rule gets written down, how it becomes a test, and what the test is for.

The rule comes first, in English

DONE.md is a file of sentences. Under each model's heading, one line per thing that must be true:

## Company
A company can't be saved without a name.
Two companies can't have the same name.

No Ruby. The question it answers is "what can't be allowed?", and the person answering it is you, because you're the one who knows what the business needs. The agent reads this file and never writes to it. That split is deliberate: if the agent wrote the rules, it would be grading its own work.

This is the definition of done. Not "the code is written", but "these sentences are true, and something proves it."

One test per sentence

The prompt from class:

My rules for Company are under Company in DONE.md. Write a test for each one, run them and show me they fail. Don't change the model yet.

The agent writes test/models/company_test.rb:

require "test_helper"

class CompanyTest < ActiveSupport::TestCase
  # One test for each sentence under Company in DONE.md, named with that sentence.

  test "a company can't be saved without a name" do
    company = Company.new
    company["city"] = "Beaverton"
    company["state"] = "OR"
    company.save

    assert_equal 0, Company.all.count
  end

  test "two companies can't have the same name" do
    company1 = Company.new
    company1["name"] = "Nike"
    company1.save

    company2 = Company.new
    company2["name"] = "Nike"
    company2.save

    assert_equal 1, Company.all.count
  end

end

Each test block is one experiment. It sets up a situation (a company with no name), does the thing (save), and states what must be true afterwards (assert_equal 0, Company.all.count: no rows). The name of the test is the sentence from DONE.md, word for word, so that when a test fails, its name tells you which rule broke.

The tests run against a separate, empty database, rebuilt for every test. Nothing you do in scripts/ or on the Data page is visible to them, and nothing they do touches your data.

Red first

bin/rails test

Failure:
CompanyTest#test_a_company_can't_be_saved_without_a_name
Expected: 0
  Actual: 1

Failure:
CompanyTest#test_two_companies_can't_have_the_same_name
Expected: 1
  Actual: 2

2 runs, 2 assertions, 2 failures, 0 errors, 0 skips

Both fail, and that is the result you wanted. The model has no rules yet, so a nameless company saved (1 row, not 0) and both Nikes saved (2, not 1). The tests are describing a problem that really exists.

This is why the prompt said "don't change the model yet." A test you have never seen fail might not be checking anything. If the agent had written the model and the tests together and shown you green, you'd have no way to tell a test that proves the rule from a test that passes no matter what. Code-along 0 made the same point with one = in place of ==: a check that can't fail is worse than no check, because it looks like one.

The name for this way of working is test-driven development, TDD: write the test, watch it fail, write the code, watch it pass. The order is the whole technique.

Check before green

Before "make them pass", open DONE.md beside the test file and do two things the agent can't do for you:

  • Count. Every sentence has a test? The agent will say "done" either way. A sentence with no test is a rule your app doesn't enforce, and you'd only find out when it's broken.
  • Read. Each test checks what its sentence says? The first test above sets a city and a state and leaves out the name, then expects nothing saved. If it had set a name too, it would be testing nothing.

This is the manager's seat. You don't need to write the test to tell whether it tests the rule.

Green

Make them pass.

The agent adds one line to app/models/company.rb:

class Company < ApplicationRecord
  validates :name, presence: true, uniqueness: true
end

validates states a rule on a column. presence: true refuses a blank; uniqueness: true looks in the table and refuses a duplicate. When a rule fails, save returns false instead of saving, and company.errors.full_messages says which rule: Name can't be blank.

2 runs, 2 assertions, 0 failures, 0 errors, 0 skips

Run that yourself, not just the agent. Then read the model. One line for two sentences, and you can read it against the sentences: presence is the first rule, uniqueness the second.

What the tests are for: the helpful change

Code-along 4. Imports from a spreadsheet keep failing on rows with no company name. A developer decides to help: when the name is blank, fill one in.

code-along/4-introduce-bug.rb tries to save a company with a city and no name. Before any change:

Not saved: Name can't be blank

Then the change, pasted into the model:

before_validation :fill_in_missing_name

def fill_in_missing_name
  if self["name"].blank?
    self["name"] = "Unknown company"
  end
end

before_validation runs the method every time a company is about to be checked and saved, so the blank is filled before validates looks. Run the file again:

Saved, with the name "Unknown company"

The import works now. The app runs. Nobody would notice, except:

bin/rails test

Failure:
CompanyTest#test_a_company_can't_be_saved_without_a_name
Expected: 0
  Actual: 1

2 runs, 2 assertions, 1 failures, 0 errors, 0 skips

The test named with the rule is the one that failed. It says, in the words you wrote, what the helpful change broke. The developer meant well, the code is tidy, and the rule is gone.
Without the test, the first "Unknown company" would turn up in a report three weeks later.

Take the lines back out, run the tests, green again.

Why this is the week's idea

The agent in your terminal is non-deterministic. Ask it the same thing twice and you get two different files. That is fine as long as something deterministic decides whether what it wrote is right, and that something is the tests. You write the rule; the test enforces it; the agent works until the test is green; and the next change, by you, by a teammate or by an agent, runs into the same test.

Write the definition of done before the code. Everything else in the week follows from it.

If no test fails in step 5

Then the rule was never written down as a test. Add the sentence to DONE.md and have your agent write its test. Lab 4 is a harder version of exactly this: a change that gets past a test which was written, and why.