SOF_TEST_INTERVAL fixes #821
Conversation
case-lib/config.sh
Outdated
| # safer: it makes sure no kernel error escapes. | ||
| # See initial review in https://github.com/thesofproject/sof-test/pull/639 | ||
| # | ||
| # Note the default value used by Intel's sof-jenkins is 5 seconds, as |
There was a problem hiding this comment.
Please note that Jenkins doesn't run any test case, it run case by making use of the test runner, which is sof-framework.
The sof-framework never sets this value, it just wait 5 seconds before it run next test case, that's why we make the initial value 5 seconds, only log within the 1 second before running test case will be collected
There was a problem hiding this comment.
The sof-framework never sets this value
Why not? That seems wrong. sof-test should be user friendly, it should not force everyone else and especially not interactive users to configure this because one script in sof-framework is lazy.
that's why we make the initial value 5 seconds
You mean the default value?
only log within the 1 second before running test case will be collected
I fail to parse this part sorry.
There was a problem hiding this comment.
here you make SOF_TEST_INTERVAL=${SOF_TEST_INTERVAL:-1}, the default value is 1, so the log collect point is 4 (- SOF_TEST_INTERVAL - 3) seconds before running the next test case. but the last test end for CI is 5 second before running the test case, so the log in the 1 second after last test end is not collected in CI.
The framework currently uses default values of sof-test, only TPLG and MODEL are set by sof-framework. In later improvement, we should provide a unified way in sof-framework to set sof-test config variables.
There was a problem hiding this comment.
It is a bit ridiculous that the most frequent sof-test user and the one that needs SOF_TEST_INTERVAL the most does not define SOF_TEST_INTERVAL. I filed internal bug 158, kept the default value as is, adjusted the comments and resubmitted all the other changes.
marc-hb
force-pushed
the
test-interval
branch
2 times, most recently
from
7ebd8d1 to
2d0cadf
Compare
|
The 2 timeouts in https://sof-ci.01.org/softestpr/PR821/build941/devicetest/ don't seem related. I force-pushed something unrelated to this branch by mistake, apologies. I force-pushed and restored the earlier commits. |
Pure comment changes, zero code change. Notably clarify that SOF_TEST_INTERVAL is not something that can be freely defined, it must reflect reality. Remove KERNEL_CHECKPOINT comment from config.sh because it's not something we want most users to mess with. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
SOF_TEST_INTERVAL is the "sleep" delay between the end of one sof-test and the start of the next one. However this does not account for our own delays collecting logs, which makes some bugs like thesofproject/sof#5032 being caught semi-randomly. Err on the safe side and add 3 seconds for more determinism (this will of course never be completely deterministic). Signed-off-by: Marc Herbert <marc.herbert@intel.com>
It is necessary to inform sof-test otherwise some errors can be reported twice. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb
force-pushed
the
test-interval
branch
from
2d0cadf to
f44b228
Compare
|
CML was not available in https://sof-ci.01.org/softestpr/PR821/build945/devicetest/ but everything else is green. |
|
As reported by @plbossart , just found that SOF_TEST_INTERVAL was also used in test iterations. Fix submitted in: |
2 participants
See commit messages