On 12/16/2012 12:56 AM, Rajeev S wrote:
The crontab.pyc has been gitignored.
Thanks and sorry for the delay. Below you can find some comments, I'd
like you to comment/work on the points raised, and resend the code with
them fixed.
---
crontab/control | 22 ++++++++
crontab/crontab.py | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 164 insertions(+)
create mode 100644 crontab/control
create mode 100644 crontab/crontab.py
diff --git a/crontab/control b/crontab/control
new file mode 100644
index 0000000..6d605a3
--- /dev/null
+++ b/crontab/control
@@ -0,0 +1,22 @@
+AUTHOR = "Rajeev S <[email protected]>"
+NAME = "crontab"
+TEST_CATEGORY = "Functional"
+TEST_CLASS = "Kernel"
+TEST_TYPE = "client"
+TIME = 'SHORT'
+DOC = '''
+Test for testing the crontab.
+
+Uses run-parts to execute the scripts under the standard cron
directories,/etc/cron.xxxx.
+Automation is done using the /etc/crontab file.
+
+Args:
+ wait_time:Duration of the sleep time.Should be greater than 60(sec)
+'''
+from datetime import datetime
+
+LOGFILE='/tmp/autotest_cron-%s'%(str(datetime.now())[:10])
^ I think you're better off with the more lightweight time module,
something like:
LOGFILE = '/tmp/autotest_cron-%s' % time.strftime('%Y-%m-%d-%H.%M.%S')
+
+tests=['normalCron','denyCron','allowCron']
+for i in range(0,3):
+ job.run_test('crontab',test=tests[i],wait_time=65,tag=tests[i],log=LOGFILE)
^ This is a nitpick, but please put spaces right after the commas, to
make it comply with PEP8. Autotest does not follow PEP8 in all aspects,
but the exceptions can be found on the CODING_STYLE document in the top
of the autotest tree.
diff --git a/crontab/crontab.py b/crontab/crontab.py
new file mode 100644
index 0000000..0ef509a
--- /dev/null
+++ b/crontab/crontab.py
@@ -0,0 +1,142 @@
+#!/bin/python
+import os, shutil, glob, logging
+from autotest.client import test, utils
+from autotest.client.shared import error
+from time import sleep
+
+
+class crontab(test.test):
+ '''
+ Autotest module for crontab.
+
+ Creates a new cron job that creates a log entry in log.The count of
+ log entry is compared before and after a period of 80 sec.Difference decide
+ s whether the cron executed
+ successfully.
+
+ @author :Rajeev S <[email protected]>
+ '''
+ version = 1
+ initialCount = 0
+ log=''
^ Same here, make sure there's proper spacing between each side of the
assignment:
log = ''
I trust you will fix all other places.
+ def count_log(self,string):
+ '''returns count of the 'string' in log'''
+ count=0
+ try:
+ f=open(self.log)
+ except IOError:
+ utils.open_write_close(self.log,'')
+ f=open(self.log)
+ for i in f.readlines():
+ if string in i:
+ count+=1
+ f.close()
+ return count
+
+ def initialize(self,test,log):
+ '''Does the init part of the test
+ 1.Finds initial count of entry in log
+ 2.Creates a file 'cron' under cron.d
+ 3.Backs up /etc/crontab
+ 4.Modifies /etc/crontab '''
+ self.log=log
+
+ self.initialCount=self.count_log('Cron automation')
^ One more word about coding style: CamelCase is OK only for class
names, and some corner case that really justifies it (in this case,
negative).
+ f=open('/etc/cron.d/cron','w')
+ f.write('''#!/bin/bash
+touch %s
+echo 'Cron automation' >> %s
+ '''%(self.log,self.log))
+ f.close()
+ utils.system('chmod +x /etc/cron.d/cron')
+ utils.system('cat /etc/crontab > /tmp/backup')
+ f=open('/etc/crontab','w')
+ f.write('* * * * * root run-parts /etc/cron.d/\n')
+ f.close()
+ if test == 'denyCron':
+ if os.path.exists('/etc/cron.d/jobs.deny'):
+ utils.system('mv /etc/cron.d/jobs.deny /tmp/jobs.deny
2>/dev/null')
+ f=open('/etc/cron.d/jobs.deny','w')
+ f.write('cron')
+ f.close()
+ elif test =='allowCron' :
+ utils.system('rm /etc/cron.d/jobs.deny')
+ if os.path.exists('/etc/cron.d/jobs.allow'):
+ utils.system('mv /etc/cron.d/jobs.allow /tmp/jobs.allow
2>/dev/null')
+ f=open('/etc/cron.d/jobs.allow','w')
+ f.write('cron')
+ f.close()
+
+
+ def allowCron(self,wait_time):
^ Let's not mix CamelCase with lowercase_with_underscores, at least in
method names. Turn it all to lowercase_with_underscores.
+ if self.count_log('Cron automation')>self.initialCount:
+ utils.open_write_close(self.results_path,'''Test Successful.
+ Test time:%s
+ Test mode:2(run-parts with jobs.allow)'''%(wait_time))
+ else:
+ utils.open_write_close(self.results_path,'''Test Failed.
+ There were no new entries in log.
+ Job not executed inspite of jobs.allow entry.
+ Test time:%s
+ Test mode:2 (run-parts with jobs.allow)'''%(wait_time))
^ Hmm, interesting, you are creating some custom results file in the
results directory. Any reason in particular you want to do this?
In autotest, it is more customary to:
1) Have a global failure counter
2) Execute the tests, when they fail store failure in the failure counter
3) When the test ends, verify if there's something in the failure
counter, and then fail things when there is.
+
+ def normalCron(self,wait_time):
+ if self.count_log('Cron automation')>self.initialCount:
+ utils.open_write_close(self.results_path,'''Test successful.
+ Test time:%s
+ Test mode:0(normal test for run-parts)'''%(wait_time))
+ else:
+ utils.open_write_close(self.results_path,'''Test Failed.
+ There were no new entries in log.
+ Test time:%s
+ Test mode:0 (normal test for run-parts)'''%(wait_time))
+
+
+ def denyCron(self,wait_time):
+ if self.count_log('Cron automation')>self.initialCount:
+ utils.open_write_close(self.results_path,'''Test Failed.
+ run-parts overrides jobs.deny.
+ Test time:%s
+ Test mode:1(run-parts with jobs.deny)'''%(wait_time))
+ else:
+ utils.open_write_close(self.results_path,'''Test Successful.
+ There were no new entries in log.
+ Test time:%s
+ Test mode:1 (run-parts with jobs.deny)'''%(wait_time))
+
+
+ def make_fun(self,test,wait_time):
+ '''Calls the function with the name <test> '''
+ test=getattr(self,test)
+ test(wait_time)
You don't really need this intermediate, you can do this directly in
run_once.
+
+ def run_once(self,test,wait_time):
+ '''Runs the test,writes test success if cron successfully executes,else
+ writes test failed.
+ Resets /etc/crontab
+ Pass 0:Normal operation of run-parts
+ Pass 1:run-parts with jobs.deny
+ Pass 2:run-parts with jobs.allow
+ '''
+ os.chdir(self.srcdir)
+ self.test=test
+ self.results_path = os.path.join(self.resultsdir,
+ 'raw_output_mode_%s'%test)
+ try:
+ sleep(wait_time)
+ self.make_fun(test,wait_time)
+ except Exception as e:
+ logging.error('\nTest failed:%s'%e)
Problem 1: The subtests don't raise exceptions explicitly, to tell
whether they've failed or passed.
Problem 2: Your test *never* fails, which is a bad practice. You *must*
make the test fail in the end of the execution (if it actually failed,
of course), by raising error.TestFail().
...
For the below, remember one of our unwritten golden rules (I should
write this somewhere):
1) Thou shall not call external shell commands when there's a python API
that does the same thing. So all calls to
* cp
* mv
* cat
Are probably better of made using their python counterparts:
* shutil.copy or copytree
* shutils.move
* read()
+ def cleanup(self):
+ utils.system('cat /tmp/backup > /etc/crontab')
^ Better to use shutils to do this.
+ if os.path.exists('/etc/cron.d/jobs.allow'):
+ utils.system('rm /etc/cron.d/jobs.allow /etc/cron.d/cron')
^ Better to use os.remove() instead.
+ else:
+ utils.system('rm /etc/cron.d/cron')
+ if os.path.exists('/tmp/jobs.allow'):
+ utils.system('mv /tmp/jobs.allow /etc/cron.d/jobs.allow
2>/dev/null')
+ if os.path.exists('/tmp/jobs.deny'):
+ utils.system('mv /tmp/jobs.allow /etc/cron.d/jobs.allow
2>/dev/null')
Ditto.
_______________________________________________
Autotest-kernel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/autotest-kernel
