-
Notifications
You must be signed in to change notification settings - Fork 328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve stability of riscv-tests for targets with low remotetimeout value #1065
Comments
@aap-sc Please, what is the relationship between the timeouts in the testsuite and the coverage? In what way does the extended timeout reduce the coverage? Thank you for explanation. |
I presumed that the reasoning here was that (subject to further investigation/analysis) some of the unexpected test failures/exceptions might point to problems with OpenOCD itself - in particular not sending sufficient "keep alive" packets to GDB to avoid anomalous timeouts? FWIW at the moment I'm encountering several problems when running tests:
|
One may have an impression that the current testsuite from riscv-tests is a set of directed tests. It is not. It is more like a set of end-to-end tests that involve a rather complex communication between gdb, openocd, spike/jtag adapter. Typical GDB command should never fail due to a timeout value even if the target is relatively slow (there are limits of course). Most ordinary users won't touch (and should not touch) "Reducing coverage" in this context means that after I've increased the For example, in context of https://github.com/riscv-collab/riscv-openocd/issues/1049 ( (sporadic DownloadTest failure part) the underlying issue is that when OpenOCD processes qCRC request it does not send keep_alive packets. Here is the patch to address this: https://review.openocd.org/c/openocd/+/8227 If we always have these timeouts increased, situations like the one presented above won't happen. Means it won't be tested -> coverage is reduced. |
Yes, this is precisely the reason. |
@aap-sc @TommyMurphyTM1234 Understood, thank you for explanation! |
After riscv-software-src/riscv-tests#553 was merged in @en-sc expressed the concern that this is a rather radical change and it reduces coverage of the test suite.
During a very heated discussion we've decided that:
@en-sc , @TommyMurphyTM1234 FYI
The text was updated successfully, but these errors were encountered: