Skip to content

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

Merged

Conversation

davidsaada
Copy link
Contributor

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

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

Copy link
Contributor

@yossi2le yossi2le left a 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.

Copy link
Contributor

@0xc0170 0xc0170 left a 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?

@davidsaada
Copy link
Contributor Author

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.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 2, 2018

Thanks for the answer for testing also upper limit that not all boards can make. 👍

@cmonr
Copy link
Contributor

cmonr commented Aug 14, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Aug 14, 2018

Build : SUCCESS

Build number : 2792
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7670/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Aug 14, 2018

@cmonr
Copy link
Contributor

cmonr commented Aug 14, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Aug 14, 2018

@cmonr
Copy link
Contributor

cmonr commented Aug 15, 2018

Wat. There are no test results...

/morph test

@mbed-ci
Copy link

mbed-ci commented Aug 15, 2018

@davidsaada
Copy link
Contributor Author

Yet another DNS test error. Everything else looks fine.

@adbridge
Copy link
Contributor

Looks like this has failed due to some networking issues in CI. Suggest we re-run this once the CI is less heavily loaded.

@cmonr
Copy link
Contributor

cmonr commented Aug 15, 2018

@davidsaada This particular test is also being moved out of the regular CI since it's causing so many false issues.

@cmonr
Copy link
Contributor

cmonr commented Aug 16, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Aug 16, 2018

@cmonr cmonr merged commit 4672658 into ARMmbed:master Aug 16, 2018
@davidsaada davidsaada deleted the david_fix_nvstore_test_max_possible_keys branch August 22, 2018 09:09
pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018
…t_max_possible_keys

Check max possible keys in NVStore tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing nvstore-functionality tests on GR_LYCHEE
6 participants