On 07/20/2011 01:09 PM, Pradeep Kumar wrote:

Hi, I have comments on this one, similar in nature to those made for the 
blast test. Please clean them both and send them back to us.

>  From 12b6d2b088058d308ff18553a156b6ec629e1c3d Mon Sep 17 00:00:00 2001
> From: pradeep<[email protected]>
> Date: Wed, 20 Jul 2011 21:27:21 +0530
> Subject: [PATCH]     [Autotest][Patch]: Add connecthon test
>
>       Signed-off-by: Poornima Nayak<[email protected]>
>       Modifiled-by: Pradeep K Surisetty<[email protected]>
>
>       new file:   client/tests/connecthon/connecthon.py
        new file:   client/tests/connecthon/control

^ Added lines should not be part of the commit messages.

> ---
>   client/tests/connecthon/connecthon.py |   72 
> +++++++++++++++++++++++++++++++++
>   client/tests/connecthon/control       |   26 ++++++++++++
>   2 files changed, 98 insertions(+), 0 deletions(-)
>   create mode 100755 client/tests/connecthon/connecthon.py
>   create mode 100755 client/tests/connecthon/control
>
> diff --git a/client/tests/connecthon/connecthon.py 
> b/client/tests/connecthon/connecthon.py
> new file mode 100755
> index 0000000..c482f56
> --- /dev/null
> +++ b/client/tests/connecthon/connecthon.py
> @@ -0,0 +1,72 @@
> +import os, shutil, glob, logging
> +from autotest_lib.client.bin import test, utils
> +from autotest_lib.client.common_lib import error
> +
> +class connecthon(test.test):
> +    """
> +    Connecthon test is nfs testsuite which can run on
> +    both BSD and System V based systems. The tests.init file
> +    has to be modified based on the OS in which this test is run.
> +    The tar file in this dir has init file which works for Linux
> +    platform. As well as it has some minor fixes for the testsuit
> +
> +    @author Poornima.Nayak([email protected])(original code)
> +    """
> +
> +    version=1
> +    def initialize(self):
> +        """
> +        Sets the overall failure counter for the test.
> +        """
> +        self.nfail = 0
> +
> +    def run_once(self, tarball = None, testdir = None, args = '', 
> iteration=1):
> +        """
> +        Runs the test, with the appropriate control file.
> +        """

^ Inconsistent spacing

> +        # Report the parameters we've received and write them as keyvals
> +        logging.debug("Test parameters: %s %s %s"%(iteration, args,testdir))
> +
> +        if (not os.path.exists('cthon04')):
> +            if not tarball:
> +                tarball = 'cthon04.tgz'
> +            print "tarball is ", tarball

^ What's the purpose of this?

> +            connecthon_tarball = utils.unmap_url(self.bindir, tarball, 
> self.tmpdir)
> +            print "connecthon tarball", connecthon_tarball

^ Rather than print statements, we should use logging.info, 
logging.debug statements. Logging is even imported at the top of the file.

> +            utils.extract_tarball_to_dir(connecthon_tarball, self.srcdir)
> +
> +            os.chdir(self.srcdir)
> +            utils.system('make clean')
> +            utils.system('make')
> +        else:
> +            os.chdir(self.srcdir)
> +
> +        if not testdir:
> +            testdir = self.tmpdir
> +
> +        try:
> +            if args == '':
> +                #run basic test
> +                args="-b -t"
> +
> +            self.results = utils.system_output('./runtests -N %s %s %s' 
> %(iteration,args,testdir))
> +            #self.results_path = os.path.join(self.resultsdir,
> +                                         #'raw_output_%s' % self.iteration)
> +            #self.analysisdir = os.path.join(self.resultsdir,
> +                                        #'analysis_%s' % self.iteration)
> +
> +            #utils.open_write_close(self.results_path, self.results)

^ Not a good practice to have a lot of commented lines. Finished code 
should have (hopefully) no commented lines of *code* (comments are OK, 
of course).

> +        except Exception, e:
> +            self.nfail += 1
> +            logging.error("Test failed: %s", e)
> +
> +
> +    def cleanup(self):
> +        """
> +        Cleans up source directory and raises on failure.
> +        """
> +        if os.path.isdir(self.srcdir):
> +            shutil.rmtree(self.srcdir)
> +        if self.nfail != 0:
> +            raise error.TestFail('Connecthon test suite failed.')
> diff --git a/client/tests/connecthon/control b/client/tests/connecthon/control
> new file mode 100755
> index 0000000..6fd3689
> --- /dev/null
> +++ b/client/tests/connecthon/control
> @@ -0,0 +1,26 @@
> +AUTHOR = "Poornima Nayak<[email protected]"
> +NAME = "connecthon"
> +TEST_CATEGORY = "NFS FVT"
> +TEST_CLASS = "General"
> +TEST_TYPE = "client"
> +TIME = 'MEDIUM'
> +DOC='''
> +Test for testing nfs mounted path.
> +
> +More information about connecthon can be found at
> +http://www.connectathon.org/nfstests.html
> +'''
> +
> +import datetime
> +now = datetime.datetime.now()
> +#iter_range=[10, 100, 1000, 10000]
> +iter_range=[1]
> +#tests=['-b -t', '-g -t', '-g -f', '-s -t', '-s -f', '-l -f', '-l -t']
> +tests=['-s -t']
> +tag_ver=0
> +for test in tests:
> +    for j in iter_range:
> +        dir = '/mnt/test'+now.strftime("%Y-%m-%d%H:%M")

^ On assignments, the string addition should have spaces separating the 
operators.

> +        
> job.run_test('connecthon',tarball=None,testdir=dir,args=test,iteration=j,tag=("itera-%s-test-%s"
>  %(j,tag_ver)))
> +        tag_ver=tag_ver + 1
> +

_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to