Skip to content

Prevent stack overflow from general_block_device test. #10405

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

Closed
wants to merge 1 commit into from

Conversation

korjaa
Copy link
Contributor

@korjaa korjaa commented Apr 15, 2019

This test was failing randomly on NUCLEO_F412ZG using SDCard (https://jira.arm.com/browse/IOTSTOR-828).

Description

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

This test was failing randomly on NUCLEO_F412ZG using SDCard (https://jira.arm.com/browse/IOTSTOR-828).
@ciarmcom
Copy link
Member

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

@korjaa
Copy link
Contributor Author

korjaa commented Apr 15, 2019

Related ticket https://jira.arm.com/browse/IOTSTOR-828

@cmonr
Copy link
Contributor

cmonr commented Apr 16, 2019

@cmonr
Copy link
Contributor

cmonr commented Apr 16, 2019

CI started.

@0xc0170 0xc0170 requested a review from a team April 17, 2019 08:16
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 17, 2019

@korjaa Why is this not targeting master but release branch?

Copy link
Contributor

@davidsaada davidsaada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have modified this test to address these issues. Does it still happen in the latest master?
Increasing stack size will hurt boards having low memory. I don't think this change is required.

@mbed-ci
Copy link

mbed-ci commented Apr 17, 2019

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@korjaa
Copy link
Contributor Author

korjaa commented Apr 23, 2019

@0xc0170 The tests have changed in the master branch to a shorter version (only one block operation per thread if I didn't read the log wrong) and I was not able to reproduce the issue. The block operations need to be done several times in multiple threads for the issue to show up. I feel the new tests are so short that they usually always pass.

@adbridge
Copy link
Contributor

adbridge commented Apr 26, 2019

@korjaa Why is this not targeting master but release branch?

@korjaa We don't actually accept PRs directly to a release branch which is what @0xc0170 was trying to say...Please re-direct this to master and fix any issues accordingly.

@korjaa
Copy link
Contributor Author

korjaa commented May 3, 2019

This PR is not valid against master as the test flow has changed in the master. However, I do still think this would useful for anyone using 5.12.x. But if patches are not accepted to releases, I'm not sure how a change like this can go in.

@0xc0170
Copy link
Contributor

0xc0170 commented May 3, 2019

Based on the feedback above, this can be closed. It was addressed on master, should not be present on release branch neither.

@0xc0170 0xc0170 closed this May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants