tests/lapi: add sysprof test #151
Conversation
ligurio
force-pushed
the
ligurio/gh-xxxx-lapi-sysprof
branch
from
586ceda to
421cf11
Compare
ligurio
force-pushed
the
ligurio/gh-xxxx-lapi-sysprof
branch
from
421cf11 to
4172439
Compare
tests/lapi/misc_sysprof_test.lua
Outdated
| local MAX_INT = test_lib.MAX_INT | ||
| local MIN_INT = test_lib.MIN_INT | ||
|
|
||
| local has_tnt_sysprof, sysprof = pcall(require, "misc.sysprof") |
There was a problem hiding this comment.
I would rather separate those profilers to make test a little bit clearer.
There was a problem hiding this comment.
splited to two tests:
tests/lapi/jit_p_test.lua
tests/lapi/misc_sysprof_test.lua
There was a problem hiding this comment.
We can move misc_sysprof_test.lua to tarantool repo if you wish. Because it is not applicable to other Lua runtimes and Tarantool-specific.
.luacheckrc
Outdated
| @@ -1,3 +1,6 @@ | |||
| -- Tarantool introduces a global for misc API namespace. | |||
| read_globals = { "misc" } | |||
There was a problem hiding this comment.
This is not necessary, since you may refer the sysprof's functions via require result.
tests/lapi/misc_sysprof_test.lua
Outdated
| "i", -- Sampling interval in milliseconds (default 10ms). | ||
| } | ||
|
|
||
| local SYSPROF_DEFAULT_INTERVAL = 10 -- ms |
There was a problem hiding this comment.
It's better to use the minimum possible interval here.
tests/lapi/misc_sysprof_test.lua
Outdated
| mode = mode, | ||
| path = "/dev/null", | ||
| }) | ||
| assert(err) |
There was a problem hiding this comment.
Nit: It may be written as the following:
assert(misc.sysprof.start(...))
Feel free to ignore.
There was a problem hiding this comment.
wrapped with assert(), thanks!
There was a problem hiding this comment.
[PATCH 2/2] tests/capi: omit sysprof testing
With test misc_sysprof_test.lua
added in the previous commit sysprof in luaL_loadbuffer_proto_test
is not needed anymore.
I do not agree with this:
- It handles the different API invocations (not the C callback, but the Lua function).
- The test added in the previous commit is not very useful unless the Lua mutator is added.
Thus, I suggest dropping this commit for now.
removed from the patchset (cherry-picked to the branch with |
The patch adds tests for sysprof built into LuaJIT and Tarantool's sysprof.
ligurio
force-pushed
the
ligurio/gh-xxxx-lapi-sysprof
branch
from
4172439 to
08f5667
Compare
No description provided.