From b44ef1fe38da2b1b73f5b5a514d90068b614e0a6 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Wed, 11 Apr 2018 16:42:59 +0100 Subject: [PATCH 1/5] env.py: TestEnv: rename some attributes __ prefix triggers a special behavior in the parser and is replaced by ___ which was not intended and would have made external access to that member harder. __module and __connection_settings attributes were not required as it was only used from one method, so downgrade it to a variable. --- libs/utils/env.py | 82 +++++++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/libs/utils/env.py b/libs/utils/env.py index 438bebe62..8ed6f6dd2 100644 --- a/libs/utils/env.py +++ b/libs/utils/env.py @@ -198,9 +198,7 @@ class TestEnv(ShareState): self.target = None self.ftrace = None self.workdir = None - self.__installed_tools = set() - self.__modules = [] - self.__connection_settings = None + self._installed_tools = set() self._calib = None # Keep track of target IP and MAC address @@ -263,7 +261,7 @@ class TestEnv(ShareState): # Initialize binary tools to deploy test_conf_tools = self.test_conf.get('tools', []) target_conf_tools = self.conf.get('tools', []) - self.__tools = list(set(test_conf_tools + target_conf_tools)) + self._tools = list(set(test_conf_tools + target_conf_tools)) # Initialize ftrace events # test configuration override target one @@ -277,7 +275,7 @@ class TestEnv(ShareState): ) self.conf['ftrace'] = ftrace if ftrace['events']: - self.__tools.append('trace-cmd') + self._tools.append('trace-cmd') # Initialize features if '__features__' not in self.conf: @@ -392,25 +390,25 @@ class TestEnv(ShareState): if not force and self.target is not None: return self.target - self.__connection_settings = {} + connection_settings = {} # Configure username if 'username' in self.conf: - self.__connection_settings['username'] = self.conf['username'] + connection_settings['username'] = self.conf['username'] else: - self.__connection_settings['username'] = USERNAME_DEFAULT + connection_settings['username'] = USERNAME_DEFAULT # Configure password or SSH keyfile if 'keyfile' in self.conf: - self.__connection_settings['keyfile'] = self.conf['keyfile'] + connection_settings['keyfile'] = self.conf['keyfile'] elif 'password' in self.conf: - self.__connection_settings['password'] = self.conf['password'] + connection_settings['password'] = self.conf['password'] else: - self.__connection_settings['password'] = PASSWORD_DEFAULT + connection_settings['password'] = PASSWORD_DEFAULT # Configure port if 'port' in self.conf: - self.__connection_settings['port'] = self.conf['port'] + connection_settings['port'] = self.conf['port'] # Configure the host IP/MAC address if 'host' in self.conf: @@ -419,7 +417,7 @@ class TestEnv(ShareState): (self.mac, self.ip) = self.resolv_host(self.conf['host']) else: self.ip = self.conf['host'] - self.__connection_settings['host'] = self.ip + connection_settings['host'] = self.ip except KeyError: raise ValueError('Config error: missing [host] parameter') @@ -458,7 +456,7 @@ class TestEnv(ShareState): # Setup board default if not specified by configuration self.nrg_model = None platform = None - self.__modules = ['cpufreq', 'cpuidle'] + modules = ['cpufreq', 'cpuidle'] if 'board' not in self.conf: self.conf['board'] = 'UNKNOWN' @@ -467,12 +465,12 @@ class TestEnv(ShareState): # Initialize TC2 board if board_name == 'TC2': platform = devlib.platform.arm.TC2() - self.__modules = ['bl', 'hwmon', 'cpufreq'] + modules = ['bl', 'hwmon', 'cpufreq'] # Initialize JUNO board elif board_name in ('JUNO', 'JUNO2'): platform = devlib.platform.arm.Juno() - self.__modules = ['bl', 'hwmon', 'cpufreq'] + modules = ['bl', 'hwmon', 'cpufreq'] if board_name == 'JUNO': self.nrg_model = juno_r0_energy @@ -480,28 +478,28 @@ class TestEnv(ShareState): # Initialize OAK board elif board_name == 'OAK': platform = Platform(model='MT8173') - self.__modules = ['bl', 'cpufreq'] + modules = ['bl', 'cpufreq'] # Initialized HiKey board elif board_name == 'HIKEY': self.nrg_model = hikey_energy - self.__modules = [ "cpufreq", "cpuidle" ] + modules = [ "cpufreq", "cpuidle" ] platform = Platform(model='hikey') # Initialize HiKey960 board elif board_name == 'HIKEY960': - self.__modules = ['bl', 'cpufreq', 'cpuidle'] + modules = ['bl', 'cpufreq', 'cpuidle'] platform = Platform(model='hikey960') # Initialize Pixel phone elif board_name == 'PIXEL': self.nrg_model = pixel_energy - self.__modules = ['bl', 'cpufreq'] + modules = ['bl', 'cpufreq'] platform = Platform(model='pixel') # Initialize gem5 platform elif board_name == 'GEM5': - self.__modules=['cpufreq'] + modules=['cpufreq'] platform = self._init_target_gem5() elif board_name != 'UNKNOWN': @@ -516,13 +514,13 @@ class TestEnv(ShareState): big_core=board.get('big_core', None) ) if 'modules' in board: - self.__modules = board['modules'] + modules = board['modules'] ######################################################################## # Modules configuration ######################################################################## - modules = set(self.__modules) + modules = set(modules) # Refine modules list based on target.conf modules.update(self.conf.get('modules', [])) @@ -533,8 +531,8 @@ class TestEnv(ShareState): self.test_conf.get('exclude_modules', [])) modules.difference_update(remove_modules) - self.__modules = list(modules) - self._log.info('Devlib modules to load: %s', self.__modules) + modules = list(modules) + self._log.info('Devlib modules to load: %s', modules) ######################################################################## # Devlib target setup (based on target.config::platform) @@ -542,7 +540,7 @@ class TestEnv(ShareState): # If the target is Android, we need just (eventually) the device if platform_type.lower() == 'android': - self.__connection_settings = None + connection_settings = None device = 'DEFAULT' # Workaround for ARM-software/devlib#225 @@ -551,43 +549,43 @@ class TestEnv(ShareState): if 'device' in self.conf: device = self.conf['device'] - self.__connection_settings = {'device' : device} + connection_settings = {'device' : device} elif 'host' in self.conf: host = self.conf['host'] port = '5555' if 'port' in self.conf: port = str(self.conf['port']) device = '{}:{}'.format(host, port) - self.__connection_settings = {'device' : device} + connection_settings = {'device' : device} self._log.info('Connecting Android target [%s]', device) else: self._log.info('Connecting %s target:', platform_type) - for key in self.__connection_settings: + for key in connection_settings: self._log.info('%10s : %s', key, - self.__connection_settings[key]) + connection_settings[key]) self._log.info('Connection settings:') - self._log.info(' %s', self.__connection_settings) + self._log.info(' %s', connection_settings) if platform_type.lower() == 'linux': self._log.debug('Setup LINUX target...') - if "host" not in self.__connection_settings: + if "host" not in connection_settings: raise ValueError('Missing "host" param in Linux target conf') self.target = devlib.LinuxTarget( platform = platform, - connection_settings = self.__connection_settings, + connection_settings = connection_settings, working_directory = self.workdir, load_default_modules = False, - modules = self.__modules) + modules = modules) elif platform_type.lower() == 'android': self._log.debug('Setup ANDROID target...') self.target = devlib.AndroidTarget( platform = platform, - connection_settings = self.__connection_settings, + connection_settings = connection_settings, working_directory = self.workdir, load_default_modules = False, - modules = self.__modules) + modules = modules) elif platform_type.lower() == 'host': self._log.debug('Setup HOST target...') self.target = devlib.LocalLinuxTarget( @@ -595,7 +593,7 @@ class TestEnv(ShareState): working_directory = '/tmp/devlib-target', executables_directory = '/tmp/devlib-target/bin', load_default_modules = False, - modules = self.__modules, + modules = modules, connection_settings = {'unrooted': True}) else: raise ValueError('Config error: not supported [platform] type {}'\ @@ -611,10 +609,10 @@ class TestEnv(ShareState): self._log.info(' %s', self.target.working_directory) self.target.setup() - self.install_tools(self.__tools) + self.install_tools(self._tools) # Verify that all the required modules have been initialized - for module in self.__modules: + for module in modules: self._log.debug('Check for module [%s]...', module) if not hasattr(self.target, module): self._log.warning('Unable to initialize [%s] module', module) @@ -695,7 +693,7 @@ class TestEnv(ShareState): tools.update(['taskset', 'trace-cmd', 'perf', 'cgroup_run_into.sh']) # Remove duplicates and already-instaled tools - tools.difference_update(self.__installed_tools) + tools.difference_update(self._installed_tools) tools_to_install = [] for tool in tools: @@ -708,7 +706,7 @@ class TestEnv(ShareState): for tool_to_install in tools_to_install: self.target.install(tool_to_install) - self.__installed_tools.update(tools) + self._installed_tools.update(tools) def ftrace_conf(self, conf): self._init_ftrace(True, conf) @@ -887,7 +885,7 @@ class TestEnv(ShareState): if not force and self._calib: return self._calib - required = force or 'rt-app' in self.__installed_tools + required = force or 'rt-app' in self._installed_tools if not required: self._log.debug('No RT-App workloads, skipping calibration') -- GitLab From 1204ac26a6668ee874caa355aa5fcbfaca1fd644 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Wed, 11 Apr 2018 16:37:40 +0100 Subject: [PATCH 2/5] env.py: TestEnv: make force=True the default behavior Sanitize the code by using a non-surprising default. force=False is an optimization that should be used carefully when needed, not the default behavior. TestEnv.calibration() is an exception as it is already wildly used outside TestEnv, so breaking compatibilty is an issue. --- ipynb/energy/EnergyModel_ClusterEnergy.ipynb | 2 +- ipynb/energy/EnergyModel_SystemEnergy.ipynb | 2 +- ipynb/examples/devlib/cgroups_example.ipynb | 2 +- ipynb/examples/energy_meter/EnergyMeter_ACME.ipynb | 2 +- ipynb/examples/energy_meter/EnergyMeter_AEP.ipynb | 2 +- .../examples/energy_meter/EnergyMeter_HWMON.ipynb | 2 +- .../energy_meter/EnergyMeter_Monsoon.ipynb | 2 +- .../TraceAnalysis_FunctionsProfiling.ipynb | 2 +- .../trace_analysis/TraceAnalysis_IdleStates.ipynb | 2 +- .../TraceAnalysis_TasksLatencies.ipynb | 2 +- ipynb/releases/ReleaseNotes_v16.10.ipynb | 8 ++++---- ipynb/releases/ReleaseNotes_v16.12.ipynb | 2 +- libs/utils/env.py | 14 +++++++------- 13 files changed, 22 insertions(+), 22 deletions(-) diff --git a/ipynb/energy/EnergyModel_ClusterEnergy.ipynb b/ipynb/energy/EnergyModel_ClusterEnergy.ipynb index bece7fe95..ec05faa9a 100644 --- a/ipynb/energy/EnergyModel_ClusterEnergy.ipynb +++ b/ipynb/energy/EnergyModel_ClusterEnergy.ipynb @@ -178,7 +178,7 @@ "source": [ "# Initialize a test environment using:\n", "# the provided target configuration (my_conf)\n", - "te = TestEnv(target_conf=my_conf, force_new=True)\n", + "te = TestEnv(target_conf=my_conf)\n", "target = te.target" ] }, diff --git a/ipynb/energy/EnergyModel_SystemEnergy.ipynb b/ipynb/energy/EnergyModel_SystemEnergy.ipynb index c813f097b..65b76e92d 100644 --- a/ipynb/energy/EnergyModel_SystemEnergy.ipynb +++ b/ipynb/energy/EnergyModel_SystemEnergy.ipynb @@ -207,7 +207,7 @@ "source": [ "# Initialize a test environment using:\n", "# the provided target configuration (my_conf)\n", - "te = TestEnv(target_conf=my_conf, force_new=True)\n", + "te = TestEnv(target_conf=my_conf)\n", "target = te.target" ] }, diff --git a/ipynb/examples/devlib/cgroups_example.ipynb b/ipynb/examples/devlib/cgroups_example.ipynb index 1e1dbb43f..fd2f5ecb9 100644 --- a/ipynb/examples/devlib/cgroups_example.ipynb +++ b/ipynb/examples/devlib/cgroups_example.ipynb @@ -202,7 +202,7 @@ " }\n", "}\n", "\n", - "te = TestEnv(my_conf, force_new=True)\n", + "te = TestEnv(my_conf)\n", "target = te.target\n", "\n", "# Report target connection\n", diff --git a/ipynb/examples/energy_meter/EnergyMeter_ACME.ipynb b/ipynb/examples/energy_meter/EnergyMeter_ACME.ipynb index 01291a2b6..ea4f9d123 100644 --- a/ipynb/examples/energy_meter/EnergyMeter_ACME.ipynb +++ b/ipynb/examples/energy_meter/EnergyMeter_ACME.ipynb @@ -159,7 +159,7 @@ ], "source": [ "# Initialize a test environment using:\n", - "te = TestEnv(my_conf, wipe=False, force_new=True)\n", + "te = TestEnv(my_conf, wipe=False)\n", "target = te.target" ] }, diff --git a/ipynb/examples/energy_meter/EnergyMeter_AEP.ipynb b/ipynb/examples/energy_meter/EnergyMeter_AEP.ipynb index f71839e56..b3fb909e0 100644 --- a/ipynb/examples/energy_meter/EnergyMeter_AEP.ipynb +++ b/ipynb/examples/energy_meter/EnergyMeter_AEP.ipynb @@ -158,7 +158,7 @@ ], "source": [ "# Initialize a test environment using:\n", - "te = TestEnv(my_conf, wipe=False, force_new=True)\n", + "te = TestEnv(my_conf, wipe=False)\n", "target = te.target" ] }, diff --git a/ipynb/examples/energy_meter/EnergyMeter_HWMON.ipynb b/ipynb/examples/energy_meter/EnergyMeter_HWMON.ipynb index 2357662fb..c99048631 100644 --- a/ipynb/examples/energy_meter/EnergyMeter_HWMON.ipynb +++ b/ipynb/examples/energy_meter/EnergyMeter_HWMON.ipynb @@ -158,7 +158,7 @@ ], "source": [ "# Initialize a test environment using:\n", - "te = TestEnv(my_conf, wipe=False, force_new=True)\n", + "te = TestEnv(my_conf, wipe=False)\n", "target = te.target" ] }, diff --git a/ipynb/examples/energy_meter/EnergyMeter_Monsoon.ipynb b/ipynb/examples/energy_meter/EnergyMeter_Monsoon.ipynb index db7568323..b265f9fb3 100644 --- a/ipynb/examples/energy_meter/EnergyMeter_Monsoon.ipynb +++ b/ipynb/examples/energy_meter/EnergyMeter_Monsoon.ipynb @@ -191,7 +191,7 @@ ], "source": [ "# Initialize a test environment using:\n", - "te = TestEnv(my_conf, wipe=False, force_new=True)\n", + "te = TestEnv(my_conf, wipe=False)\n", "target = te.target" ] }, diff --git a/ipynb/examples/trace_analysis/TraceAnalysis_FunctionsProfiling.ipynb b/ipynb/examples/trace_analysis/TraceAnalysis_FunctionsProfiling.ipynb index ba6a0721e..179dbcc14 100644 --- a/ipynb/examples/trace_analysis/TraceAnalysis_FunctionsProfiling.ipynb +++ b/ipynb/examples/trace_analysis/TraceAnalysis_FunctionsProfiling.ipynb @@ -169,7 +169,7 @@ ], "source": [ "# Initialize a test environment using:\n", - "te = TestEnv(my_conf, wipe=False, force_new=True)\n", + "te = TestEnv(my_conf, wipe=False)\n", "target = te.target" ] }, diff --git a/ipynb/examples/trace_analysis/TraceAnalysis_IdleStates.ipynb b/ipynb/examples/trace_analysis/TraceAnalysis_IdleStates.ipynb index 3659e6f76..6130c215b 100644 --- a/ipynb/examples/trace_analysis/TraceAnalysis_IdleStates.ipynb +++ b/ipynb/examples/trace_analysis/TraceAnalysis_IdleStates.ipynb @@ -191,7 +191,7 @@ ], "source": [ "# Initialize a test environment\n", - "te = TestEnv(my_conf, wipe=False, force_new=True)\n", + "te = TestEnv(my_conf, wipe=False)\n", "target = te.target" ] }, diff --git a/ipynb/examples/trace_analysis/TraceAnalysis_TasksLatencies.ipynb b/ipynb/examples/trace_analysis/TraceAnalysis_TasksLatencies.ipynb index 27023db7e..d2987c9e3 100644 --- a/ipynb/examples/trace_analysis/TraceAnalysis_TasksLatencies.ipynb +++ b/ipynb/examples/trace_analysis/TraceAnalysis_TasksLatencies.ipynb @@ -197,7 +197,7 @@ ], "source": [ "# Initialize a test environment using:\n", - "te = TestEnv(my_conf, wipe=False, force_new=True)\n", + "te = TestEnv(my_conf, wipe=False)\n", "target = te.target" ] }, diff --git a/ipynb/releases/ReleaseNotes_v16.10.ipynb b/ipynb/releases/ReleaseNotes_v16.10.ipynb index 3e3ece946..9f0e2a003 100644 --- a/ipynb/releases/ReleaseNotes_v16.10.ipynb +++ b/ipynb/releases/ReleaseNotes_v16.10.ipynb @@ -255,7 +255,7 @@ "source": [ "from env import TestEnv\n", "\n", - "te = TestEnv(my_conf, force_new=True)\n", + "te = TestEnv(my_conf)\n", "target = te.target" ] }, @@ -532,7 +532,7 @@ "source": [ "from env import TestEnv\n", "\n", - "te = TestEnv(my_conf, force_new=True)" + "te = TestEnv(my_conf)" ] }, { @@ -766,7 +766,7 @@ "source": [ "from env import TestEnv\n", "\n", - "te = TestEnv(my_conf, force_new=True)\n", + "te = TestEnv(my_conf)\n", "target = te.target" ] }, @@ -1662,7 +1662,7 @@ "source": [ "from env import TestEnv\n", "\n", - "te = TestEnv(my_conf, force_new=True)\n", + "te = TestEnv(my_conf)\n", "target = te.target" ] }, diff --git a/ipynb/releases/ReleaseNotes_v16.12.ipynb b/ipynb/releases/ReleaseNotes_v16.12.ipynb index b51b30085..791644806 100644 --- a/ipynb/releases/ReleaseNotes_v16.12.ipynb +++ b/ipynb/releases/ReleaseNotes_v16.12.ipynb @@ -493,7 +493,7 @@ " 'platform' : 'android',\n", " 'board' : 'pixel',\n", " 'ANDROID_HOME' : '/home/patbel01/Code/lisa/tools/android-sdk-linux/'\n", - " }, force_new=True)\n", + " })\n", "target = te.target" ] }, diff --git a/libs/utils/env.py b/libs/utils/env.py index 8ed6f6dd2..c8d58b28b 100644 --- a/libs/utils/env.py +++ b/libs/utils/env.py @@ -187,7 +187,7 @@ class TestEnv(ShareState): _initialized = False def __init__(self, target_conf=None, test_conf=None, wipe=True, - force_new=False): + force_new=True): super(TestEnv, self).__init__() if self._initialized and not force_new: @@ -313,13 +313,13 @@ class TestEnv(ShareState): self._init() # Initialize FTrace events collection - self._init_ftrace(True) + self._init_ftrace() # Initialize RT-App calibration values self.calibration() # Initialize energy probe instrument - self._init_energy(True) + self._init_energy() self._log.info('Set results folder to:') self._log.info(' %s', self.res_dir) @@ -349,7 +349,7 @@ class TestEnv(ShareState): conf.load() return conf.json - def _init(self, force = False): + def _init(self, force=True): # Initialize target self._init_target(force) @@ -385,7 +385,7 @@ class TestEnv(ShareState): self._init_platform() - def _init_target(self, force = False): + def _init_target(self, force=True): if not force and self.target is not None: return self.target @@ -711,7 +711,7 @@ class TestEnv(ShareState): def ftrace_conf(self, conf): self._init_ftrace(True, conf) - def _init_ftrace(self, force=False, conf=None): + def _init_ftrace(self, force=True, conf=None): if not force and self.ftrace is not None: return self.ftrace @@ -748,7 +748,7 @@ class TestEnv(ShareState): return self.ftrace - def _init_energy(self, force): + def _init_energy(self, force=True): # Initialize energy probe to board default self.emeter = EnergyMeter.getInstance(self.target, self.conf, force, -- GitLab From 1cf306155880e913862cf12ed01d9f37c9e45af6 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Thu, 15 Feb 2018 14:38:11 +0000 Subject: [PATCH 3/5] env.py: Remove implicit state in TestEnv Implicitly sharing the state of TestEnv breaks the execution of multiple tests relying on different TestEnv. It forces to launch them in separate lisa-test invocation. Remove this shared state in TestEnv to have a separate result directory per TestEnv instance. To ease TestEnv creation in notebook environments, TestEnv.create_once() class method is added to only create an instance the first time it is called. This avoids boilerplate code around TestEnv invocation. TestEnv.clear_once() allows reseting this instance, if ever needed. --- libs/utils/env.py | 48 +++++++++++++++++++++---------------- tests/lisa/test_executor.py | 3 +-- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/libs/utils/env.py b/libs/utils/env.py index c8d58b28b..181b3fbb9 100644 --- a/libs/utils/env.py +++ b/libs/utils/env.py @@ -49,13 +49,7 @@ LATEST_LINK = 'results_latest' basepath = os.path.dirname(os.path.realpath(__file__)) basepath = basepath.replace('/libs/utils', '') -class ShareState(object): - __shared_state = {} - - def __init__(self): - self.__dict__ = self.__shared_state - -class TestEnv(ShareState): +class TestEnv(object): """ Represents the environment configuring LISA, the target, and the test setup @@ -150,11 +144,6 @@ class TestEnv(ShareState): :param wipe: set true to cleanup all previous content from the output folder :type wipe: bool - - :param force_new: Create a new TestEnv object even if there is one available - for this session. By default, TestEnv only creates one - object per session, use this to override this behaviour. - :type force_new: bool """ critical_tasks = { @@ -184,14 +173,35 @@ class TestEnv(ShareState): freeze when using freeeze_userspace. """ - _initialized = False + __singleton = (None, None) - def __init__(self, target_conf=None, test_conf=None, wipe=True, - force_new=True): - super(TestEnv, self).__init__() + @classmethod + def create_once(cls, *args, **kwargs): + """ + Create a new TestEnv instance the first time it is called, and then + always returns it. - if self._initialized and not force_new: - return + :param args: Parameters passed to the constructor. + :param kwargs: Parameters passed to the constructor. + + """ + + params = (tuple(args), tuple(kwargs.items())) + if cls.__singleton is None: + cls.__singleton = params, cls(*args, **kwargs) + + # We only try to check if the parameters are the same. This check is + # not perfect but should catch some common mistakes. It will not catch + # a change to a JSON file's content for example. + orig_params, singleton = cls.__singleton + if orig_params != params: + raise ValueError( + 'Original instance created using different parameters') + + return singleton + + def __init__(self, target_conf=None, test_conf=None, wipe=True): + super(TestEnv, self).__init__() self.conf = {} self.test_conf = {} @@ -326,8 +336,6 @@ class TestEnv(ShareState): self._log.info('Experiment results available also in:') self._log.info(' %s', res_lnk) - self._initialized = True - def loadTargetConfig(self, filepath=None): """ Load the target configuration from the specified file. diff --git a/tests/lisa/test_executor.py b/tests/lisa/test_executor.py index 6502521e1..a424a5345 100644 --- a/tests/lisa/test_executor.py +++ b/tests/lisa/test_executor.py @@ -44,8 +44,7 @@ class SetUpTarget(TestCase): 'ftrace': { 'events': [] } - }, - force_new=True) + }) class TestMagicSmoke(SetUpTarget): def test_files_created(self): -- GitLab From fd583e77747592182ef19d251231e0b0623506a9 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Wed, 11 Apr 2018 18:41:30 +0100 Subject: [PATCH 4/5] env.py: TestEnv: Allow sharing target attribute When LISA_TARGET_SINGLETON env var is defined to 1, TestEnv will only build the target once and will then reuse the same target in all following TestEnv instances. This works around the fact that nosetest does not allow passing arbitrary data down to test cases. TestEnv instances have to horizontally cooperate instead to avoid this reinitialization. Move TestEnv._installed_tools, TestEnv._calib and TestEnv.nrg_model to a storage dict attached to the target itself. That makes sure these values will stick to their target while limiting to the absolute minimum the number of TestEnv attributes directly shared. Having it as a dictionary instead of inheriting from devlib.Target makes it easier to pinpoint such shared data. TestEnv.nrg_model is still exposed as a TestEnv attribute but is initialized using that target storage dict. --- libs/utils/env.py | 163 +++++++++++++++++++++++++++---------------- src/shell/lisa_shell | 3 + 2 files changed, 104 insertions(+), 62 deletions(-) diff --git a/libs/utils/env.py b/libs/utils/env.py index 181b3fbb9..c43be550e 100644 --- a/libs/utils/env.py +++ b/libs/utils/env.py @@ -53,6 +53,11 @@ class TestEnv(object): """ Represents the environment configuring LISA, the target, and the test setup + If LISA_TARGET_SINGLETON environment variable is defined to 1, only the + first TestEnv instance will create a target and the following ones will + reuse the same target object. That avoids reinitializing the target when + starting tests using lisa-test command. + The test environment is defined by: - a target configuration (target_conf) defining which HW platform we @@ -173,6 +178,9 @@ class TestEnv(object): freeze when using freeeze_userspace. """ + # target class attribute is used as a shared storage between instances + target = None + __singleton = (None, None) @classmethod @@ -205,11 +213,8 @@ class TestEnv(object): self.conf = {} self.test_conf = {} - self.target = None self.ftrace = None self.workdir = None - self._installed_tools = set() - self._calib = None # Keep track of target IP and MAC address self.ip = None @@ -326,7 +331,8 @@ class TestEnv(object): self._init_ftrace() # Initialize RT-App calibration values - self.calibration() + # If the target is already existing and calibrated, we skip it + self.calibration(force=False) # Initialize energy probe instrument self._init_energy() @@ -336,6 +342,22 @@ class TestEnv(object): self._log.info('Experiment results available also in:') self._log.info(' %s', res_lnk) + @property + def _target_state(self): + """ + Monkey patch devlib Target to store some LISA-specific data. + + That allows storing all target-specific state in the target object that + can then be shared across multiple TestEnv instances. _lisa_state + member is unlikely to be used by devlib code so this hack should be + harmless. + """ + try: + return self.target._lisa_state + except AttributeError: + self.target._lisa_state = dict() + return self.target._lisa_state + def loadTargetConfig(self, filepath=None): """ Load the target configuration from the specified file. @@ -394,10 +416,6 @@ class TestEnv(object): def _init_target(self, force=True): - - if not force and self.target is not None: - return self.target - connection_settings = {} # Configure username @@ -575,48 +593,57 @@ class TestEnv(object): self._log.info('Connection settings:') self._log.info(' %s', connection_settings) - if platform_type.lower() == 'linux': - self._log.debug('Setup LINUX target...') - if "host" not in connection_settings: - raise ValueError('Missing "host" param in Linux target conf') - - self.target = devlib.LinuxTarget( - platform = platform, - connection_settings = connection_settings, - working_directory = self.workdir, - load_default_modules = False, - modules = modules) - elif platform_type.lower() == 'android': - self._log.debug('Setup ANDROID target...') - self.target = devlib.AndroidTarget( - platform = platform, - connection_settings = connection_settings, - working_directory = self.workdir, - load_default_modules = False, - modules = modules) - elif platform_type.lower() == 'host': - self._log.debug('Setup HOST target...') - self.target = devlib.LocalLinuxTarget( - platform = platform, - working_directory = '/tmp/devlib-target', - executables_directory = '/tmp/devlib-target/bin', - load_default_modules = False, - modules = modules, - connection_settings = {'unrooted': True}) - else: - raise ValueError('Config error: not supported [platform] type {}'\ - .format(platform_type)) + # If a target does not already exist or if we want to recreate the + # target every time + if self.target is None or not int(os.getenv('LISA_TARGET_SINGLETON',0)): + if platform_type.lower() == 'linux': + self._log.debug('Setup LINUX target...') + if "host" not in connection_settings: + raise ValueError('Missing "host" param in Linux target conf') + + self.target = devlib.LinuxTarget( + platform = platform, + connection_settings = connection_settings, + working_directory = self.workdir, + load_default_modules = False, + modules = modules) + elif platform_type.lower() == 'android': + self._log.debug('Setup ANDROID target...') + self.target = devlib.AndroidTarget( + platform = platform, + connection_settings = connection_settings, + working_directory = self.workdir, + load_default_modules = False, + modules = modules) + elif platform_type.lower() == 'host': + self._log.debug('Setup HOST target...') + self.target = devlib.LocalLinuxTarget( + platform = platform, + working_directory = '/tmp/devlib-target', + executables_directory = '/tmp/devlib-target/bin', + load_default_modules = False, + modules = modules, + connection_settings = {'unrooted': True}) + else: + raise ValueError('Config error: not supported [platform] type {}'\ + .format(platform_type)) + + self._log.debug('Checking target connection...') + self._log.debug('Target info:') + self._log.debug(' ABI: %s', self.target.abi) + self._log.debug(' CPUs: %s', self.target.cpuinfo) + self._log.debug(' Clusters: %s', self.target.core_clusters) - self._log.debug('Checking target connection...') - self._log.debug('Target info:') - self._log.debug(' ABI: %s', self.target.abi) - self._log.debug(' CPUs: %s', self.target.cpuinfo) - self._log.debug(' Clusters: %s', self.target.core_clusters) + self._log.info('Initializing target workdir:') + self._log.info(' %s', self.target.working_directory) - self._log.info('Initializing target workdir:') - self._log.info(' %s', self.target.working_directory) + self.target.setup() + self._target_state['installed_tools'] = set() + + # Store the target in the class attribute so they will be available + # if target reinint is skipped + type(self).target = self.target - self.target.setup() self.install_tools(self._tools) # Verify that all the required modules have been initialized @@ -630,11 +657,20 @@ class TestEnv(object): 'update your kernel or test configurations'.format(module)) if not self.nrg_model: - try: - self._log.info('Attempting to read energy model from target') - self.nrg_model = EnergyModel.from_target(self.target) - except (TargetError, RuntimeError, ValueError) as e: - self._log.error("Couldn't read target energy model: %s", e) + # If the target already has an energy model attached, we just use + # that to avoid spending time re-fetching it. + target_em = self._target_state.get('nrg_model') + if target_em: + self._log.info('Using energy model already fetched from target') + self.nrg_model = target_em + else: + try: + self._log.info('Attempting to read energy model from target') + self.nrg_model = EnergyModel.from_target(self.target) + except (TargetError, RuntimeError, ValueError) as e: + self._log.error("Couldn't read target energy model: %s", e) + + self._target_state['nrg_model'] = self.nrg_model def _init_target_gem5(self): system = self.conf['gem5']['system'] @@ -701,7 +737,7 @@ class TestEnv(object): tools.update(['taskset', 'trace-cmd', 'perf', 'cgroup_run_into.sh']) # Remove duplicates and already-instaled tools - tools.difference_update(self._installed_tools) + tools.difference_update(self._target_state['installed_tools']) tools_to_install = [] for tool in tools: @@ -714,7 +750,7 @@ class TestEnv(object): for tool_to_install in tools_to_install: self.target.install(tool_to_install) - self._installed_tools.update(tools) + self._target_state['installed_tools'].update(tools) def ftrace_conf(self, conf): self._init_ftrace(True, conf) @@ -890,10 +926,11 @@ class TestEnv(object): force=False and we have not installed rt-app. """ - if not force and self._calib: - return self._calib + if not force and self._target_state.get('calib'): + return self._target_state['calib'] - required = force or 'rt-app' in self._installed_tools + + required = force or 'rt-app' in self._target_state['installed_tools'] if not required: self._log.debug('No RT-App workloads, skipping calibration') @@ -901,19 +938,21 @@ class TestEnv(object): if not force and 'rtapp-calib' in self.conf: self._log.info('Using configuration provided RTApp calibration') - self._calib = { + calib = { int(key): int(value) for key, value in self.conf['rtapp-calib'].items() } else: self._log.info('Calibrating RTApp...') - self._calib = RTA.calibrate(self.target) + calib = RTA.calibrate(self.target) self._log.info('Using RT-App calibration values:') self._log.info(' %s', - "{" + ", ".join('"%r": %r' % (key, self._calib[key]) - for key in sorted(self._calib)) + "}") - return self._calib + "{" + ", ".join('"%r": %r' % (key, calib[key]) + for key in sorted(calib)) + "}") + + self._target_state['calib'] = calib + return calib def resolv_host(self, host=None): """ diff --git a/src/shell/lisa_shell b/src/shell/lisa_shell index bc6a4f528..0cd5b12ed 100755 --- a/src/shell/lisa_shell +++ b/src/shell/lisa_shell @@ -393,6 +393,9 @@ fi # Execute in a sub-shell ( export LISA_TEST_ITERATIONS LISA_TARGET_CONF LISA_RESULTS_DIR + # Allow all TestEnv to share the same target to avoid reinitialization between + # tests + export LISA_TARGET_SINGLETON=1 _lisa-test "${extra_args[@]}" ) local retcode=$? -- GitLab From c0b7c9eba54f13f87bee5f44cce8ca8fa361d6a1 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Wed, 28 Feb 2018 18:58:07 +0000 Subject: [PATCH 5/5] env.py: Add per-lisa-test command results folder The results directory used by tests currently defaults to a timestamp-based folder name, created when the first TestEnv of the test session is instantiated. Since they share the state, subsequent test cases reuse the same folder. Cleanup that behavior by introducing true per-lisa-test invocation directory, and storing the tests results in folders named after the test case class or after a timestamp. The timestamp naming is mainly intended to be used from a notebook where a TestEnv is instantiated by hand. The added benefits are that results.xml xUnit file has a permanent home, and that the tests results folders are clearly named after the class that created them. --- libs/utils/env.py | 81 ++++++++++++++++++++++++------------- libs/utils/test.py | 11 ++++- src/shell/lisa_shell | 37 ++++++++--------- tests/eas/preliminary.py | 16 ++++---- tests/lisa/test_executor.py | 1 + 5 files changed, 90 insertions(+), 56 deletions(-) diff --git a/libs/utils/env.py b/libs/utils/env.py index c43be550e..b00b5a6d2 100644 --- a/libs/utils/env.py +++ b/libs/utils/env.py @@ -39,15 +39,18 @@ from platforms.juno_r0_energy import juno_r0_energy from platforms.hikey_energy import hikey_energy from platforms.pixel_energy import pixel_energy +# This should be an absolute path pointing to the root of LISA repository. +BASEPATH = os.path.abspath(os.path.join( + os.path.dirname(os.path.realpath(__file__)), '..', '..' +)) + USERNAME_DEFAULT = 'root' PASSWORD_DEFAULT = '' FTRACE_EVENTS_DEFAULT = ['sched:*'] FTRACE_BUFSIZE_DEFAULT = 10240 -OUT_PREFIX = 'results' +DEFAULT_RESULTS_BASE = os.path.abspath(os.path.join(BASEPATH, 'results')) LATEST_LINK = 'results_latest' -basepath = os.path.dirname(os.path.realpath(__file__)) -basepath = basepath.replace('/libs/utils', '') class TestEnv(object): """ @@ -64,9 +67,12 @@ class TestEnv(object): want to use to run the experiments - a test configuration (test_conf) defining which SW setups we need on that HW target - - a folder to collect the experiments results, which can be specified using - the target_conf::results_dir option, or using LISA_RESULTS_DIR environment - variable and is by default wiped from all the previous contents + - a folder to collect the experiments results. If a relative path is given + through the test_conf, it will be interpreted relatively to the results + base path. This base path can be set using the LISA_RESULTS_BASE + environment variable or via target_conf::results_base key. Relative + base paths are interpreted relatively to the default base path. + By default, the results directory is wiped from all the previous contents (if wipe=True) :param target_conf: @@ -109,7 +115,10 @@ class TestEnv(object): target. LISA does *not* manage this TFTP server, it must be provided externally. Optional. **results_dir** - location of results of the experiments. + location of results of the experiments. If the path is relative, + it will be created under the results base. If it is an absolute + path, it is used as-is. If no results_dir is provided, a + timestamp-based folder is created under the results base. **ftrace** Ftrace configuration merged with test-specific configuration. Currently, only additional events through "events" key is supported. @@ -240,7 +249,7 @@ class TestEnv(object): self._log = logging.getLogger('TestEnv') # Compute base installation path - self._log.info('Using base path: %s', basepath) + self._log.info('Using base path: %s', BASEPATH) # Setup target configuration if isinstance(target_conf, dict): @@ -296,22 +305,40 @@ class TestEnv(object): if '__features__' not in self.conf: self.conf['__features__'] = [] - # Initialize local results folder. - # The test configuration overrides the target's one and the environment - # variable overrides everything else. - self.res_dir = ( - os.getenv('LISA_RESULTS_DIR') or - self.conf.get('results_dir') + # Base directory under which results directories are stored. The + # default location can be overriden by defining LISA_RESULTS_BASE + # environment variable and from target config. + res_base = ( + os.getenv('LISA_RESULTS_BASE') or + self.conf.get('results_base') or + DEFAULT_RESULTS_BASE ) - # Default result dir based on the current time - if not self.res_dir: - self.res_dir = datetime.now().strftime( - os.path.join(basepath, OUT_PREFIX, '%Y%m%d_%H%M%S') + if not os.path.isabs(res_base): + res_base = os.path.join(DEFAULT_RESULTS_BASE, res_base) + + # Initialize local results folder. + res_dir = ( + # Setting it from the target config is sloppy but it avoids a + # compatibility break with previous behavior. That should work as + # long as only one TestEnv is created, for example in a notebook. + self.conf.get('results_dir') or + # This should be set to the tested class name so that the results + # folder can be linked easily to the class that created the + # TestEnv. + self.test_conf.get('results_dir') or + # If no results dir is given by the test configuration, we default + # to a timestamp-based name. This is mostly useful when a TestEnv + # is created from a notebook. Automated tests are expected to set + # results_dir to . + datetime.now().strftime('%Y%m%d_%H%M%S') ) - # Relative paths are interpreted as relative to a fixed root. - if not os.path.isabs(self.res_dir): - self.res_dir = os.path.join(basepath, OUT_PREFIX, self.res_dir) + # A TestEnv instantiated in a notebook may specify an absolute path + # which must be honored. + if os.path.isabs(res_dir): + self.res_dir = res_dir + else: + self.res_dir = os.path.join(res_base, res_dir) if wipe and os.path.exists(self.res_dir): self._log.warning('Wipe previous contents of the results folder:') @@ -320,7 +347,7 @@ class TestEnv(object): if not os.path.exists(self.res_dir): os.makedirs(self.res_dir) - res_lnk = os.path.join(basepath, LATEST_LINK) + res_lnk = os.path.join(BASEPATH, LATEST_LINK) if os.path.islink(res_lnk): os.remove(res_lnk) os.symlink(self.res_dir, res_lnk) @@ -372,7 +399,7 @@ class TestEnv(object): filepath = filepath or 'target.config' # Loading default target configuration - conf_file = os.path.join(basepath, filepath) + conf_file = os.path.join(BASEPATH, filepath) self._log.info('Loading target configuration [%s]...', conf_file) conf = JsonConf(conf_file) @@ -741,10 +768,10 @@ class TestEnv(object): tools_to_install = [] for tool in tools: - binary = '{}/tools/scripts/{}'.format(basepath, tool) + binary = '{}/tools/scripts/{}'.format(BASEPATH, tool) if not os.path.isfile(binary): binary = '{}/tools/{}/{}'\ - .format(basepath, self.target.abi, tool) + .format(BASEPATH, self.target.abi, tool) tools_to_install.append(binary) for tool_to_install in tools_to_install: @@ -837,7 +864,7 @@ class TestEnv(object): self.platform['cpus_count'] = len(self.target.core_clusters) def _load_em(self, board): - em_path = os.path.join(basepath, + em_path = os.path.join(BASEPATH, 'libs/utils/platforms', board.lower() + '.json') self._log.debug('Trying to load default EM from %s', em_path) if not os.path.exists(em_path): @@ -851,7 +878,7 @@ class TestEnv(object): return board.json['nrg_model'] def _load_board(self, board): - board_path = os.path.join(basepath, + board_path = os.path.join(BASEPATH, 'libs/utils/platforms', board.lower() + '.json') self._log.debug('Trying to load board descriptor from %s', board_path) if not os.path.exists(board_path): diff --git a/libs/utils/test.py b/libs/utils/test.py index d3c30928b..7e741ee03 100644 --- a/libs/utils/test.py +++ b/libs/utils/test.py @@ -15,6 +15,7 @@ # limitations under the License. # +import copy import os import unittest import logging @@ -63,7 +64,15 @@ class LisaTest(unittest.TestCase): def _getTestConf(cls): if cls.test_conf is None: raise NotImplementedError("Override `test_conf` attribute") - return cls.test_conf + # Shallow copy of the test conf so we can update its results_dir + # without modifying it for all classes that may share the same test_conf + # object (through their base class for example). + test_conf = copy.copy(cls.test_conf) + # Using a qualified name avoids clashes if the same class name is used + # by different modules. + test_name = cls.__module__ + '.' + cls.__name__ + test_conf.setdefault('results_dir', test_name) + return test_conf @classmethod def _getExperimentsConf(cls, test_env): diff --git a/src/shell/lisa_shell b/src/shell/lisa_shell index 0cd5b12ed..69238cf12 100755 --- a/src/shell/lisa_shell +++ b/src/shell/lisa_shell @@ -296,8 +296,7 @@ cat <