Skip to content
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

Reduce number of threads in block device test #9931

Merged
merged 3 commits into from Mar 6, 2019

Conversation

@offirko
Copy link
Contributor

commented Mar 4, 2019

Description

Storage Tests Fix:

  1. Prevent FS tests from running on internal FlashIAP
  2. Reduce number of threads for general BD multi-thread test
    and verify memory resources before running test

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change
@offirko

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

@mikisch81

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

As outsider for these tests - a reason for reducing the test coverage (not enabling to run on internal flash) ? should be in the commit msg ?

Was there a problem ?

@dannybenor

This comment has been minimized.

Copy link

commented Mar 4, 2019

Filesystem is not intended to work in internal flash, even if the test works. We want to avoid spending time fixing tests or code that should not be part of our offer. The solution for internal flash is TDBStore

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Mar 4, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

@offirko, thank you for your changes.
@ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

@mikisch81

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

@ARMmbed/mbed-os-maintainers @maclobdell @amiraloosh this is needed for #9910 (new PSA target) so it needs to have 5.12 label

features/storage/TESTS/blockdevice/general_block_device/main.cpp Outdated
char *dummy;

rtos::Thread **bd_thread = new (std::nothrow) rtos::Thread*[TEST_NUM_OF_THREADS];
TEST_SKIP_UNLESS_MESSAGE((*bd_thread) != NULL, "no block device found.");

This comment has been minimized.

Copy link
@theamirocohen

theamirocohen Mar 5, 2019

Contributor

Message should reflect the skip reason

This comment has been minimized.

Copy link
@davidsaada

davidsaada Mar 5, 2019

Contributor

Fixed. Please re-review.

@ChiefBureaucraticOfficer
Copy link

left a comment

Meets PM Criteria - targeted fix.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

Please update this commit 1f756ec such that the reasoning in this comment is added: #9931 (comment)

It makes figuring out why a skip was added considerably easier, since searching for the PR where that change was introduced is not easy.

@cmonr
Copy link
Contributor

left a comment

#9931 (comment)

Can start CI once corrected.

offirko and others added some commits Mar 4, 2019

Reduce number of threades in block device test
In addition, prevent FS tests from running on internal flash,
due to the fact that file system on internal flash is not part of
our offering (TDBStore should be used there instead).

@davidsaada davidsaada force-pushed the offirko:offir_nxp branch to 3c14dd4 Mar 5, 2019

@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

#9931 (comment)

Can start CI once corrected.

Corrected now.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

CI started for now.

Suspect export issue is still a problem, but that job can be restarted once fixed.

@cmonr

cmonr approved these changes Mar 5, 2019

@cmonr cmonr added needs: CI and removed needs: review labels Mar 5, 2019

@cmonr cmonr changed the title Reduce number of threades in block device test Reduce number of threads in block device test Mar 5, 2019

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 5, 2019

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 1
Build artifacts

@cmonr cmonr merged commit 26920fe into ARMmbed:master Mar 6, 2019

21 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARMC5 Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR8 Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/greentea-test Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9078 cycles (-1084 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the needs: CI label Mar 6, 2019

@offirko

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

Issue created to handle the TFM threads limitation:
https://jira.arm.com/browse/IOTSEC-1120

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.