Specify absolute path to benchcab executable #117
Conversation
Codecov Report
@@ Coverage Diff @@
## master #117 +/- ##
==========================================
+ Coverage 88.48% 88.55% +0.07%
==========================================
Files 26 26
Lines 1372 1381 +9
==========================================
+ Hits 1214 1223 +9
Misses 158 158
|
This change specifies the absolute path to the benchcab executable in the job script so that we don't run into path issues when running using a 'locally installed' version of benchcab. This will be helpful making code changes and testing them on NCI. Fixes #95
SeanBryan51
force-pushed
the
95-specify-absolute-path-to-executable
branch
from
296077c
to
c969c5e
Compare
benchcab/benchcab.py
Outdated
| benchcab_path=self.benchcab_exe_path | ||
| if self.benchcab_exe_path | ||
| else "benchcab", |
There was a problem hiding this comment.
Is this really the behaviour we want when calling render_job_script? If self.benchcab_exe_path evaluates to False, shouldn't we instead raise an error? Isn't it a sign of something that has gone wrong within benchcab?
There was a problem hiding this comment.
Yes we should probably raise an error here since it means something has gone wrong with the path. This was a bit of a hack to get the unit tests to pass since self.benchcab_exe_path is always None unless we have installed benchcab. The right way is probably to specify self.benchcab_exe_path as an argument so we can mock it in the tests.
We should not use a default value when trying to infer the path to the benchcab executable. If the path is undefined, we raise an exception as this is likely a sign that the user's PATH variable is configured incorrectly. Co-authored-by: Claire Carouge <[email protected]>
SeanBryan51
force-pushed
the
95-specify-absolute-path-to-executable
branch
from
018c4d8
to
9238d08
Compare
This change specifies the absolute path to the benchcab executable in the job script so that we don't run into path issues when running using a 'locally installed' version of benchcab. This will be helpful making code changes and testing them on NCI.
Fixes #95