Skip to content

Fix the reset_reason host test in the CI #11742

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
merged 1 commit into from
Oct 28, 2019

Conversation

fkjagodzinski
Copy link
Member

Description

Use a BaseHostTestAbstract.reset() method instead of BaseHostTestAbstract.reset_dut() for more consistent behavior with various platforms in the CI.

In contrast to a local setup, the reset_dut() method flashes the binary again after performing the reset in a remote setup (RaaS). This was not intentional in reset_reason tests. Moreover, this led to a different behavior (a different value of the reset_reason_t enum) for different flashing methods which are target specific.

Pull request type

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

Reviewers

@TuomoHautamaki, @OPpuolitaival, @jamesbeyond

Release Notes

Use a BaseHostTestAbstract.reset() method instead of
BaseHostTestAbstract.reset_dut() for more consistent behavior with
various platforms in the CI.

In contrast to a local setup, the reset_dut() method flashes the binary
again after performing the reset in a remote setup (RaaS). This was not
intentional in reset_reason tests. Moreover, this led to a different
behavior (a different reset_reason) for different flashing methods which
are target specific.
@ciarmcom
Copy link
Member

@fkjagodzinski, thank you for your changes.
@jamesbeyond @OPpuolitaival @TuomoHautamaki @ARMmbed/mbed-os-maintainers please review.

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 25, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Oct 25, 2019

Test run: SUCCESS

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

Copy link
Contributor

@OPpuolitaival OPpuolitaival left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

@jamesbeyond jamesbeyond left a comment

Choose a reason for hiding this comment

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

LGTM

@0xc0170 0xc0170 merged commit 6f7c1c9 into ARMmbed:master Oct 28, 2019
@fkjagodzinski fkjagodzinski deleted the fix-reset_reason-ci_nightly branch October 28, 2019 12:41
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.

6 participants