[feat] Log all tests in perfloggers, not only performance tests

[ekouts]

Fixes #3460 .

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.22%. Comparing base (8000b58) to head (efb61fc).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3516      +/-   ##
===========================================
- Coverage    91.23%   91.22%   -0.01%     
===========================================
  Files           62       62              
  Lines        13464    13473       +9     
===========================================
+ Hits         12284    12291       +7     
- Misses        1180     1182       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vkarak]

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.

[ekouts]

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.

[vkarak]

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.

[ekouts]

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]

I'd like to do it a bit more native if possible. This is like we are hacking the perfvalues to make it work for logging sanity-only tests.

[vkarak]

Actually this is not needed at all. It's very easy to achieve what we want by simply removing the following check:

diff --git a/reframe/core/logging.py b/reframe/core/logging.py
index e166f857b..b44b1d8d8 100644
--- a/reframe/core/logging.py
+++ b/reframe/core/logging.py
@@ -931,7 +931,7 @@ class LoggerAdapter(logging.LoggerAdapter):
         )
 
     def log_performance(self, level, task, msg=None, multiline=False):
-        if self.check is None or not self.check.is_performance_check():
+        if self.check is None:
             return
 
         _, part, env = task.testcase

[vkarak]

So I think the only changes needed are the following:

1. Remove the check above
2. Rename the log_performance() to log_result()

The only question remaining is whether we need a configuration option for this.