Skip to content

Conversation

@varokas
Copy link

@varokas varokas commented May 12, 2012

Just a minor suggestion.

Please see if you like this:

  • Changed the method name to exactly say what it is testing.
  • For those method that test two things. Refactored them to two methods, so that they can fail on one condition only.

For example: Now the testTransfer will on a separate test that

  1. Amount is transferred correctly (and)
  2. Reciet is produced correctly.

Refactor the test to fail for only one reason.
@roofimon
Copy link
Owner

Many thanks krub,

On Sat, May 12, 2012 at 7:13 AM, varokas <
reply@reply.github.com

wrote:

Just a minor suggestion.

Please see if you like this:

  • Changed the method name to exactly say what it is testing.
  • For those method that test two things. Refactored them to two methods,
    so that they can fail on one condition only.

For example: Now the testTransfer will on a separate test that

  1. Amount is transferred correctly (and)
  2. Reciet is produced correctly.

You can merge this Pull Request by running:

git pull https://github.com/varokas/tdd-using-spring master

Or you can view, comment on it, or merge it online at:

#1

-- Commit Summary --

  • Rename tests

-- File Changes --

M src/test/com/bank/service/internal/DefaultDepositServiceTests.java (4)
M src/test/com/bank/service/internal/DefaultTransferServiceTests.java (32)
M src/test/com/bank/service/internal/VariableFeePolicyTests.java (18)

-- Patch Links --

https://github.com/roofimon/tdd-using-spring/pull/1.patch
https://github.com/roofimon/tdd-using-spring/pull/1.diff


Reply to this email directly or view it on GitHub:
#1

Write Code Not War

@roofimon
Copy link
Owner

I will press "Merge Pull Request" at 4:00 pm during my session at Bug Day. To show how TDD encourage us to change our code to be better and better.
By the way...Thanks for your suggestion krub

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants