A testing policy

[Andrew Bartlett]

G'Day,

With all this recent discussion of testing, I thought I would see how
well I could hoist myself on my own petard and propose in writing the
testing policy we discussed at SambaXP last year.  I'm very sorry for
taking so long to write this up.

To give an example of what I was hoping it would avoid, see this:

commit 7ddf47951bd472841c5f365e5fff7d28b1ce4972
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Sep 11 21:39:44 2017 +1200

    scripting: Add script (backportable) to undo a GUID index
    
    This script allows the DB to be read, and re-indexed, by an earlier Samba version,
    most likely 4.7 with some backported patches.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>
    

Harmless enough, but given some folks really will rely on it to
downgrade to Samba 4.7 from 4.8, I should have had a test that
confirmed it really did what it claimed to do.  (By opening the DB as a
TDB and checking the domain DN existed as a key). 

We all write great code that we are very confident of and most of us as
developers and reviewers stick pretty strictly to the idea that we
should write tests for our changes.  This formalises it in a way we can
point to for external evaluation. 

Please comment and review!

Thanks,

Andrew Bartlett
-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
From 7f30a6730e23172ef8b391253d1a3be67b3283af Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 22 Feb 2018 11:43:13 +1300
Subject: [PATCH] Add Samba Testing Policy to README.Coding

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 README.Coding | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/README.Coding b/README.Coding
index e89925cad26..a928aa51b60 100644
--- a/README.Coding
+++ b/README.Coding
@@ -512,3 +512,53 @@ DBG_DEBUG("Received %d bytes\n", count);
 
 The messages from these macros are automatically prefixed with the
 function name.
+
+
+====================
+Samba Testing Policy
+====================
+
+"Untested code is broken code"
+
+Samba has a comprehensive Continuous Integration system invoked on
+every push to master.  Samba also has a strict Code Review policy.
+
+However this alone is not enough to ensure changes to Samba are
+correct, because CI only checks that previously written tests still
+pass, and code review only confirms that the new code looks like it
+does what it says it does.  
+
+Therefore: Code changes in Samba must be tested, and that testing
+should be automated.
+
+Specifically, patches implementing new features, changing behaviour or
+fixing bugs should have a comprehensive test-suite showing that the new
+feature, changed behaviour or bug fix is indeed as the author intends.
+
+While it is attractive (to the original developer) to say that their
+change has been manually tested or 'obviously correct', this is not
+sufficient, as otherwise future developers cannot rely on the CI
+process when making their own changes.
+
+Sometimes changes are not intended to change behaviour, and provided
+there are already tests, tasks like improving comments, writing
+documentation, fixing warnings or removing unused code do not need
+additional tests.
+
+There are always exceptions, these should be noted in the commit
+messages and discussed on the mailing list when seeking code review.
+
+However special appreciation is given to those who take the statement
+'this is impossible to (automatically) test' and instead prove that it
+can be tested, in the way cwrap evolved from the #define based
+socket_wrapper to cover other applications via LD_PRELOAD.
+
+Writing tests
+-------------
+
+There are helpful pointers on how to write Samba tests here:
+
+https://wiki.samba.org/index.php/Developer_Documentation#Debugging_.2F_Testing
+
+Tests for sub-projects like ldb, talloc and tdb should be in each
+sub-project, not in Samba.
-- 
2.11.0