AMBARI-2140. Improve Ambari-agent test coverage. (swagle)

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/trunk@1483448 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -12,6 +12,8 @@ Trunk (unreleased changes):
 
  NEW FEATURES
 
+ AMBARI-2140. Improve Ambari-agent test coverage. (swagle)
+
  AMBARI-2133. Add Nagios alerts for Hadoop 2.0 in Ambari. (swagle)
 
  AMBARI-2123. Allow the user to specify a non-root ssh user in Install Options.

ambari-agent/src/main/python/ambari_agent/Controller.py

@@ -59,13 +59,6 @@ class Controller(threading.Thread):
     self.cachedconnect = None
     self.range = range
 
-  def start(self):
-    self.actionQueue = ActionQueue(self.config)
-    self.actionQueue.start()
-    self.register = Register(self.config)
-    self.heartbeat = Heartbeat(self.actionQueue)
-    pass
-  
   def __del__(self):
     logger.info("Server connection disconnected.")
     pass
@@ -206,6 +199,11 @@ class Controller(threading.Thread):
     pass
 
   def run(self):
+    self.actionQueue = ActionQueue(self.config)
+    self.actionQueue.start()
+    self.register = Register(self.config)
+    self.heartbeat = Heartbeat(self.actionQueue)
+
     opener = urllib2.build_opener()
     urllib2.install_opener(opener)
 

ambari-agent/src/main/python/ambari_agent/UpgradeExecutor.py

@@ -139,9 +139,9 @@ class UpgradeExecutor:
     stack_name = verdict["stackName"].strip()
 
     matchObj = re.match( r'(\d+).(\d+)', verdict["stackVersion"].strip(), re.M|re.I)
-    stack_major_ver = matchObj.group(1)
-    stack_minor_ver = matchObj.group(2)
     if matchObj:
+      stack_major_ver = matchObj.group(1)
+      stack_minor_ver = matchObj.group(2)
       return stack_name, stack_major_ver, stack_minor_ver
     else:
       return None

ambari-agent/src/main/python/ambari_agent/main.py

@@ -33,10 +33,13 @@ from shell import killstaleprocesses
 import AmbariConfig
 from security import CertificateManager
 from NetUtil import NetUtil
+import security
 
 
 logger = logging.getLogger()
+formatstr = "%(levelname)s %(asctime)s %(filename)s:%(lineno)d - %(message)s"
 agentPid = os.getpid()
+configFile = "/etc/ambari-agent/conf/ambari-agent.ini"
 
 if 'AMBARI_LOG_DIR' in os.environ:
   logfile = os.environ['AMBARI_LOG_DIR'] + "/ambari-agent.log"
@@ -46,38 +49,30 @@ else:
 def signal_handler(signum, frame):
   #we want the handler to run only for the agent process and not
   #for the children (e.g. namenode, etc.)
-  if (os.getpid() != agentPid):
+  if os.getpid() != agentPid:
     os._exit(0)
   logger.info('signal received, exiting.')
   ProcessHelper.stopAgent()
 
 def debug(sig, frame):
-    """Interrupt running process, and provide a python prompt for
-    interactive debugging."""
-    d={'_frame':frame}         # Allow access to frame object.
-    d.update(frame.f_globals)  # Unless shadowed by global
-    d.update(frame.f_locals)
+  """Interrupt running process, and provide a python prompt for
+  interactive debugging."""
+  d={'_frame':frame}         # Allow access to frame object.
+  d.update(frame.f_globals)  # Unless shadowed by global
+  d.update(frame.f_locals)
 
-    message  = "Signal received : entering python shell.\nTraceback:\n"
-    message += ''.join(traceback.format_stack(frame))
-    logger.info(message)
+  message  = "Signal received : entering python shell.\nTraceback:\n"
+  message += ''.join(traceback.format_stack(frame))
+  logger.info(message)
 
 
-
-
-def main():
-  global config
-  parser = OptionParser()
-  parser.add_option("-v", "--verbose", dest="verbose", action="store_true", help="verbose log output", default=False)
-  (options, args) = parser.parse_args()
-
-  formatstr = "%(levelname)s %(asctime)s %(filename)s:%(lineno)d - %(message)s"
+def setup_logging(verbose):
   formatter = logging.Formatter(formatstr)
   rotateLog = logging.handlers.RotatingFileHandler(logfile, "a", 10000000, 25)
   rotateLog.setFormatter(formatter)
   logger.addHandler(rotateLog)
 
