-
Notifications
You must be signed in to change notification settings - Fork 3k
Check max possible keys in NVStore tests #7670
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
Check max possible keys in NVStore tests #7670
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine for me. Approved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, just a question that is not answered:
skipping this test does not have any coverage impact - all is fine?
Good question. Skipping tests is a policy enforced in very extreme lack of resource cases. In our specific case, the test is skipped if the flash can't hold 20 NVStore keys, which is very extreme (and happened due to another bug, if you follow the issue). Now, we can dilute our tests to make them fit all low end boards, but then it would harm the test quality. We can also have a fixed list of boards, ones which we know that can handle the test (I know this is done in LittleFS tests). However, this will form a sort of a "white list", which covers less boards at the end of the day. So skipping tests in extreme cases seems like the right thing to do. |
Thanks for the answer for testing also upper limit that not all boards can make. 👍 |
/morph build |
Build : SUCCESSBuild number : 2792 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 2422 |
/morph test |
Test : FAILUREBuild number : 2527 |
Wat. There are no test results... /morph test |
Test : FAILUREBuild number : 2537 |
Yet another DNS test error. Everything else looks fine. |
Looks like this has failed due to some networking issues in CI. Suggest we re-run this once the CI is less heavily loaded. |
@davidsaada This particular test is also being moved out of the regular CI since it's causing so many false issues. |
/morph test |
Test : SUCCESSBuild number : 2553 |
…t_max_possible_keys Check max possible keys in NVStore tests
Check max possible keys in NVStore tests: Boards that have large flash pages and small sectors may not be able to hold many NVStore keys. NVStore tests didn't take this into account and set a fixed number of keys. This PR checks the maximal number of keys before doing that, and if insufficient - the test is skipped.
Fixes #7649.
Pull request type