RE: [PATCH] Fix hardcoded config folders

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Tuesday, January 11, 2022 5:00 PM
> To: dts@dpdk.org
> Cc: lijuan.tu@intel.com; Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH] Fix hardcoded config folders
> 
> From: Owen Hilyard <ohilyard@iol.unh.edu>
> 
> All hardcoded instances of "DTS_ROOT/conf" or "./conf" have been replaced
> with CONFIG_ROOT_PATH, which is either the environment variable
> DTS_CFG_FOLDER if it is set, or is DTS_ROOT/conf/ by default.
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---
>  framework/checkCase.py                              | 10 ++++------
>  framework/project_dpdk.py                           |  4 ++--
>  tests/TestSuite_iavf_package_driver_error_handle.py |  3 ++-
>  tests/TestSuite_ipsec_gw_cryptodev_func.py          |  4 +++-
>  tests/TestSuite_vhost_user_live_migration.py        |  5 +++--
>  tests/vhost_peer_conf.py                            |  4 +++-
>  tools/setup.py                                      | 10 ++++++----
>  7 files changed, 23 insertions(+), 17 deletions(-)
> 

I had a quick glance at this and it seems to me that we should change the FOLDERS settings variable as well. Maybe in another patch, since it contains more than just the conf path?

Should DTS_CONFIG_PATHS in ci/get_tests_for_patchset.py also use CONFIG_ROOT_PATH?

Do we want to address comments as well? E.g. the docstring of load_global_config in dts/virt_base.py.

The same question goes for docs, there are a few instances of conf/<cfgfile>.cfg.