-  if options.verbose:
+  if verbose:
     logging.basicConfig(format=formatstr, level=logging.DEBUG, filename=logfile)
     logger.setLevel(logging.DEBUG)
     logger.info("loglevel=logging.DEBUG")
@@ -86,43 +81,45 @@ def main():
     logger.setLevel(logging.INFO)
     logger.info("loglevel=logging.INFO")
 
-  default_cfg = { 'agent' : { 'prefix' : '/home/ambari' } }
-  config = ConfigParser.RawConfigParser(default_cfg)
+
+def update_log_level(config):
+  # Setting loglevel based on config file
+  try:
+    loglevel = config.get('agent', 'loglevel')
+    if loglevel is not None:
+      if loglevel == 'DEBUG':
+        logging.basicConfig(format=formatstr, level=logging.DEBUG, filename=logfile)
+        logger.setLevel(logging.DEBUG)
+        logger.info("Newloglevel=logging.DEBUG")
+      else:
+        logging.basicConfig(format=formatstr, level=logging.INFO, filename=logfile)
+        logger.setLevel(logging.INFO)
+        logger.debug("Newloglevel=logging.INFO")
+  except Exception, err:
+    logger.info("Default loglevel=DEBUG")
+
+
+def bind_signal_handlers():
   signal.signal(signal.SIGINT, signal_handler)
   signal.signal(signal.SIGTERM, signal_handler)
   signal.signal(signal.SIGUSR1, debug)
 
-  if (len(sys.argv) >1) and sys.argv[1]=='stop':
-    # stop existing Ambari agent
-    pid = -1
-    try:
-      f = open(ProcessHelper.pidfile, 'r')
-      pid = f.read()
-      pid = int(pid)
-      f.close()
-      os.kill(pid, signal.SIGTERM)
-      time.sleep(5)
-      if os.path.exists(ProcessHelper.pidfile):
-        raise Exception("PID file still exists.")
-      os._exit(0)
-    except Exception, err:
-      if pid == -1:
-        print ("Agent process is not running")
-      else:
-        os.kill(pid, signal.SIGKILL)
-      os._exit(1)
 
-  # Check for ambari configuration file.
+def resolve_ambari_config():
   try:
     config = AmbariConfig.config
-    if os.path.exists('/etc/ambari-agent/conf/ambari-agent.ini'):
-      config.read('/etc/ambari-agent/conf/ambari-agent.ini')
+    if os.path.exists(configFile):
+      config.read(configFile)
       AmbariConfig.setConfig(config)
     else:
       raise Exception("No config found, use default")
+
   except Exception, err:
     logger.warn(err)
+  return config
 
+
+def perform_prestart_checks():
   # Check if there is another instance running
   if os.path.isfile(ProcessHelper.pidfile):
     print("%s already exists, exiting" % ProcessHelper.pidfile)
@@ -134,29 +131,60 @@ def main():
     logger.error(msg)
     print(msg)
     sys.exit(1)
-  else:
-    # Daemonize current instance of Ambari Agent
-    pid = str(os.getpid())
-    file(ProcessHelper.pidfile, 'w').write(pid)
 
-  credential = None
 
-  killstaleprocesses()
+def daemonize():
+  # Daemonize current instance of Ambari Agent
+  # Currently daemonization is done via /usr/sbin/ambari-agent script (nohup)
+  # and agent only dumps self pid to file
+  pid = str(os.getpid())
+  file(ProcessHelper.pidfile, 'w').write(pid)
 
-  # Setting loglevel based on config file
+
+def stop_agent():
+# stop existing Ambari agent
+  pid = -1
   try:
