[master][SAI-PTF] Skip test when hit expected error from sai api (#1699)

meta/gensairpc.pl

@@ -90,7 +90,6 @@
     [ 'verbose|v',       'Print more details',                                                                                    { default => 0 }        ],
     [ 'mandatory-attrs', 'Make mandatory attributes obligatory in sai_adapter.py',                                                { default => 0 }        ],
     [ 'dev-utils:s',     'Generate additional development utils within the generated code. Additional options: [=log,zero]',      { default => 0 }        ],
-    [ 'skip_error:s',    'Skip test on specified error code. Additional options: [=-2]',                                          { default => 0 }        ],
     [ 'adapter_logger',  'Enable the logger in sai_adapter, it will log all the method invocation.',                              { default => 0}         ],
     [ 'attr-header',     'Generate additional header of attributes definitions (including object types)',                         { default => 0 }        ],
     [ 'help|h',          'Print this help',                                                                                       { shortcircuit => 1 }   ],
@@ -116,7 +115,6 @@
 my $clean         = $args->clean_meta;
 my $mandatory_attrs = $args->mandatory_attrs;
 my $dev_utils       = ( $args->dev_utils ne q{} ? $args->dev_utils : 1 );
-my $skip_error       = ( $args->skip_error ne q{} ? $args->skip_error : 1 );
 my $adapter_logger   = ( $args->adapter_logger ne q{} ? $args->adapter_logger : 1 );
 my $attr_header     = $args->attr_header;
 
@@ -163,7 +161,6 @@
     dbg             => $dbg,
     mandatory_attrs => $mandatory_attrs,
     dev_utils       => $dev_utils,
-    skip_error      => $skip_error,
     adapter_logger  => $adapter_logger,
     templates_dir   => $templates_dir
 };

meta/rpc/README.md

@@ -19,7 +19,6 @@ Optional arguments are useful for development purposes.
 | `-d` `--dump`          | Dump all data to the file. **Should be used during development**. |
 | `--mandatory-attrs`    | Make mandatory attributes obligatory in *sai\_adapter.py*. It removes `=None` from attributes, which are passed as arguments into python functions. Can be useful for debugging purposes, but since most of attributes are **optionally** mandatory, this is not as useful as it could be. |
 | `--dev-utils[=STR]`    | Generate additional development utils within the generated code. Additional options: [=log,zero]. Useful for tests development and debugging. The generated code **should not** be committed. |
-| `--skip_error[=STR]`   | Skip test on specified error code. Additional options: [=-2]. The generated code **should not** be committed. |
 | `--adapter_logger`     | Enable the logger in sai_adapter, it will log all the method invocation. |
 | `-h` `--help`          | Print the help. |
 

meta/templates/sai_adapter.py.tt

