[dvsim] Fix --cov + --build|run-only bugs
The `SimCfg` prunes the number of builds if there is no uniqueness among
the existing build objects. If a simulation is rerun with --run-only
switch (the assumption here being there is no need to rebuild the
simulation executables since the source code did not change), with the
old code, the build pruning is not performed, causing the tests to
reference non-existent builds and fail. This commit fixes that issue.
If `--cov --build-only` is passed on the command line (with the intent
to pass `--cov --run-only` later), builds get prematurely pruned out
because we turn off the coverage switch. This prevents us from doing
`--cov --build-only` and `--cov --run-only` as two separate steps.
Also, the coverage table and summary table end up throwing errors. This
change fixes that.
In addition, the `--cov-merge-previous` was also broken (appending Path
vars instead of str). The `odir_limiter` also has a minor fix such that
if `odir` does not exist, it still prunes and returns the old dirs.
The cov merge task also needed information from the builds, which do not
exist if `--run-only` switch is passed. The CovMerge code is updated to
seek the same information from runs instead.
Signed-off-by: Srikrishna Iyer <sriyer@google.com>
diff --git a/util/dvsim/Deploy.py b/util/dvsim/Deploy.py
index 7ed1e01..0d2b55a 100644
--- a/util/dvsim/Deploy.py
+++ b/util/dvsim/Deploy.py
@@ -287,16 +287,17 @@
remain after deletion.
"""
- if not os.path.exists(odir):
- return []
-
- # If output directory exists, back it up.
- ts = datetime.fromtimestamp(os.stat(odir).st_ctime)
- ts = ts.strftime(self.sim_cfg.ts_format)
- shutil.move(odir, odir + "_" + ts)
+ if os.path.exists(odir):
+ # If output directory exists, back it up.
+ ts = datetime.fromtimestamp(os.stat(odir).st_ctime)
+ ts = ts.strftime(self.sim_cfg.ts_format)
+ shutil.move(odir, odir + "_" + ts)
# Get list of past output directories sorted by creation time.
pdir = Path(odir).resolve().parent
+ if not pdir.exists():
+ return []
+
dirs = sorted([old for old in pdir.iterdir() if old.is_dir()],
key=os.path.getctime,
reverse=True)
@@ -643,6 +644,7 @@
self.mandatory_misc_attrs.update({
"run_dir_name": False,
+ "cov_db_dir": False,
"cov_db_test_dir": False,
"run_pass_patterns": False,
"run_fail_patterns": False
@@ -797,9 +799,11 @@
CovMerge.items.append(self)
def __post_init__(self):
- # Add cov db dirs from all the builds that were kicked off.
- for bld in self.sim_cfg.builds:
- self.cov_db_dirs += bld.cov_db_dir + " "
+ # Extract cov db dirs from all the sim runs.
+ for item in self.dependencies:
+ if item.target == "run":
+ if item.cov_db_dir not in self.cov_db_dirs:
+ self.cov_db_dirs += item.cov_db_dir + " "
# Recursively search and replace wildcards, ignoring cov_db_dirs.
# We need to resolve it later based on cov_db_dirs value set below.
@@ -829,7 +833,8 @@
# that as well for merging, if the --cov-merge-previous command line
# switch is passed.
if self.sim_cfg.cov_merge_previous:
- self.cov_db_dirs += prev_cov_db_dirs
+ self.cov_db_dirs += " ".join(
+ [str(item) for item in prev_cov_db_dirs])
# Append cov_db_dirs to the list of exports.
self.exports["cov_db_dirs"] = "\"{}\"".format(self.cov_db_dirs)
diff --git a/util/dvsim/SimCfg.py b/util/dvsim/SimCfg.py
index c9f9074..89b16be 100644
--- a/util/dvsim/SimCfg.py
+++ b/util/dvsim/SimCfg.py
@@ -125,10 +125,6 @@
self.dry_run = args.dry_run
self.map_full_testplan = args.map_full_testplan
- # Disable cov if --build-only is passed.
- if self.build_only:
- self.cov = False
-
# Set default sim modes for unpacking
if args.waves is not None:
self.en_build_modes.append("waves")
@@ -482,41 +478,41 @@
# Create the build and run list first
self._create_build_and_run_list()
+ self.builds = []
+ build_map = {}
+ for build_mode_obj in self.build_list:
+ new_build = CompileSim(build_mode_obj, self)
+
+ # It is possible for tests to supply different build modes, but
+ # those builds may differ only under specific circumstances,
+ # such as coverage being enabled. If coverage is not enabled,
+ # then they may be completely identical. In that case, we can
+ # save compute resources by removing the extra duplicated
+ # builds. We discard the new_build if it is equivalent to an
+ # existing one.
+ is_unique = True
+ for build in self.builds:
+ if build.is_equivalent_job(new_build):
+ new_build = build
+ is_unique = False
+ break
+
+ if is_unique:
+ self.builds.append(new_build)
+ build_map[build_mode_obj] = new_build
+
+ # Update all tests to use the updated (uniquified) build modes.
+ for test in self.run_list:
+ if test.build_mode.name != build_map[test.build_mode].name:
+ test.build_mode = Modes.find_mode(
+ build_map[test.build_mode].name, self.build_modes)
+
if self.run_only:
self.builds = []
- self.runs = self._expand_run_list(None)
- else:
- self.builds = []
- build_map = {}
- for build_mode_obj in self.build_list:
- new_build = CompileSim(build_mode_obj, self)
+ build_map = None
- # It is possible for tests to supply different build modes, but
- # those builds may differ only under specific circumstances,
- # such as coverage being enabled. If coverage is not enabled,
- # then they may be completely identical. In that case, we can
- # save compute resources by removing the extra duplicated
- # builds. We discard the new_build if it is equivalent to an
- # existing one.
- is_unique = True
- for build in self.builds:
- if build.is_equivalent_job(new_build):
- new_build = build
- is_unique = False
- break
-
- if is_unique:
- self.builds.append(new_build)
- build_map[build_mode_obj] = new_build
-
- # Update all tests to use the updated (uniquified) build modes.
- for test in self.run_list:
- if test.build_mode.name != build_map[test.build_mode].name:
- test.build_mode = Modes.find_mode(
- build_map[test.build_mode].name, self.build_modes)
-
- self.runs = ([] if self.build_only else
- self._expand_run_list(build_map))
+ self.runs = ([]
+ if self.build_only else self._expand_run_list(build_map))
self.deploy = self.builds + self.runs
@@ -577,9 +573,6 @@
'''
deployed_items = self.deploy
- if self.cov:
- deployed_items.append(self.cov_merge_deploy)
-
results = Results(deployed_items, run_results)
# Set a flag if anything failed
@@ -619,7 +612,7 @@
self.results_summary = self.testplan.results_summary
# Append coverage results of coverage was enabled.
- if self.cov:
+ if self.cov_report_deploy is not None:
report_status = run_results[self.cov_report_deploy]
if report_status == "P":
results_str += "\n## Coverage Results\n"
@@ -662,26 +655,29 @@
# sim summary result has 5 columns from each SimCfg.results_summary
header = ["Name", "Passing", "Total", "Pass Rate"]
- if self.cov:
+ if self.cov_report_deploy is not None:
header.append('Coverage')
- table = [header]
+ table = []
colalign = ("center", ) * len(header)
for item in self.cfgs:
row = []
for title in item.results_summary:
row.append(item.results_summary[title])
- if row == []:
- continue
- table.append(row)
+ if row:
+ table.append(row)
self.results_summary_md = "## " + self.results_title + " (Summary)\n"
self.results_summary_md += "### " + self.timestamp_long + "\n"
if self.revision:
self.results_summary_md += "### " + self.revision + "\n"
self.results_summary_md += "### Branch: " + self.branch + "\n"
- self.results_summary_md += tabulate(table,
- headers="firstrow",
- tablefmt="pipe",
- colalign=colalign)
+ if table:
+ self.results_summary_md += tabulate(table,
+ headers=header,
+ tablefmt="pipe",
+ colalign=colalign)
+ else:
+ self.results_summary_md += "\nNo results to display.\n"
+
print(self.results_summary_md)
return self.results_summary_md
@@ -689,7 +685,7 @@
'''Publish coverage results to the opentitan web server.'''
super()._publish_results()
- if self.cov:
+ if self.cov_report_deploy is not None:
results_server_dir_url = self.results_server_dir.replace(
self.results_server_prefix, self.results_server_url_prefix)