-    loglevel = config.get('agent', 'loglevel')
-    if loglevel is not None:
-      if loglevel == 'DEBUG':
-        logging.basicConfig(format=formatstr, level=logging.DEBUG, filename=logfile)
-        logger.setLevel(logging.DEBUG)
-        logger.info("Newloglevel=logging.DEBUG")
-      else:
-        logging.basicConfig(format=formatstr, level=logging.INFO, filename=logfile)
-        logger.setLevel(logging.INFO)
-        logger.debug("Newloglevel=logging.INFO")
+    f = open(ProcessHelper.pidfile, 'r')
+    pid = f.read()
+    pid = int(pid)
+    f.close()
+    os.kill(pid, signal.SIGTERM)
+    time.sleep(5)
+    if os.path.exists(ProcessHelper.pidfile):
+      raise Exception("PID file still exists.")
+    os._exit(0)
   except Exception, err:
-    logger.info("Default loglevel=DEBUG")
+    if pid == -1:
+      print ("Agent process is not running")
+    else:
+      os.kill(pid, signal.SIGKILL)
+    os._exit(1)
+
+
+def main():
+  global config
+  parser = OptionParser()
+  parser.add_option("-v", "--verbose", dest="verbose", action="store_true", help="verbose log output", default=False)
+  (options, args) = parser.parse_args()
+
+  setup_logging(options.verbose)
+
+  default_cfg = { 'agent' : { 'prefix' : '/home/ambari' } }
+  config = ConfigParser.RawConfigParser(default_cfg)
+  bind_signal_handlers()
+
+  if (len(sys.argv) >1) and sys.argv[1]=='stop':
+    stop_agent()
+
+  # Check for ambari configuration file.
+  config = resolve_ambari_config()
+  perform_prestart_checks()
+  daemonize()
+
+  killstaleprocesses()
+
+  update_log_level(config)
 
   server_url = 'https://' + config.get('server', 'hostname') + ':' + config.get('server', 'url_port')
   print("Connecting to the server at " + server_url + "...")
@@ -169,15 +197,13 @@ def main():
   #Initiate security
   """ Check if security is enable if not then disable it"""
   logger.info("Creating certs")
-  certMan = CertificateManager(config)
+  certMan = security.CertificateManager(config)
   certMan.initSecurity()
-  
+
   # Launch Controller communication
   controller = Controller(config)
   controller.start()
-  # TODO: is run() call necessary?
-  controller.run()
   logger.info("finished")
-    
+
 if __name__ == "__main__":
   main()

ambari-agent/src/main/python/ambari_agent/security.py

@@ -22,7 +22,7 @@ import socket
 import ssl
 import os
 import logging
-from subprocess import Popen, PIPE
+import subprocess
 import json
 import pprint
 import traceback
@@ -41,8 +41,8 @@ class VerifiedHTTPSConnection(httplib.HTTPSConnection):
   def __init__(self, host, port=None, key_file=None, cert_file=None,
                      strict=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT):
     httplib.HTTPSConnection.__init__(self, host, port=port)
-    pass
-     
+    self.certMan = CertificateManager(AmbariConfig.config)
+
   def connect(self):
     if self.sock:
       self.sock.close()
@@ -52,12 +52,9 @@ class VerifiedHTTPSConnection(httplib.HTTPSConnection):
     if self._tunnel_host:
       self.sock = sock
       self._tunnel()
