TDD / BDD

Principles, patterns and guidelines

by Wojciech Zawistowski

part 1

basics

Types of tests

• end-to-end (e2e)
• unit
• integration

e2e

• treat system as a black box(no "manual" DB reads / writes!)
• slow and harder to debug(should test only main paths, not edge cases)
• goal: ensure correct "wiring"(no mocks, production-like configuration)

unit

• test small, autonomous units(typically single class)
• partially white box(black box unit internals, white box unit API and interactions)
• fast and easy to debug(should test edge cases)
• goal: maximum isolation(don't test unit dependencies - stub them)

unit & e2e = complementary

• they serve different purpose(it's not a stylistic choice)
• both types should be used together(otherwise test suite cannot be fully trusted)

integration

• test connections with 3rd party code(e.g. file system, database, web service, library)
• easy to debug but slow(use where necessary, but minimize their number)
• may or may not test edge cases(depends on your knowledge and trust in 3rd party service)
• goal: test single integration point(shouldn't test the whole system or business logic)

when to write tests

after the code

vs

before the code (TDD)

tests after the code

• prevent regression in future(tests will tell us if new feature breaks the old code)
• difficult to write(code design may turn out to be untestable)
• time consuming(we test twice: manually when coding and via test suite)

TDD

• enables emergent design(experiments and refactoring during coding are easy)
• enforces better design(low coupling, easier to use APIs, code easy to modify)
• easier to write tests(design supports testing from the start)
• less time consuming(no manual testing when coding)

two "flavors"

"classic" TDD

vs

Behavior Driven Development (BDD)

TDD

• focused on functionality(tests usually organized per API method)
• higher code coverage(may potentially cover more edge cases)
• bloated, redundant code and tests(we test - and thus code - everything we can)

BDD

• focused on specification(tests usually organized per feature)
• better requirements coverage(tests are clearer about the purpose of the code)
• works great as live documentation(tests cover real usage scenarios)
• no unnecessary code(we test - and thus code - only real business needs)

similar e2e tests "flavors"

• functional ~ TDD
• acceptance ~ BDD

OK, Example...

TDD

focus on code (methods)

describe "DartGame.scorePoints", ->
    ...

describe "DartGame.isWin", ->
    ...

describe "DartGame.scorePoints", ->
    beforeEach ->
        game.setPoints 10

    it "updates points when scoring", ->
        game.scorePoints 9
        expect(game.getPoints).toEqual 1

    it "updates points when scoring all the points left", ->
        game.scorePoints 10
        expect(game.getPoints).toEqual 0

    it "does not update points when overscoring", ->
        game.scorePoints 11
        expect(game.getPoints).toEqual 10

describe "DartGame.isWin", ->
    it "returns false when score is greater than zero", ->
        game.setPoints 1
        expect(game.isWin).toBeFalse

    it "returns true when score equals zero", ->
        game.setPoints 0
        expect(game.isWin).toBeTrue

seems legit, but...

it "updates points when scoring all the points left", ->
    game.scorePoints 10
    expect(game.getPoints).toEqual 0

Do we really care if points are zeroed?Or only if the game is marked as won?

it "returns true when score equals zero", ->
    game.setPoints 0
    expect(game.isWin).toBeTrue

Do we really care if the game is won when points EQUAL zero?Or rather when they REACH zero?

Is it realistic scenario to win the game by SETTING the points?Or rather should the game be won by SCORING proper number of points?

How to find all the legal "moves" in a game?Simple cross section of all scorePoints tests with all isWin tests gives also some obviously invalid combinations... To find only the valid ones you need to reverse engineer the tests.

BDD

focus on domain (game flow)

describe "Dart game: scoring", ->
    ...

describe "Dart game: overscoring", ->
    ...

describe "Dart game: scoring exactly", ->
    ...

describe "Dart game: scoring", ->
    beforeEach ->
        game.setPoints 10
        game.scorePoints 9

    it "updates points", ->
        expect(game.getPoints).toEqual 1

    it "does not result in a win yet", ->
        expect(game).not.toBeWin

describe "Dart game: overscoring", ->
    beforeEach ->
        game.setPoints 10
        game.scorePoints 11

    it "does not update points", ->
        expect(game.getPoints).toEqual 10

    it "does not result in a win", ->
        expect(game).not.toBeWin

describe "Dart game: scoring exactly", ->
    beforeEach ->
        game.setPoints 10
        game.scorePoints 10

    it "results in a win", ->
        expect(game).toBeWin

Why is it better?

describe "Dart game: scoring exactly", ->
    it "results in a win", ->
        expect(game).toBeWin

    # there is no test related to updating points!

We don't care if points are zeroed or not in a win conditionOf course we could IF there are such business requirements (but no guessing!)

describe "Dart game: scoring exactly", ->
    beforeEach ->
        ...
        game.scorePoints 10

    it "results in a win", ->
        ...

We check realistic win condition: scoring exactly(not setting the points "by hand")

How to find all the legal "moves" in a game?By straightforward reading the test...

describe "Dart game: scoring", ->

describe "Dart game: overscoring", ->

describe "Dart game: scoring exactly", ->

TDD workflow

• red
• green
• refactor

red

• decide about the next test(it's OK to plan ahead, but overplanning fights emergent design)
• write a test that FAILS(or even only a small part of a test)
• never skip this step!(or you won't be sure if the test really works)

green

• implement code to make test pass(only the new one from the red step - don't batch it)
• implement MINIMAL solution(even if it seems really, really dumb)
• never add code not driven by test(even if it seems simple and obvious)

refactor

• clean up code from the previous step(not always necessary, but you should CONSIDER it every cycle)
• this step is critical!(emergent design is still a DESIGN - only postponed in time)
• experiment - it's safe!(you're covered by tests)
• tests should be refactored too(they are your requirements - you want them clean!)

step-by-step example

First little bit of a test

# the test
describe "User", ->
    it "provides name and surname parts of full name", ->
        user = new User

It's only the test setup, no expectations yet, but we still pause here and run the test...

...and the test fails - no User class yet.

Now some code to make it pass

# the test
describe "User", ->
    it "provides name and surname parts of full name", ->
        user = new User

#the code
class User

Yep, just that. Now the test passes.

Now we exercise the User

# the test
describe "User", ->
    it "provides name and surname parts of full name", ->
        user = new User

        user.setFullName "Wojciech Zawistowski"

#the code
class User

Test fails - no setFullName method.

Minimal possible step to make test pass

# the test
describe "User", ->
    it "provides name and surname parts of full name", ->
        user = new User

        user.setFullName "Wojciech Zawistowski"

#the code
class User
    setFullName: (fullName) ->

Yes, method does nothing for now, not even assigns to a var.

Finally, actual expectations

# the test
describe "User", ->
    it "provides name and surname parts of full name", ->
        user = new User

        user.setFullName "Wojciech Zawistowski"

        expect(user.name).toEqual "Wojciech"
        expect(user.surname).toEqual "Zawistowski"

#the code
class User
    setFullName: (fullName) ->

And finally a business logic related test fail.

Minimal step to make it pass

# the test
describe "User", ->
    it "provides name and surname parts of full name", ->
        user = new User

        user.setFullName "Wojciech Zawistowski"

        expect(user.name).toEqual "Wojciech"
        expect(user.surname).toEqual "Zawistowski"

#the code
class User
    setFullName: (fullName) ->

    name: -> "Wojciech"

    surname: -> "Zawistowski"

Yes, hardcoded! It seems dumb, but it proves that the test works.

And the second test

# the test
describe "User", ->
    it "provides name and surname parts of full name", ->
        user = new User

        user.setFullName "Wojciech Zawistowski"

        expect(user.name).toEqual "Wojciech"
        expect(user.surname).toEqual "Zawistowski"

        # it works also when full name is updated
        user.setFullName "Chuck Norris"

        expect(user.name).toEqual "Chuck"
        expect(user.surname).toEqual "Norris"

Very ugly copy paste (but it's the easiest thing to do for now).

We make it pass

#the code
class User
    setFullName: (fullName) ->
        fullNameParts = fullName.split(" ")
        @name = fullName[0]
        @surname = fullName[1]

    name: -> @name

    surname: -> @surname

Finally, real business logic!

Now it's time to clean up the (UGLY!) test(remember red, green, REFACTOR?)

# the test
describe "User: accessing name and surname parts of full name", ->
    beforeEach ->
        user = new User
        user.setFullName "Wojciech Zawistowski"

    it "works for initially set full name", ->
        expect(user.name).toEqual "Wojciech"
        expect(user.surname).toEqual "Zawistowski"

    it "works for updated full name"
        user.setFullName "Chuck Norris"

        expect(user.name).toEqual "Chuck"
        expect(user.surname).toEqual "Norris"

Remove all duplication etc. - the same as in "real" code.

and so on...

we've seen only the test refactoring, but for code it's the same:

• code the quick and dirty solution to make the test pass
• refactor the code to look pretty immediately afterwards

e.g. to add param validation to setFullName, we would:

• write a test checking if invalid name throws error
• make it pass via inline IF in setFullName
• extract validation to a standalone method

coding in baby steps

• may seem like wasted work(and often even like a plain dumb thing to do)
• but it gives instant feedback(if something breaks, you instantly know what caused this)
• and it is much more effective(debugging big blocks of code is slower than simple refactorings)

3 rules of hardcore TDD

• always listen to the tests(if writing a test is hard, don't fight the test - redesign the code)
• no manual testing, ever(tests ≠ only validation - exploratory tests)
• tests are more important than code(easier to reproduce code from tests than tests from code)

part 2

good practices

good tests must be

• readable
• debuggable
• reliable
• maintainable

readability

• tests are live API documentation(you don't want misleading documentation)
• tests guide emergent design(you don't want to design against unclear specification)
• tests guard against regression(you need to know which requirements conflicted and why)

test method naming

• precise
• descriptive
• focused

should read as a specification

bad

test_invoice_email() {...}

better

test_that_invoice_email_is_sent_when_invoice_is_issued() {...}

explain purpose not implementation

bad

test_that_logIn_returns_null_when_moderator_flag_is_true() {...}

better

test_that_user_banned_by_moderator_cannot_log_in() {...}

self-explanatory names in test code

• variable names
• fixture names
• even variables and fixture content!

bad

user = User.logIn "qweert", "abcdefgh"

expect(user).not.toBeLoggedIn

better

validLogin = "qweert"
invalidPassword = "abcdefgh"

user = User.logIn validLogin, invalidPassword

expect(user).not.toBeLoggedIn

even better

validLogin = "valid_username"
invalidPassword = "pw_without_at_least_one_digit"

unauthorizedUser = User.logIn validLogin, invalidPassword

expect(unauthorizedUser).not.toBeLoggedIn

hide irrelevant information

• custom assertions
• helper methods
• object factories

bad

user = new User
    name: "Irrelevant Name"
    address: "Some Irrelevant Address"
    isActive: true

post = new Post
    author: user
    text: "some irrelevant text"
post.publish()
post.flagAsSpam()

expect(user.isActive).toBeFalse

better

user = createActiveUser() # factory hiding irrelevant user creation details

publishSpam(user) # helper method hiding irrelevant post flagging details

expect(user).toBeDeactivated # custom assertion hiding irr. status details

given / when / then

every test can be reduced to these 3 distinct phases:

• given = test setup (previous system state)
• when = user or system action (state change)
• then = expectations (new system state)

at least separate these 3 phases visually with whitespace

user.logIn
visit "/profile"

fillIn ".new-post", "some text"
click ".submit-post"

visit "/recent-posts"
post = findFirst ".post"
expect(post.text).toEqual "some text"

or be hardcore and reduce them to meaningful one-liners

given_user_is_logged_in_to_profile_page

when_user_creates_new_post

then_new_post_is_published_at_the_top_of_recent_posts_list

for low-level unit tests this is often an overkill:

given_empty_array_exists
when_new_item_is_added

#vs

array = []
array.push "new item"

but for more complicated tests (esp. e2e acceptance tests)

when_user_registers_an_account

#vs

visit "/sign-up"
fillIn ".login", "some_login"
fillIn ".password", "some_pwd"
fillIn ".confirm-password", "some_pwd"
click ".submit"
visit "/confirm-signup"
click ".confirmation-link"

makes a big difference

the simplest readability heuristics

• ideal unit tests should read like a good API docs
• ideal acceptance tests should read like a good user manual

debuggability

• tests must give clear and instant feedback(when a suite fails, you should immediately know which test failed)
• tests must precisely pinpoint the problem(test should tell you exactly what failed and why)
• tests should be self-sufficient(no debugger, console.log or print statements should be needed when TDD-ing)

The RED step from red, green, refactor is also for verification if your failure messages are clear.

debuggability techniques

• meaningful test names
• detailed assertion messages
• custom assertions

detailed assertion messages

bad

assertTrue(user.isAdmin)

# fail message:
# expected true but got false

better

assertTrue(user.isAdmin, "user is not an admin")

# fail message:
# user is not an admin

even better

assertTrue(user.isAdmin, "user in group #{user.group} is not an admin)

# fail message:
# user in group 'moderators' is not an admin

custom assertions

assertTrue(user.isAdmin, "user in group #{user.group} is not an admin)

# vs

assertAdmin(user)

• encapsulate complex fail messages
• make test code more concise
• reusable

how many assertions per test?

• some purists claim only 1
• this is usually a good rule
• however, there are exceptions
• better rule = 1 CONCERN / test

bad

describe "new user", ->
    it "is not activated and has empty account", ->
        expect(user).not.toBeActivated
        expect(user.accountBalance).toEqual 0

# the alarm signal:
# difficult to name the test without using ANDs or generalizations

better

describe "new user", ->
    it "is not activated", ->
        expect(user).not.toBeActivated

    it "has empty account", ->
        expect(user.accountBalance).toEqual 0

good

describe "guest user", ->
    it "has empty address", ->
        expect(user.address).toBeEmpty

    it "has empty phone number", ->
        expect(user.phoneNumber).toBeEmpty

even better

describe "guest user", ->
    it "has empty contact data", ->
        expect(user.address).toBeEmpty
        expect(user.phoneNumber).toBeEmpty

# assertions grouped under a single meaningful concern ("contact data")

Depends also how the test framework shows fail messages:

• some show only the first failed assertion in a test
• other show all failed assertions in a test

(You must decide what's more debuggable and group / distribute assertions accordingly)

the simplest debuggability heuristics

debugging is for production bugs(if you have to debug to find why a test failed, you're doing it wrong!!!)

e2e tests debuggability

higher level (requirements instead of code) but the same principles apply

reliability

• no interdependent tests(order of tests or turning some tests off shouldn't matter)
• no dependency on unreliable resources(e.g. network, web services)
• fully deterministic tests(no dependency on system clock, random number generators etc.)
• no dependency on async / timing related stuff(tests shouldn't fail because of a timeout etc.)
• fast tests(slow tests discourage running them often what is against TDD philosophy)

solution = isolation

• stubbing unreliable / slow dependencies (in tests)
• requires using dependency injection (in code)

the simplest reliability heuristics

no false alarms(if a test reports a fail it must mean code is broken, not that the DB connectoin is slow or network is down)

maintainability

Tests support modification of existing features, therefore:

• they will be modified often
• they must be easy to modify
• modifications can't ripple throughout the suite
• small changes can't break big parts of the suite

soution = clean test code

• no duplication
• encapsulation (helpers, factories etc.)
• isolation
• use all other good OO design practices

Tests must be constantly maintained (refactored etc.) the same as "normal" production code.

the simplest maintainability heuristics

the same as for any other code (design paterns, code smells etc.)

isolation

dependency injection + test doubles (stubs, mocks, spies)

quick example

#tested class
class Invoice
    issueNumber: -> currentDate = new Date

#test
it "issues correct number for 31 Dec", ->
    #no way to test it...

vs

#tested class
class Invoice
    constructor: (dateFactory) -> @dateFactory = dateFactory

    issueNumber: -> currentDate = @dateFactory.currenDate

#test
it "issues correct number for 31 Dec", ->
    dateFactoryStub = stub(DateFactory)
    dateFactoryStub.currentDate.returns(new Date("2000-12-31"))
    invoice = new Invoice(dateFactoryStub)

stubs vs mocks vs spies

• stubs: fake data received FROM dependencies
• mocks & spies: verify messages sent TO dependencies(mocks set expectations before the fact, spies verify expectations after the fact)

stubs vs spies (different purpose)

stubs

describe "FileConverter", ->
    it "uppercases file contents", ->
        fileReaderStub = stub(FileReader)
        fileReaderStub.read.returns("some text")
        converter = new FileConverter(fileReaderStub)

        expect(converter.uppercased).toEqual "SOME TEXT"

spies

describe "AlertPrinter", ->
    it "sends alert message to a printer", ->
        printDriverSpy = spy(PrintDriver)
        alertPrinter = new AlertPrinter(printDriverSpy)

        alertPrinter.sendAlert "some text"

        expect(printDriverSpy.print).toHaveBeenCalledWith "some text"

mocks vs spies (stylistic choice)

mocks

describe "AlertPrinter", ->
    it "sends alert message to a printer", ->
        printDriverMock = mock(PrintDriver)
        expect(printDriverMock.print).toBeCalledWith "some text"
        alertPrinter = new AlertPrinter(printDriverMock)

        alertPrinter.sendAlert "some text"

spies

describe "AlertPrinter", ->
    it "sends alert message to a printer", ->
        printDriverSpy = spy(PrintDriver)
        alertPrinter = new AlertPrinter(printDriverSpy)

        alertPrinter.sendAlert "some text"

        expect(printDriverSpy.print).toHaveBeenCalledWith "some text"

good practices

don't overspecify stubs

ok (you can't avoid it)

fileReaderStub.read.withParam("file_1.txt").returns "text 1"
fileReaderStub.read.withParam("file_2.txt").returns "text 2"

expect(concatenator.concat("file_1", "file_2")).toEqual "text 1 text 2"

bad

fileReaderStub.read.withParam("some_file.txt").returns "some text"

expect(formatter.uppercase("some_file")).toEqual "SOME TEXT"

# in this test we should verify uppercasing, not reading correct file!

better

# returns "some text" for ANY file - it doesn't matter for this test
fileReaderStub.read.returns "some text"

expect(formatter.uppercase("some_file")).toEqual "SOME TEXT"

don't overspecify mocks/spies

ok (this is the goal of this test)

it "sends correct mail", ->
    user.sendAlertMail

    expect(mailerMock.send).toHaveBeenCalledWith "alert!!!"

bad

it "blocks duplicate alerts from the same user", ->
    user.sendAlertMail
    user.sendAlertMail

    expect(mailerMock.send).toHaveBeenCalledOnce.with "alert!!!"
    # the goal of this test is to check duplication, not the content

better

it "blocks duplicate alerts from the same user", ->
    user.sendAlertMail
    user.sendAlertMail

    expect(mailerMock.send).toHaveBeenCalledOnce

don't mix mocking and stubbing

bad

it "sends correct alert mail", ->
    expect(mailerMock.send).toBeCalledWith("text").andReturn "ok"

    user.sendAlertMail

    expect(user.lastAlertStatus).toEqual "ok"

# tempting, but it hides second concern in a single test method

better

it "sends correct alert mail", ->
    expect(mailerMock.send).toBeCalledWith("text")

    user.sendAlertMail

it "stores last alert status code", ->
    mailerStub.returns "ok"

    user.sendAlertMail

    expect(user.lastAlertStatus).toEqual "ok"

sendAlertMail method may crash without status code...

...in such case we should give it any (simplest possible) code:

it "sends correct alert mail", ->
    expect(mailerMock.send).toBeCalledWith("text").andReturn "whatever"

    user.sendAlertMail

    # and we shouldn't exercise this code with any assertions here!!!

# only here, in separate test, like in previous example...
it "stores last alert status code", ->
    mailerStub.returns "ok"

    user.sendAlertMail

    expect(user.lastAlertStatus).toEqual "ok"

stub only 1 level deep(The Law of Demeter)

bad

userStub.getBooking.returns bookingStub
bookingStub.getHotel.returns hotelStub
hotelStub.getName.returns "Some Hotel"

mail = new WelcomeBackMail(userStub)

expect(mail.title).toEqual "Welcome back from Some Hotel!"

better

userStub.getLastBookingHotelName.returns "Some Hotel"

mail = new WelcomeBackMail(userStub)

expect(mail.title).toEqual "Welcome back from Some Hotel!"

(less brittle test and will force better code design)

don't stub simple value objects

ok

phoneNumber = new PhoneNumber(prefix: 123, number: 4567890)
user = new User(phoneNumber)

overkill

phoneNumberStub = stub(PhoneNumber)
phoneNumberStub.prefix.returns 123
phoneNumberStub.number.returns 4567890
user = new User(phoneNumberStub)

don't mix logic with resource access

bad (slow tests)

class Invoice
    addItem: (item) ->
        if self.validate(item)
            self.itemCollection.add(item)
        self.save()
        # always hits the DB (on each validation related test branch)

better

class Invoice
    addItem: (item) ->
        if self.validate(item)
            self.itemCollection.add(item)

class InvoiceRepository
    saveInvoice: (invoice) ->
        #...

# adding item to an invoice and saving an invoice are separate concerns
# (adding item = unit tests, saving invoice = integration test)

stub/mock only dependencies

Never stub or mock methods of the class you're testing!!!(If it seems necessary, you most probably should extract part of the class)

the simplest isolation heuristics

Tests should have short and simple setup.(If you have to create too many or nested stubs, consider redesigning your code!)

part 3

process

different testing processes

• production bugs
• greenfield projects
• legacy projects

production bugs

• find the cause of the bug(using normal methods: debugger, logs etc.)
• write a test covering the discovered cause (red)(it should fail exectly because of the problem found in debugging step)
• fix the bug (green)(test should now pass)
• properly merge the test into other tests (refactor)(suite should read as cohesive documentation not as a bag of random edge cases!)

What do you mean by merging the test into other tests?

• test suite describes a set of requirements
• bug causes are seldom new requirements...
• ...they're rather "holes" in existing requirements
• no new special-case tests should be introduced
• the "holes" in existing tests should be "fixed"

greenfield projects

• top-down approach
• emergent design
• reliance on stubs

process

• write acceptance test(or even a part of it)
• create necessary infrastructure(enough to make test fail on missing business logic not missing frameworks)
• create enough code to make acceptance test pass(using normal TDD process based on unit tests)
• write next acceptance test and start over(repeat untill all requirements are satisfied)

write unit tests also top-down

• initial acceptance test reveals what views are needed
• TDD-ing views reveals what controllers are needed
• TDD-ing controllers reveals what models are needed
• ...etc.

API discovery

When implementing higher layer code:

• stub all lower layer dependencies(they don't exist yet anyway...)
• don't design lower layer API in advance(let the real, emerging needs of higher layer code guide the design)

"spikes"

• sometimes you have no idea how to structure the code(so it's difficult to do proper TDD - you have no idea what test to write)
• you can do a quick experiment (a "spike")(try several possible designs, get a feeling how they look like etc.)
• "spikes" are allowed to have no tests(although they MAY have tests - often it's easier/faster to create a spike using TDD)
• "spikes" MUST be treated as a throwaway prototype(ALWAYS delete the spike and rewrite it from scratch using proper TDD process)

legacy projects

• require refactoring
• reliance on higher level, throwaway tests
• exploratory testing

refactoring

• if legacy code is clean, only untested - just add tests
• ok, enough jokes ;)
• to make code testable, we usually need to refactor it

the conundrum

• to make code testable, we need to refactor
• to be able to refactor safely, we need tests

you can't use unit tests

• code has too many dependencies to be properly isolated
• refactoring will move code outside of unit test scope

but you can use pseudo-e2e tests

• they cover big enough part of the code so it can move inside
• they don't require isolation

how to write such pseudo-e2e tests

• as close to the refactored code (to true unit test) as possible
• high enough to remain stable when reorganizing code
• think of them as multi-unit tests (the less units the better)
• if you can inject / mock a dependency then still do it!
• if you can't, use fixtures and other e2e level test techniques

pseudo-e2e tests example

some tests may be closer, some must be farther from the refactored code

pseudo-e2e tests are throwaway!!!

• cover refactored code with proper unit tests
• delete pseudo-e2e tests afterwards
• don't keep them for long
• don't try to maintain them

pseudo-e2e tests

• don't have to cover full API(only these methods that use refactored code)
• may be more detailed than normal e2e tests(they need to cover some edge cases to enable safe refactoring)
• don't have to be clean and well factored(they are only temporary safety net and won't be maintained)
• may be slow(you won't run the whole suite of such tests)

Yes, creating such safety net is a lot of work...

...that will be thrown away :( ...

...but refactoring without it is much more work (and risk)!

exploratory tests

• don't guess how the code works
• write a test that will show you

explore dependencies

Just try to instantiate a class and see why the test breaks.(what other classes it requires)

explore behavior

Set nonsensical assertions and find real value in error message.

a tricky question

• what part of discovered behavior are requirements(and should be covered by test suite)
• and what part of it is accidental(and don't have to be covered by test suite)

no good answer

But a good starting point for discussion with your analyst ;)

recommended reading

• Gerard Meszaros “xUnit Test Patterns: Refactoring Test Code”http://my.safaribooksonline.com/book/software-engineering-and-development/software-testing/9780131495050
• Steve Freeman; Nat Pryce “Growing Object-Oriented Software, Guided by Tests”http://my.safaribooksonline.com/book/software-engineering-and-development/software-testing/9780321574442
• Michael Feathers “Working Effectively with Legacy Code”http://my.safaribooksonline.com/book/software-engineering-and-development/0131177052

That's all Folks!