This site is the archived OWASP Foundation Wiki and is no longer accepting Account Requests.
To view the new OWASP Foundation website, please visit https://owasp.org
Difference between revisions of "CRV2 CodeReviewAgile"
m |
m |
||
| Line 14: | Line 14: | ||
| − | ==Pair Programming== | + | ===Pair Programming=== |
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously. | This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously. | ||
| Line 22: | Line 22: | ||
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system. | This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system. | ||
| − | ==Life Cycle== | + | ===Life Cycle=== |
Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle. | Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle. | ||
| Line 29: | Line 29: | ||
| − | + | ===Clean Code and "Smells"=== | |
| − | ==Clean Code and "Smells"== | ||
==The Agile Ecosystem== | ==The Agile Ecosystem== | ||
| Line 43: | Line 42: | ||
| − | ==Agile | + | ==Code Reviewing an Agile Project== |
| − | + | If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you. | |
| + | |||
| + | Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review. | ||
| + | |||
| + | In order to review an Agile Project, you will have to extend the review to the tests. | ||
| + | |||
| + | Are there enough tests? | ||
| − | + | Is the code covered by the tests? Code coverage measure main value is to find unused code. | |
| + | Are there the trivial tests? They are as needed as any other test. | ||
| + | Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run. | ||
| − | + | Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition. | |
| − | |||
| + | Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition? | ||
| − | + | Are the test automatic? If the tests requiere manual intervention, they will not be run. | |
| + | Do the tests conform to F.I.R.S.T.? | ||
| + | Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle. | ||
| − | + | Independent: one test can not depend on the execution of another one, the order should not matter. | |
| − | + | Repeatable: one test should give always the same result in any environment if nothing has changed in the code. | |
| − | + | Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed. | |
| − | . | + | Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system. |
| − | |||
| − | |||
| − | |||
| − | + | ==Other Agile Resources== | |
| − | The | + | ===The role of testing=== |
| + | It is so fundamental, that the xDD pervades Agile, test first, test earlier | ||
| − | |||
| − | |||
| − | |||
| − | |||
| − | + | ===Continuous integration=== | |
| + | it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly. | ||
| − | |||
| − | + | ===The role of automatic static code analysis in the Agile Methodologies=== | |
| − | |||
| − | |||
| − | + | ===Test Driven Development=== | |
| − | + | It aims at code simplicity due to the need of making it testeable | |
| − | + | ===Behavior Driven Development=== | |
| − | + | .... | |
| − | + | ===Domain Driven Design=== | |
| − | + | .... | |
| − | + | ===Refactoring=== | |
| − | + | Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests. | |
| − | + | The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis. | |
| Line 117: | Line 118: | ||
| − | References and sources | + | ==References and sources== |
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin | Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin | ||
Revision as of 23:25, 23 June 2013
Charly, remember that it's "code review guide", not "testing guide"
Some definitions about Agile
The Agile name is an umbrella for quite a lot of practices that range from programming, to testing, to project management and everything in between.
Agile has some key practices that could affect the way the code is reviewed.
Agile Development is well suited for code review, as two of its practices are "pair programming" and "peer review". AD incorporates code review in itself, in what traditionally was seem as another phase.
Agile blurs the difference between developing and testing, and so does with code review. It is not an external activity.
Pair Programming
This technique is quite controversial, but when its adopted it has the benefits of making better code, as there is one programmer supervising cooperatively the other one's work. If both programmers know this guide, they will apply it continuously.
Peer Review
This one is enforced by the usage of tools like .... that ask another user for a code review before commiting to the versioning system.
Life Cycle
Agile tries to keep the code testing and review as near as possible to the development phase, there is no such thing as the "develop, test, code review cycle.
Clean Code and "Smells"
The Agile Ecosystem
bdd----------------------------------------------------------------------+
| |
tdd-------------------------------------------------------------+ |
| | |
+----+--> pair programming --------+--> peer review -----> continuous integration
| ( collective ownership ) |
+--> individual programming ---+
Code Reviewing an Agile Project
If you are going to review an Agile Team project code, the best thing that you can do is give this guide to that Team as early as possible and most of your work will be done for you.
Code review must be done at least at the end of every story and be very fast in order to not introduce delays and detect any error as soon as possible. It's better to do that in every commit to the code repository. That is the reason that peer review is the prefered method for agile code review.
In order to review an Agile Project, you will have to extend the review to the tests.
Are there enough tests?
Is the code covered by the tests? Code coverage measure main value is to find unused code.
Are there the trivial tests? They are as needed as any other test.
Are there commented out tests? Commented out tests means that some one made a test that the code could not pass or take a long time to run.
Are boundary conditions tested? These are the tests around maximum and minimum values or near a change in a condition.
Are bugs exhaustively tested? That is, is an off-by-one bug is found, are there boundary tests for that condition?
Are the test automatic? If the tests requiere manual intervention, they will not be run.
Do the tests conform to F.I.R.S.T.?
Fast: a slow test is a candidate for removal, as it slows down the "make test, implement, test, refactor" cycle.
Independent: one test can not depend on the execution of another one, the order should not matter.
Repeatable: one test should give always the same result in any environment if nothing has changed in the code.
Self-validating: the result of a test should be Pass or Fail, nothing else. There should not be any manual intervention needed.
Timely: the test should be written before the code. That can not be detected with code review, but you can always see the logs of the versioning system.
Other Agile Resources
The role of testing
It is so fundamental, that the xDD pervades Agile, test first, test earlier
Continuous integration
it triggers the tests and often static code analysis too. it makes detection of integration problems arise very quickly.
The role of automatic static code analysis in the Agile Methodologies
Test Driven Development
It aims at code simplicity due to the need of making it testeable
Behavior Driven Development
....
Domain Driven Design
....
Refactoring
Refactoring is the art of changing the code with out changing its behavior. In order to refactor, there must exists a rich battery of tests.
The problem with refactoring, is that thanks to the heavy testing, you can trust that the interface does not change, but behind it, the code can be very volatile. You have to review the code continously, another argument for peer review and automatic static analysis.
References and sources
Clean Code: A handbook for Agile Software Craftsmanship - Robert C. Martin
http://groups.yahoo.com/group/foro-agiles/
http://martinfowler.com/articles/continuousIntegration.html
http://martinfowler.com/bliki/TestCoverage.html
not used http://refactoring.com/
not used http://dddcommunity.org/
