From 29986b08e0288b4ea34d13e7c74dd88e58014738 Mon Sep 17 00:00:00 2001 From: Rupert Swarbrick Date: Fri, 3 Apr 2026 22:46:34 +0100 Subject: [PATCH] refactor: Move tool name to FlowCfg This value was always actually supplied (either directly in SimCfg or through OneShotCfg). Move it to the base class and consume it in just one place. A specific type looks like there might be problems where we try to run a job without having decided on the tool. The second instance of this (in CovReport.post_finish) can definitely never happen, but the example in Deploy.get_job_spec is less obvious to me, so I've put in a runtime check. Signed-off-by: Rupert Swarbrick --- src/dvsim/flow/base.py | 4 ++++ src/dvsim/flow/one_shot.py | 1 - src/dvsim/job/deploy.py | 15 +++++++++++++++ src/dvsim/sim/flow.py | 1 - 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/dvsim/flow/base.py b/src/dvsim/flow/base.py index e90c17d1..becb5571 100644 --- a/src/dvsim/flow/base.py +++ b/src/dvsim/flow/base.py @@ -79,6 +79,10 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None: self.scratch_path = "" self.scratch_base_path = "" + # The command line might specify the tool (with --tool). If not, we + # leave it as None (allowing an hjson file to populate it) + self.tool: str | None = args.tool + # Add exports using 'exports' keyword - these are exported to the child # process' environment. self.exports = [] diff --git a/src/dvsim/flow/one_shot.py b/src/dvsim/flow/one_shot.py index c717eea2..df0ee3e8 100644 --- a/src/dvsim/flow/one_shot.py +++ b/src/dvsim/flow/one_shot.py @@ -25,7 +25,6 @@ class OneShotCfg(FlowCfg): def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None: # Options set from command line - self.tool = args.tool self.verbose = args.verbose self.flist_gen_cmd = "" self.flist_gen_opts = [] diff --git a/src/dvsim/job/deploy.py b/src/dvsim/job/deploy.py index b31f534a..e441040a 100644 --- a/src/dvsim/job/deploy.py +++ b/src/dvsim/job/deploy.py @@ -116,6 +116,16 @@ def __init__(self, sim_cfg: "FlowCfg") -> None: def get_job_spec(self) -> "JobSpec": """Get the job spec for this deployment.""" + # At this point, the configuration should have populated its tool field + # (either from a command line argument or a value in the hjson that was + # loaded. If not, we don't know what to do. + if self.sim_cfg.tool is None: + msg = ( + "No tool selected in job configuration. It must either be " + "specified in the hjson or passed with the --tool argument." + ) + raise RuntimeError(msg) + return JobSpec( name=self.name, job_type=self.__class__.__name__, @@ -845,6 +855,11 @@ def callback(status: JobStatus) -> None: if self.dry_run or status != JobStatus.PASSED: return + # At this point, we have finished running a tool, so we know that + # self.sim_cfg.tool must have been set. + if self.sim_cfg.tool is None: + raise AssertionError("sim_cfg.tool cannot be None now.") + plugin = get_sim_tool_plugin(tool=self.sim_cfg.tool) results, self.cov_total = plugin.get_cov_summary_table( diff --git a/src/dvsim/sim/flow.py b/src/dvsim/sim/flow.py index 664f3163..731a81f5 100644 --- a/src/dvsim/sim/flow.py +++ b/src/dvsim/sim/flow.py @@ -80,7 +80,6 @@ class SimCfg(FlowCfg): def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None: # Options set from command line - self.tool = args.tool self.build_opts = [] self.build_opts.extend(args.build_opts) self.en_build_modes = args.build_modes.copy()