Resiliency Tests: Catch errors while requesting to webrunner and Fix tests

/reviewed-on nexedi/slapos.toolbox!24

slapos/resiliencytest/__init__.py

@@ -88,7 +88,7 @@ def parseArguments():
 
   return parser.parse_args()
 
-def setupLogging(log_path=None, name=__name__):
+def setupLogging(name=__name__, log_path=None):
   logger_format = '%(asctime)s %(name)-13s: %(levelname)-8s %(message)s'
   formatter = logging.Formatter(logger_format)
   logging.basicConfig(level=logging.INFO, format=logger_format)

slapos/resiliencytest/suites/slaprunner.py

@@ -111,7 +111,10 @@ class SlaprunnerTestSuite(ResiliencyTestSuite):
         data="file=instance_root/slappart0/var/log/log.log"
     )
     try:
-        data = json.loads(data)['result']
+        json_data = json.loads(data)
+        if json_data['code'] is 0:
+            raise IOError(data['result'])
+        data = json_data['result']
         self.logger.info('Retrieved data are:\n%s' % data)
     except (ValueError, KeyError):
         if data.find('<') is not -1:
@@ -134,7 +137,7 @@ class SlaprunnerTestSuite(ResiliencyTestSuite):
     """
     data = self._connectToSlaprunner(
              resource='getFileLog',
-             data="filename=software.log&truncate=%s" % truncate)
+             data="filename=instance_root/../software.log&truncate=%s" % truncate)
     try:
       data = json.loads(data)['result']
       self.logger.info('Tail of software.log:\n%s' % data)
@@ -199,10 +202,14 @@ class SlaprunnerTestSuite(ResiliencyTestSuite):
   def _gitClone(self):
     self.logger.debug('Doing git clone of https://lab.nexedi.com/nexedi/slapos.git..')
     try:
-      self._connectToSlaprunner(
+      data = self._connectToSlaprunner(
           resource='cloneRepository',
           data='repo=https://lab.nexedi.com/nexedi/slapos.git&name=workspace/slapos&email=slapos@slapos.org&user=slapos'
       )
+      data = json.loads(data)
+      if data['code'] is 0:
+        self.logger.warning(data['result'])  
+
     except (NotHttpOkException, urllib2.HTTPError):
       # cloning can be very long.
       # XXX: quite dirty way to check.
@@ -211,10 +218,13 @@ class SlaprunnerTestSuite(ResiliencyTestSuite):
 
   def _openSoftwareRelease(self, software_release='erp5testnode/testsuite/dummy'):
     self.logger.debug('Opening %s software release...' % software_release)
-    self._connectToSlaprunner(
+    data = self._connectToSlaprunner(
         resource='setCurrentProject',
         data='path=workspace/slapos/software/%s/' % software_release
     )
+    if json.loads(data)['code'] is 0:
+      self.logger.warning(data['result'])
+      raise IOError(data['result'])
 
   def generateData(self):
     """

[Nicolas Wavrant]

I think this change is not useful (filename=software.log works and it is exactly what the function is expected) and make the test code harder to understand.

[Hardik Juneja]

The issue is the software_root was changed to ${buildout:directory}/../../soft : here

And runner was looking for software.log in the "software_root".

hence, I had to use this hack.

[Nicolas Wavrant]

Could you explain me the purpose of this function ? I thought it was trying to read the software.log written by the webrunner. It seems to me that you are talking about the software.log written by the test node.

Also, if you add hacks in the code, please notify it with an "#XXX" comment and explain the need.

[Hardik Juneja]

Sure. Yes, this function aims to retrieve the software.log of the dummy instance which is going to be build by the webrunner.

With "filename=software.log" passed in data, the "getFileLog" function will look for the file "software_root/software.log" see - [this] (https://lab.nexedi.com/hjuneja/slapos.toolbox/blob/master/slapos/runner/views.py#L239) and [this] (https://lab.nexedi.com/hjuneja/slapos.toolbox/blob/master/slapos/runner/run.py#L119)

Now since the software root of the dummy instance is changed to "${buildout:directory}/../../soft" it was looking to a file that don't exits. so I decided to use the instance_root to fetch the software.log

[Nicolas Wavrant]

I'm not sure this condition is trustable :

>>> a = {'a': 257}
>>> a is 257
False

Please always use "==" to test equality for integers

[Nicolas Wavrant]

Also, as we are in tests, I'm pretty sure a code equal to 0 means "there's an issue", and test could raise an AssertionError in that case (with an understandable error message). Then, the error will be much more noticeable when reading the test failure log (usually people just read the last line, don't scroll up, and don't even pay attention to warnings).

[Hardik Juneja]

true! My bad