@@ -191,6 +191,12 @@ client.[% function.thrift_name %](
 [%- BLOCK catch_exception -%]
     except sai_thrift_exception as e:
         status = e.status
+        if SKIP_TEST_ON_EXPECTED_ERROR and status in EXPECTED_ERROR_CODE:
+            reason = "SkipTest on expected error. [% function.thrift_name %] with errorcode: {} error: {}".format(
+                status, e)
+            print(reason)
+            testutils.skipped_test_count=1
+            raise SkipTest(reason)
         if CATCH_EXCEPTIONS:
     [%- IF function.operation == 'stats' %]
             pass
@@ -409,6 +415,12 @@ client.[% function.thrift_name %](
 [%- BLOCK return_from_empty_function -%]
     global status
     status = SAI_STATUS_NOT_SUPPORTED
+    if SKIP_TEST_ON_EXPECTED_ERROR and status in EXPECTED_ERROR_CODE:
+        reason = "SkipTest on expected error. [% function.name %] with errorcode: {} error: {}".format(
+            status, e)
+        print(reason)
+        testutils.skipped_test_count=1
+        raise SkipTest(reason)
 
     if CATCH_EXCEPTIONS:
     [%- IF function.operation == 'create' AND NOT function.rpc_return.is_list %]
@@ -431,7 +443,6 @@ client.[% function.thrift_name %](
 
 
     [%- PROCESS decorate_method IF dev_utils -%]
-    [%- PROCESS decorate_skip_test_on_error IF skip_error -%]
     [%- PROCESS decorate_invocation_logger IF adapter_logger -%]
     [%- PROCESS function_header %]
     [%- PROCESS function_docstring %]
@@ -489,9 +500,11 @@ Thrift SAI interface basic tests
 # pylint: disable=too-many-return-statements,line-too-long,invalid-name
 
 [%- PROCESS dev_utils_imports IF dev_utils -%]
-[%- PROCESS skip_test_on_error_imports IF skip_error -%]
 [%- PROCESS invocation_logger_imports IF adapter_logger -%]
 
+from unittest import SkipTest
+from ptf import testutils
+
 from sai_thrift.ttypes import *
 from sai_thrift.sai_headers import *
 
@@ -503,10 +516,16 @@ from sai_thrift.sai_headers import *
 # In order to catch exceptions and get error codes
 # in the application, it should be disabled.
 CATCH_EXCEPTIONS = True
+# Expected error code
+# Used with SKIP_TEST_ON_EXPECTED_ERROR
+# For some expected errors in test
+# Like SAI_STATUS_NOT_SUPPORTED = -2
+EXPECTED_ERROR_CODE = [-2]
+# Skip test when hitting an expected error
+SKIP_TEST_ON_EXPECTED_ERROR = True
 status = 0
 
 [%- PROCESS dev_utils IF dev_utils -%]
-[%- PROCESS skip_error IF skip_error -%]
 [%- PROCESS invocation_logger IF adapter_logger -%]
 
 [%- FOREACH api IN apis.keys.sort -%]

meta/templates/sai_adapter_utils.tt

@@ -4,13 +4,7 @@
 from __future__ import print_function
 from collections import defaultdict
 from collections import Counter
-[% IF NOT skip_error %]from functools import wraps[% END %]
-[% END -%]
-
-[%- BLOCK skip_test_on_error_imports %]
 from functools import wraps
-from unittest import SkipTest
-from ptf import testutils
 [% END -%]
 
 [%- BLOCK invocation_logger_imports %]
@@ -29,13 +23,6 @@ import logging
 [% PROCESS instance_counter %]
 [% END -%]
 
-[%- BLOCK skip_error %]
-
-# skip_error
-
-[% PROCESS skip_test_on_error %]
-[% END -%]
-
 [%- BLOCK invocation_logger %]
 
 # invocation_logger
@@ -186,38 +173,6 @@ instance_counter.removed = defaultdict(Counter)
 instance_counter.not_removed = defaultdict(Counter)
 [%- END -%]
 
-[%- BLOCK skip_test_on_error -%]
-def skip_test_on_error(errorcode=[-2]):
-    def skip_test_on_error_decorator(func):
-        """
-        Decorator for skip the test when error happened.
-        Args:
-            errorcode: a list of the error code that test will be skipped.
-        """
-
-        @wraps(func)
-        def decorated(*args, **kwargs):
-            """
-            Check the return value and check if the status is the error code
-            on which the test should be skipped.
-
-            Args:
-                args(List): original args
-                kwargs(Dict): original kwargs
-            Returns:
-                retval(Any): the original return value
-            """
-            retval = func(*args, **kwargs)
-            global status             
-            if status in errorcode:
-                reason = "SkipTest: {} with errorcode: {}".format(func.__name__, status)
-                print(reason)
-                testutils.skipped_test_count=1
-                raise SkipTest(reason)
-            return retval
-        return decorated
-    return skip_test_on_error_decorator
-[%- END -%]
 
 [%- BLOCK invocation_logger_func -%]
 def invocation_logger(func):
@@ -232,10 +187,14 @@ def invocation_logger(func):
 
         args_dict = dict(zip(args_name, args))
         args_dict.update(kwargs)
+        args_dict = { key:str(value) for (key,value) in args_dict.items()}
         logging.info("sai_adapter_invoke func:[{}] args: [{}]".format(func.__name__, args_dict))
 
         args_values = args_dict.values()
-        return func(*args, **kwargs)
+
+        retval = func(*args, **kwargs)
+        logging.info("sai_adapter_return func:[{}] retval:[{}]".format(func.__name__, repr(retval)))
+        return retval
 
     return inner_logger
 [%- END -%]
@@ -248,9 +207,6 @@ def invocation_logger(func):
 @instance_counter("[% function.object %]", "[% function.operation %]"[% IF dev_utils.match('log') %], log=True[% END %][% IF dev_utils.match('zero') %], zero=True[% END %])
 [%- END -%]
 
-[%- BLOCK decorate_skip_test_on_error %]
-@skip_test_on_error([% IF skip_error %]errorcode=[[% skip_error %]][% END %])
-[%- END -%]
 
 [%- BLOCK decorate_invocation_logger %]
 @invocation_logger

ptf/utest/MockClient.py

@@ -1,6 +1,17 @@
+import logging
+try:
+    from meta.sai_adapter import *
+except ImportError:
+    from sai_thrift.sai_adapter import *
+
 class MockSuccessClient():
 
     def sai_thrift_remove_acl_table(self, var):
+        logging.info("sai_thrift_remove_acl_table invoked")
+        # e = sai_thrift_exception()
+        # e.status = -2
+        # raise e
+
         return 0
 
     def sai_thrift_create_switch(client,
@@ -88,4 +99,21 @@ def sai_thrift_create_switch(client,
                              qos_tc_and_color_to_mpls_exp_map=None,
                              failover_config_mode=None,
                              tunnel_objects_list=None):
+        logging.info("sai_thrift_create_switch invoked")
         return 0
+
+
+    def sai_thrift_get_acl_table_attribute(client, 
+                                            oid,
+                                            attr_list):
+        logging.info("sai_thrift_get_acl_table_attribute invoked")
+        attr_list = []
+        attribute1 = sai_thrift_attribute_t(id=SAI_ACL_TABLE_ATTR_ACL_STAGE)
+        attribute1.value = sai_thrift_attribute_value_t()
+        attribute = sai_thrift_attribute_t(id=SAI_ACL_TABLE_ATTR_ACL_STAGE, value=attribute1.value)
+        attr_list.append(attribute)
+
+
+        attr_lists = sai_thrift_attribute_list_t(attr_list=attr_list)
+        attr_lists.attr_list = attr_list
+        return attr_lists

ptf/utest/TemplateTest.py

@@ -11,7 +11,7 @@
 """
 Class use to test the template.
 Run command under folder 'meta'
-    perl -Irpc gensairpc.pl --adapter_logger --skip_error=-2
+    perl -Irpc gensairpc.pl --adapter_logger
     copy sai_adaptor to ./test/saithriftv2/gen-py/sai
     cd ./test/saithriftv2
     sudo python3 setup.py install
@@ -42,6 +42,8 @@ def test_logger(self):
                                  init_switch=True, 
                                  hardware_access_bus="11:11:11:11:11:11")
         self.check_file_contains(LOG_FILE_PATH, 'hardware_access_bus')
+        sai_thrift_get_acl_table_attribute(self.client, acl_table_oid=1, acl_stage=1)
+        self.check_file_contains(LOG_FILE_PATH, 'SAI_ACL_TABLE_ATTR_ACL_STAGE')
 
 
     def check_file_contains(self, file, content):