Hi, Sergey!

Updated and force-pushed.

Sergey

Hi, Sergey!
See my answer below.

On 24.02.25, Sergey Bronnikov wrote:

Hi, Sergey,

thanks for review!

<snipped>

---
+
+  local default_output_file = 'memprof.bin'
+  os.remove(default_output_file)
+
+  local _, _ = misc.memprof.start()

Minor: Do we want to check that the profiler starts successfully?
I suppose we should, since this is the expected behaviour for this
feature. In case the profiler is not started (old behaviour) we would
get an error from the branch below: profiler not running. This isn't a
verbose definition of what goes wrong.

I don't get why we should check that profiler is started in a test for 
default output file.

If the `memprof.start()` call will fail in the test (for any reason),
the next call of `memprof.stop()` will return `false, "profiler is not
running"`. So, when debugging the test, we have no clue about the reason
for the not-started profiler, which is not very convenient.

The patch below handles error from memprof start and stop:

diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua
index ae8a73c9..55b5c60b 100644
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi-default-file.test.lua
@@ -16,21 +16,26 @@ test:test('default-output-file', function(subtest)
   local default_output_file = 'memprof.bin'
   os.remove(default_output_file)
 
-  local _, _ = misc.memprof.start()
-
-  local res, err = misc.memprof.stop()
+  local res, err = misc.memprof.start()
+  -- Want to cleanup carefully if something went wrong.
+  if not res then
+    test:fail('sysprof was not started: ' .. err)
+    os.remove(default_output_file)
+  end
 
+  res, err = misc.memprof.stop()
   -- Want to cleanup carefully if something went wrong.
   if not res then
+    test:fail('sysprof was not started: ' .. err)
     os.remove(default_output_file)
-    error(err)
   end
 
+
   local profile_buf = tools.read_file(default_output_file)
   subtest:ok(profile_buf ~= nil and #profile_buf ~= 0,
              'default output file is not empty')
 
-  -- We don't need it any more.
+  -- We don't need it anymore.
   os.remove(default_output_file)
 end)
 

I suppose we may use `goto` here like the following:

|   local res, err = misc.memprof.start()
|   -- Should start successfully.
|   if not res then
|     goto err_handling
|   end
|
|   res, err = misc.memprof.stop()
|
| ::err_handling::
|   -- Want to cleanup carefully if something went wrong.
|   if not res then

+
+  local res, err = misc.memprof.stop()
+
+  -- Want to cleanup carefully if something went wrong.
+  if not res then
+    os.remove(default_output_file)
+    error(err)
+  end

<snipped>