> diff --git a/framework/checkCase.py b/framework/checkCase.py index
> 3182dc77..013b9551 100644
> --- a/framework/checkCase.py
> +++ b/framework/checkCase.py
> @@ -1,14 +1,12 @@
>  import collections
>  import json
> +import os
> 
> -import xlrd
> -
> -from .settings import HOST_DRIVER_SETTING, get_nic_name,
> load_global_setting
> +from .settings import HOST_DRIVER_SETTING, get_nic_name,
> +load_global_setting, CONFIG_ROOT_PATH
>  from .utils import RED
> 
> -filter_json_file = './conf/test_case_checklist.json'
> -support_json_file = './conf/test_case_supportlist.json'
> -
> +filter_json_file = os.path.join(CONFIG_ROOT_PATH,
> +'test_case_checklist.json') support_json_file =
> +os.path.join(CONFIG_ROOT_PATH, 'test_case_supportlist.json')
> 
>  class CheckCase(object):
>      """
> diff --git a/framework/project_dpdk.py b/framework/project_dpdk.py index
> 9927bcc1..05462b6d 100644
> --- a/framework/project_dpdk.py
> +++ b/framework/project_dpdk.py
> @@ -46,7 +46,7 @@ from .settings import (
>      NICS,
>      accepted_nic,
>      load_global_setting,
> -    save_global_setting,
> +    save_global_setting, CONFIG_ROOT_PATH,
>  )
>  from .ssh_connection import SSHConnection  from .tester import Tester @@ -
> 571,7 +571,7 @@ class DPDKdut(Dut):
>          folder_info = folder.split('/')
>          name = folder_info[-1]
>          if name != 'examples' and name not in self.apps_name:
> -            raise Exception('Please config %s file path on conf/app_name.cfg' %
> name)
> +            raise Exception(f'Please config {name} file path on
> + {os.path.join(CONFIG_ROOT_PATH, "app_name.cfg")}')
> 
>          if name == 'examples':
>              example = 'all'
> diff --git a/tests/TestSuite_iavf_package_driver_error_handle.py
> b/tests/TestSuite_iavf_package_driver_error_handle.py
> index 0cdba385..c4277fc7 100644
> --- a/tests/TestSuite_iavf_package_driver_error_handle.py
> +++ b/tests/TestSuite_iavf_package_driver_error_handle.py
> @@ -35,6 +35,7 @@ import time
> 
>  from framework.config import UserConf
>  from framework.pmd_output import PmdOutput
> +from framework.settings import CONFIG_ROOT_PATH
>  from framework.test_case import TestCase
> 
> 
> @@ -47,7 +48,7 @@ class Testiavf_package_and_driver_check(TestCase):
>          self.verify(len(self.dut_ports) >= 1, "Insufficient ports")
>          self.PF_QUEUE = 16
> 
> -        conf_file = 'conf/iavf_driver_package.cfg'
> +        conf_file = os.path.join(CONFIG_ROOT_PATH,
> + 'iavf_driver_package.cfg')
>          conf_peer = UserConf(conf_file)
>          conf_session = conf_peer.conf._sections['suite']
>          self.driverPath_latest = conf_session['ice_driver_file_location_latest']
> diff --git a/tests/TestSuite_ipsec_gw_cryptodev_func.py
> b/tests/TestSuite_ipsec_gw_cryptodev_func.py
> index a5d6e775..57ca79d9 100644
> --- a/tests/TestSuite_ipsec_gw_cryptodev_func.py
> +++ b/tests/TestSuite_ipsec_gw_cryptodev_func.py
> @@ -30,11 +30,13 @@
>  # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> 
>  import binascii
> +import os.path
>  import time
> 
>  import framework.packet as packet
>  import framework.utils as utils
>  import tests.cryptodev_common as cc
> +from framework.settings import CONFIG_ROOT_PATH
>  from framework.test_case import TestCase
> 
> 
> @@ -87,7 +89,7 @@ class TestIPsecGW(TestCase):
>              "u": "0x1"
>          }
> 
> -        conf_file = r'conf/ipsec_ep0.cfg'
> +        conf_file = os.path.join(CONFIG_ROOT_PATH, 'ipsec_ep0.cfg')
>          self.dut.session.copy_file_to(conf_file, '/tmp')
> 
>      def set_up(self):
> diff --git a/tests/TestSuite_vhost_user_live_migration.py
> b/tests/TestSuite_vhost_user_live_migration.py
> index e50800d5..8796f16a 100644
> --- a/tests/TestSuite_vhost_user_live_migration.py
> +++ b/tests/TestSuite_vhost_user_live_migration.py
> @@ -28,13 +28,14 @@
>  # THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> USE  # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> -
> +import os.path
>  import re
>  import time
> 
>  import framework.utils as utils
>  from framework.config import UserConf
>  from framework.exception import VirtDutInitException
> +from framework.settings import CONFIG_ROOT_PATH
>  from framework.test_case import TestCase  from framework.virt_common
> import VM
> 
> @@ -54,7 +55,7 @@ class TestVhostUserLiveMigration(TestCase):
>                          "Insufficient ports for testing")
> 
>          # get mount info from cfg file
> -        conf_info = UserConf('conf/%s.cfg' % self.suite_name)
> +        conf_info = UserConf(os.path.join(CONFIG_ROOT_PATH,
> + f'{self.suite_name}.cfg'))
>          conf_session = conf_info.conf._sections['mount_info']
>          self.mount_path = conf_session['backup_mount_path']
>          self.share_path = conf_session['host_share_dir'] diff --git
> a/tests/vhost_peer_conf.py b/tests/vhost_peer_conf.py index
> c8802277..4dc47341 100644
> --- a/tests/vhost_peer_conf.py
> +++ b/tests/vhost_peer_conf.py
> @@ -28,10 +28,12 @@
>  # THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> USE  # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> +import os
> 
>  from framework.config import UserConf
> +from framework.settings import CONFIG_ROOT_PATH
> 
> -conf_file = 'conf/vhost_peer_conf.cfg'
> +conf_file = os.path.join(CONFIG_ROOT_PATH, 'vhost_peer_conf.cfg')
>  conf_peer = UserConf(conf_file)
>  conf_session = conf_peer.conf._sections['peerconf']
> 
> diff --git a/tools/setup.py b/tools/setup.py index c70730c7..f4cc5d74 100755
> --- a/tools/setup.py
> +++ b/tools/setup.py
> @@ -20,10 +20,12 @@ DTS_TOOLS = DTS_PATH + '/tools'
>  DTS_SUITES = DTS_PATH + '/tests'
>  DTS_EXECS = DTS_PATH + '/executions'
> 
> -DTS_EXEC_CFG = DTS_PATH + '/execution.cfg'
> -DTS_CRBS_CFG = DTS_PATH + '/conf/crbs.cfg'
> -DTS_PORTS_CFG = DTS_PATH + '/conf/ports.cfg'
> -DTS_IXIA_CFG = DTS_PATH + '/conf/ixia.cfg'
> +CONFIG_ROOT_PATH = os.environ.get("DTS_CFG_FOLDER") or
> +os.path.join(DTS_PATH, "conf/")
> +
> +DTS_EXEC_CFG = os.path.join(DTS_PATH, 'execution.cfg') DTS_CRBS_CFG =
> +os.path.join(CONFIG_ROOT_PATH, 'crbs.cfg') DTS_PORTS_CFG =
> +os.path.join(CONFIG_ROOT_PATH, 'ports.cfg') DTS_IXIA_CFG =
> +os.path.join(CONFIG_ROOT_PATH, 'ixia.cfg')
> 
>  sys.path.append(DTS_FRAMEWORK)
>  sys.path.append(DTS_TOOLS)
> --
> 2.30.2
>