-    agent_key = AmbariConfig.config.get('security', 'keysdir') + os.sep + \
-     hostname.hostname() + ".key"
-    agent_crt = AmbariConfig.config.get('security', 'keysdir') + os.sep \
-    + hostname.hostname() + ".crt" 
-    server_crt = AmbariConfig.config.get('security', 'keysdir') + os.sep \
-    + "ca.crt"
+    agent_key = self.certMan.getAgentKeyName()
+    agent_crt = self.certMan.getAgentCrtName()
+    server_crt = self.certMan.getSrvrCrtName()
     
     self.sock = ssl.wrap_socket(sock,
                                 keyfile=agent_key,
@@ -77,11 +74,12 @@ class CachedHTTPSConnection:
     self.connect()
   
   def connect(self):
-      if  not self.connected:
-        self.httpsconn = VerifiedHTTPSConnection(self.server, self.port)
-        self.httpsconn.connect()
-        self.connected = True
-      # possible exceptions are caught and processed in Controller
+    if  not self.connected:
+      self.httpsconn = VerifiedHTTPSConnection(self.server, self.port)
+      self.httpsconn.connect()
+      self.connected = True
+    # possible exceptions are caught and processed in Controller
+
 
   
   def forceClear(self):
@@ -115,12 +113,15 @@ class CertificateManager():
   def getAgentKeyName(self):
     keysdir = self.config.get('security', 'keysdir')
     return keysdir + os.sep + hostname.hostname() + ".key"
+
   def getAgentCrtName(self):
     keysdir = self.config.get('security', 'keysdir')
     return keysdir + os.sep + hostname.hostname() + ".crt"
+
   def getAgentCrtReqName(self):
     keysdir = self.config.get('security', 'keysdir')
     return keysdir + os.sep + hostname.hostname() + ".csr"
+
   def getSrvrCrtName(self):
     keysdir = self.config.get('security', 'keysdir')
     return keysdir + os.sep + "ca.crt"
@@ -148,10 +149,10 @@ class CertificateManager():
     agent_crt_exists = os.path.exists(self.getAgentCrtName())
     
     if not agent_crt_exists:
-        logger.info("Agent certificate not exists, sending sign request")
-        self.reqSignCrt()
+      logger.info("Agent certificate not exists, sending sign request")
+      self.reqSignCrt()
     else:
-        logger.info("Agent certificate exists, ok")
+      logger.info("Agent certificate exists, ok")
             
   def loadSrvrCrt(self):
     get_ca_url = self.server_url + '/cert/ca/'
@@ -183,14 +184,15 @@ class CertificateManager():
       agentCrtF = open(self.getAgentCrtName(), "w")
       agentCrtF.write(agentCrtContent)
     else:
+      # Possible exception is catched higher at Controller
       logger.error("Certificate signing failed")
 
   def genAgentCrtReq(self):
     generate_script = GEN_AGENT_KEY % {'hostname': hostname.hostname(),
                                      'keysdir' : self.config.get('security', 'keysdir')}
     logger.info(generate_script)
-    p = Popen([generate_script], shell=True, stdout=PIPE)
-    p.wait()
+    p = subprocess.Popen([generate_script], shell=True, stdout=subprocess.PIPE)
+    p.communicate()
       
   def initSecurity(self):
     self.checkCertExists()

ambari-agent/src/test/python/TestController.py

@@ -21,7 +21,7 @@ limitations under the License.
 
 import StringIO
 import unittest
-from ambari_agent import Controller
+from ambari_agent import Controller, ActionQueue
 from ambari_agent import hostname
 import sys
 from mock.mock import patch, MagicMock, call
@@ -51,20 +51,6 @@ class TestController(unittest.TestCase):
     self.controller.netutil.HEARTBEAT_NOT_IDDLE_INTERVAL_SEC = 0.1
 
 
-  @patch.object(Controller, "Heartbeat")
-  @patch.object(Controller, "Register")
-  @patch.object(Controller, "ActionQueue")
-  def test_start(self, ActionQueue_mock, Register_mock, Heartbeat_mock):
-
-    aq = MagicMock()
-    ActionQueue_mock.return_value = aq
-
-    self.controller.start()
-    self.assertTrue(ActionQueue_mock.called)
-    self.assertTrue(aq.start.called)
-    self.assertTrue(Register_mock.called)
-    self.assertTrue(Heartbeat_mock.called)
-
   @patch("json.dumps")
   @patch("json.loads")
   @patch("time.sleep")
@@ -128,7 +114,10 @@ class TestController(unittest.TestCase):
 
   @patch("urllib2.build_opener")
   @patch("urllib2.install_opener")
-  def test_run(self, installMock, buildMock):
+  @patch.object(Controller, "ActionQueue")
+  def test_run(self, ActionQueue_mock, installMock, buildMock):
+    aq = MagicMock()
+    ActionQueue_mock.return_value = aq
 
     buildMock.return_value = "opener"
     registerAndHeartbeat  = MagicMock("registerAndHeartbeat")
@@ -152,8 +141,15 @@ class TestController(unittest.TestCase):
     self.controller.run()
     self.assertEqual(3, registerAndHeartbeat.call_count)
 
+    # Action queue should be started during calls
+    self.assertTrue(ActionQueue_mock.called)
+    self.assertTrue(aq.start.called)
+
 
-  def test_heartbeatWithServer(self, installMock, buildMock):
+  @patch("urllib2.build_opener")
+  @patch("urllib2.install_opener")
+  @patch.object(ActionQueue.ActionQueue, "run")
+  def test_repeatRegistration(self, run_mock, installMock, buildMock):
 
     registerAndHeartbeat = MagicMock(name="registerAndHeartbeat")
 

ambari-agent/src/test/python/TestGrep.py

@@ -19,7 +19,7 @@ limitations under the License.
 '''
 
 from unittest import TestCase
-from Grep import Grep
+from ambari_agent.Grep import Grep
 import socket
 import os, sys
 import logging

ambari-agent/src/test/python/TestHeartbeat.py

@@ -28,9 +28,22 @@ import os
 import time
 from mock.mock import patch, MagicMock, call
 from ambari_agent.StackVersionsFileHandler import StackVersionsFileHandler
+import StringIO
+import sys
 
 class TestHeartbeat(TestCase):
 
+  def setUp(self):
+    # disable stdout
+    out = StringIO.StringIO()
+    sys.stdout = out
+
+
+  def tearDown(self):
+    # enable stdout
+    sys.stdout = sys.__stdout__
+
+
   def test_build(self):
     actionQueue = ActionQueue(AmbariConfig.AmbariConfig().getConfig())
     heartbeat = Heartbeat(actionQueue)

ambari-agent/src/test/python/TestLiveStatus.py

@@ -22,9 +22,20 @@ from unittest import TestCase
 from ambari_agent.LiveStatus import LiveStatus
 from ambari_agent.AmbariConfig import AmbariConfig
 import socket
-import os
+import os, sys, StringIO
 
 class TestLiveStatus(TestCase):
+
+  def setUp(self):
+    # disable stdout
+    out = StringIO.StringIO()
+    sys.stdout = out
+
+
+  def tearDown(self):
+    # enable stdout
+    sys.stdout = sys.__stdout__
+
   def test_build(self):
     for component in LiveStatus.COMPONENTS:
       config = AmbariConfig().getConfig()

ambari-agent/src/test/python/TestManifestGenerator.py

@@ -17,7 +17,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 '''
-import os
+import os, sys, StringIO
 
 from unittest import TestCase
 from ambari_agent import manifestGenerator
@@ -32,16 +32,21 @@ from mock.mock import patch, MagicMock, call
 class TestManifestGenerator(TestCase):
 
   def setUp(self):
+    # disable stdout
+    out = StringIO.StringIO()
+    sys.stdout = out
+
     self.dir = tempfile.mkdtemp()
     self.config = AmbariConfig()
     jsonCommand = file('../../main/python/ambari_agent/test.json').read()
     self.parsedJson = json.loads(jsonCommand)
 
-    pass
 
   def tearDown(self):
     shutil.rmtree(self.dir)
-    pass
+
+    # enable stdout
+    sys.stdout = sys.__stdout__
 
 
   def testWriteImports(self):

ambari-agent/src/test/python/TestPuppetExecutorManually.py

@@ -19,7 +19,7 @@ limitations under the License.
 '''
 
 from unittest import TestCase
-from PuppetExecutor import PuppetExecutor
+from ambari_agent.PuppetExecutor import PuppetExecutor
 from pprint import pformat
 import socket
 import os

ambari-agent/src/test/python/TestStackVersionsFileHandler.py

@@ -82,5 +82,35 @@ class TestStackVersionsFileHandler(TestCase):
     self.assertEqual(result, True)
 
 
+  def test_write_stack_version(self):
+    #saving old values
+    oldFilePathValue = stackVersionsFileHandler.versionsFilePath
+    oldversionsFileDir = stackVersionsFileHandler.versionsFileDir
+    oldVerFile = stackVersionsFileHandler.VER_FILE
+    #preparations and invocation
+    tmpfile = tempfile.mktemp()
+    stackVersionsFileHandler.versionsFilePath = tmpfile
+    stackVersionsFileHandler.VER_FILE = \
+      os.path.basename(tmpfile)
+    stackVersionsFileHandler.versionsFileDir = \
+      os.path.dirname(tmpfile)
+    stackVersionsFileHandler.touch_file()
+    stackVersionsFileHandler.write_stack_version(
+      "NAGIOS_SERVER", '"stackVersion":"1.3.0"')
+    # Checking if backup file exists
+    expectedBackupFile = tmpfile + ".bak"
+    self.assertTrue(os.path.isfile(expectedBackupFile))
+    os.remove(expectedBackupFile)
+    # Checking content of created file
+    content = stackVersionsFileHandler.read_all_stack_versions()
+    self.assertEquals(len(content), 1)
+    self.assertEqual(content['NAGIOS_SERVER'], '"stackVersion":"1.3.0"')
+    self.assertTrue(os.path.isfile(tmpfile))
+    os.remove(tmpfile)
+    # Restoring old values
+    stackVersionsFileHandler.versionsFilePath = oldFilePathValue
+    stackVersionsFileHandler.versionsFileDir = oldversionsFileDir
+    stackVersionsFileHandler.VER_FILE = oldVerFile
+
 if __name__ == "__main__":
   unittest.main(verbosity=2)

ambari-agent/src/test/python/TestUpgradeExecutor.py

@@ -67,6 +67,30 @@ class TestUpgradeExecutor(TestCase):
     result = executor.perform_stack_upgrade(command, 'tmpout', 'tmperr')
     self.assertTrue('not supported' in result['stderr'])
     self.assertFalse(write_stack_version_method.called)
+    # Checking wrong source version
+    write_stack_version_method.reset()
+    command = {
+      'commandParams' :	{
+        'source_stack_version' : '{\"stackName\":\"HDP\",\"stackVersion\":\"Wrong\"}',
+        'target_stack_version' : '{\"stackName\":\"HDP\",\"stackVersion\":\"1.3.0\"}',
+        },
+      'role' : 'HDFS'
+    }
+    result = executor.perform_stack_upgrade(command, 'tmpout', 'tmperr')
+    self.assertTrue('does not match pattern' in result['stderr'])
+    self.assertFalse(write_stack_version_method.called)
+    # Checking wrong target version
+    write_stack_version_method.reset()
+    command = {
+      'commandParams' :	{
+        'source_stack_version' : '{\"stackName\":\"HDP\",\"stackVersion\":\"1.3.0\"}',
+        'target_stack_version' : '{\"stackName\":\"HDP\",\"stackVersion\":\"Wrong\"}',
+        },
+      'role' : 'HDFS'
+    }
+    result = executor.perform_stack_upgrade(command, 'tmpout', 'tmperr')
+    self.assertTrue('does not match pattern' in result['stderr'])
+    self.assertFalse(write_stack_version_method.called)
     # Checking successful result
     write_stack_version_method.reset()
     command = {
@@ -138,6 +162,8 @@ class TestUpgradeExecutor(TestCase):
     self.assertEquals(result, ('HDP', '1', '3'))
     result = executor.split_stack_version('{\"stackName\":\"ComplexStackVersion\",\"stackVersion\":\"1.3.4.2.2\"}')
     self.assertEquals(result, ('ComplexStackVersion', '1', '3'))
+    result = executor.split_stack_version('{\"stackName\":\"HDP\",\"stackVersion\":\"1\"}')
+    self.assertEquals(result, None)
     pass
 
 
@@ -187,6 +213,23 @@ class TestUpgradeExecutor(TestCase):
     self.assertEquals(result['stderr'],"\nstderr - first.py\nstderr - second.pp\nstderr - third.py\nNone\nstderr - fifth-failing.py")
 
 
+  @patch('os.path.isdir')
+  def test_execute_dir_not_existing(self, isdir_method):
+    pythonExecutor = MagicMock()
+    puppetExecutor = MagicMock()
+
+    command = {'debug': 'command'}
+    isdir_method.return_value = False
+
+    executor = UpgradeExecutor.UpgradeExecutor(pythonExecutor,
+        puppetExecutor, AmbariConfig.AmbariConfig().getConfig())
+
+    result= executor.execute_dir(command, 'basedir', 'not_existing_dir', 'tmpout', 'tmperr')
+    self.assertEquals(result['exitcode'],0)
+    self.assertEquals(result['stdout'],'Script directory basedir/not_existing_dir does not exist, skipping')
+    self.assertEquals(result['stderr'],'None')
+
+
   @patch('os.listdir')
   @patch('os.path.isdir')
   def test_execute_dir_ignore_badly_named(self, isdir_method, listdir_method):