[feat] Log also results from sanity-only tests through the perflog handlers#3516
[feat] Log also results from sanity-only tests through the perflog handlers#3516vkarak merged 10 commits intoreframe-hpc:developfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3516 +/- ##
========================================
Coverage 91.25% 91.25%
========================================
Files 62 62
Lines 13534 13534
========================================
Hits 12350 12350
Misses 1184 1184 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vkarak
left a comment
There was a problem hiding this comment.
lI think it should be even simpler. Since log_performance() is already called in sanity failures for performance tests, what if we simply rename it to log_result() and remove the check is_performance_check() from the log_performance()?
I think this should pretty much get us to what we want.
But could it be an issue for users if we add more entries in their logging, without giving the option to disable? For CSCS it's of course fine, but I added the option so that we keep compatibility. |
I don't it's such a big issue as it is not breaking anything. Besides, the perflogs are per test, so there will not be new entries in existing files. So I'd rather go with this change than adding yet another configuration parameter. |
Ok! will make the change :) |
vkarak
left a comment
There was a problem hiding this comment.
Lgtm, just a couple of minor comments.
77021c9 to
95f26be
Compare
Fixes #3460 .