Uh oh!
There was an error while loading. Please reload this page.
boot test and virtual heap testing - #7688
Conversation
lyakh
commented
Jun 6, 2023
SOFCI TEST |
1 similar comment
lyakh
commented
Jun 6, 2023
SOFCI TEST |
2d2fc61 to
5a11244Compare
lgirdwood
left a comment
There was a problem hiding this comment.
@dabekjakub is this test now passing with latest VMH PR ?
marc-hb
left a comment
There was a problem hiding this comment.
Do we really need yet another, brand new test framework? Genuine question. Can't Zephyr's ZTEST and twister help with this?
https://docs.zephyrproject.org/latest/develop/test/ztest.html
lyakh
commented
Nov 17, 2023
@marc-hb as far as I understand - no, not easily, ztest seems to be (mostly) used with twister when building a separate image just for testing, whereas what we want is a kind of a boot self-test, similar to what the kernel can do. I'd much prefer to use an existing framework and if ztest or another Zephyr API can do that - I'd love to know about it and to switch to it! Or maybe we want to add boot-time self-testing to Zephyr? |
lyakh
commented
Nov 17, 2023
@lgirdwood no, I see a failure in the log, I'll double check whether I'm calling the API wrongly or if there's a problem in VMH. |
lyakh
commented
Nov 17, 2023
Unfortunately all https://sof-ci.01.org/sofpr/PR7688/build14963/devicetest/index.html test results are empty, but in my local test I got this: So either I'm testing wrongly or there's a problem with VMH, @dabekjakub ? |
ztest should be usable outside of twister, its a way of structuring test code and checks (asserts) You can run any test in the zephyr/tests directory by building and flashing normally with west, nothing particularly special about twister doing this. And you can decide when and how to run but by default it creates a main to do this for you. |
lyakh
commented
Nov 20, 2023
@teburd right, thanks. My understanding is that maybe it can be used but it hardly would bring anything in this configuration. What we need is to run multiple tests at SOF startup time. So it should be an otherwise normal SOF build, having run those tests it will proceed and function normally as any other SOF firmware. And it seems to me that we need to run them at a specific point in SOF run-time sequence, long after all Zephyr initialisation has completed. What advantages would we still gain if using ztest - if at all possible? We'd need to make a new launch point inside of SOF too, right? |
lgirdwood
commented
Nov 24, 2023
Today we have very little UTs, so yes, we can add more and also add existing cmocka tests where appropriate (and if posible). Kconfig needs to set a flag in the ext manifest so that kernel knows "this is a test" |
lgirdwood
left a comment
There was a problem hiding this comment.
LGTM - ztest "hello world"
@lyakh dont see the extended manifest flag yet. How do we report failures when a test does not assert() or cause an exception ? e.g. will everything go out via trace
test 0: feature1 vmh.c:54 - pass
test 1: feature2 vmh.c:87 - failed - got 97 expected 78.
....
test: all test completed 89 passed 3 failed
| config SOF_BOOT_TEST | ||
| bool "enable SOF run-time testing" | ||
| depends on ZTEST | ||
| help |
There was a problem hiding this comment.
Can we expand this more and tell the users that its for testing use only and should not be enabled for normal audio usage. i.e what this enables/disables and impact.
| if (!h) | ||
| return -ENOMEM; | ||
| return vmh_free_heap(h); |
There was a problem hiding this comment.
@lyakh@dabekjakub this is a simple test today i.e. "hello world", but lets add to it and stress it when we have the ztest infra understood and merged (as I think we all have good test ideas baking....).
lyakh
commented
Nov 28, 2023
@lgirdwood can we discuss and add manifest and IPC reporting as follow-up PRs? Manifest is needed to make sure that the kernel is expecting any eventual delays?
Yes, logging, unfortunately I don't see any CI device results in this PR, so copying from my local testing (possibly of a modified version): |
lgirdwood
commented
Nov 29, 2023
I think we need to list each UT executed and its result as a 1st step (the total pass/fail can come later). Th point is that all tests should be visible and result recorded. |
lyakh
commented
Nov 30, 2023
@lgirdwood you mean list them in the log? The above output is Zephyr's |
kv2019i
commented
Dec 1, 2023
@lyakh@lgirdwood I think the log output goes to logging subsystem as normal so it looks like this: I'm not sure what happens if the test fails, that should probably be tested. We need some means to observe what happens if the test fails. @lyakh ? |
lyakh
commented
Dec 1, 2023
@kv2019i That would be a separate PR to sof-test? But I'm not sure if we rely on parsing the log, if not, we need a notification IPC and a debugfs entry in the kernel? |
lyakh
commented
Dec 1, 2023
@kv2019i a failure looks like |
lgirdwood
commented
Dec 4, 2023
Yes, pls use ZTEST() API, but this needs to at least print the line number and file name otherwise its less helpful than using original log(). |
lyakh
commented
Dec 5, 2023
@lgirdwood as you see above there's "vmh" in that is the name of the test. We can agree to give more descriptive names, containing at least the file name, and if there are more than one tests in a file, then also an indication of which one this is. Would that be enough? |
| } else { | ||
| ztest_test_pass(); | ||
| } | ||
| } |
There was a problem hiding this comment.
in fact, thinking about this now, this code would probably be common for all SOF boot tests, so we can have a macro to be called in this case something like SOF_ZTEST_BOOT(vmh, vmh_test); I can try to do this now or we can try to switch to one like this later when we get at least a second such boot test
lgirdwood
commented
Dec 5, 2023
As long as all tests are uniquely named with easy discovery then I'm good. |
lgirdwood
commented
Dec 5, 2023
@lyakh can you check CI. I assume this is disabled in default Kconfig as this would obviously fail a CI audio test. |
| if (CONFIG_ACE_VERSION_1_5) | ||
| zephyr_library_sources_ifdef(CONFIG_SOF_BOOT_TEST | ||
| test/vmh.c |
Add a ZTEST test-suite to run SOF boot-time self-tests at the time of the first FW_GEN_MSG IPC. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
This adds an initial Virtual Memory Heap test. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Add calls to vmh_alloc() and vmh_free() for testing. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Test both cases - when spanning multiple blocks is allowed and when it isn't. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
lyakh
commented
Dec 8, 2023
The only CI complaints are style - don't think using |
marc-hb
commented
Dec 13, 2023
From the moment this PR enabled this test, it has always failed on MTL (I think it always passed on cavs25) We never noticed because we never paid attention to FW errors: Now it's causing a DSP panic when combined with another change: |
marc-hb
commented
Dec 14, 2023
Temporarily disabling this test in: |
No description provided.