Skip to content

Test set for critical section #5201

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 24, 2018
Merged

Test set for critical section #5201

merged 1 commit into from
Jan 24, 2018

Conversation

maciejbocianski
Copy link
Contributor

@maciejbocianski maciejbocianski commented Sep 26, 2017 •
edited
Loading

Description

New test suite for critical section

Status

READY

Blocked by

#5346

Related PRs

#5346 - Add Critical Section HAL API specification

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 27, 2017

cc @pan- @bulislaw @c1728p9

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 29, 2017

bump for review prior the upcoming weekend :)

@bulislaw
Copy link
Member

It's blocked and may spark creation of new critical section HAL API. I'm guessing we should hold on till #5198 is resolved.

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 4, 2017

@maciejbocianski we can move this along to remove from this patch the lines that are causing the behavior we do not have covered.
I believe this is definitely improvement (adding new test unit). We will expand it once the issue is resolved. What do you think?

@maciejbocianski
Copy link
Contributor Author

maciejbocianski commented Oct 5, 2017 •
edited
Loading

@0xc0170 to do that we should comment out all TEST_ASSERT_FALSE(core_util_are_interrupts_enabled());
@bulislaw what do you think

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 5, 2017

@maciejbocianski I would stash it somewhere on the branch, not leaving deadcode in the codebase (we got version control for that :) ) , just just add a commit removing them and we revert it once we have API specified

@bulislaw What do you think about the above proposal?

@bulislaw
Copy link
Member

@scartmell-arm is working on critical section HAL, lets hold on with the tests till it's ready.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 16, 2017

This might get trackion once #5346 gets integrated

@maciejbocianski
Copy link
Contributor Author

@0xc0170 and this also #5420

@maciejbocianski maciejbocianski force-pushed the critical_section_tests branch 2 times, most recently from 5c8383e to 1c14636 Compare December 21, 2017 09:21
@maciejbocianski maciejbocianski force-pushed the critical_section_tests branch 2 times, most recently from 0480892 to 710e6ca Compare January 17, 2018 07:22
@0xc0170 0xc0170 requested review from bulislaw and a user January 17, 2018 10:18
@bulislaw bulislaw requested a review from jamesbeyond January 19, 2018 12:05
jamesbeyond
jamesbeyond reviewed Jan 22, 2018
View reviewed changes
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.

Didn't the tests require a header file comes with it?

@maciejbocianski
Copy link
Contributor Author

I think it's required for HAL tests only

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2018

I think it's required for HAL tests only

It might be good to have it as a requirement for new tests? Shall be discussed apart from this PR, but would be nice to have alignment for tests documentation.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2018

/morph build

jamesbeyond
jamesbeyond approved these changes Jan 23, 2018
View reviewed changes
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.

Looks good to me, if the platform API test does not require a header file.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 23, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Jan 23, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Jan 23, 2018

Test : SUCCESS

Build number : 758
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5201/758

@mbed-ci
Copy link

mbed-ci commented Jan 23, 2018

Exporter Build : FAILURE

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

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 23, 2018

/morph export-build

@mbed-ci
Copy link

mbed-ci commented Jan 23, 2018

Exporter Build : SUCCESS

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

@cmonr
Copy link
Contributor

cmonr commented Jan 23, 2018

/morph uvisor-test

@cmonr cmonr merged commit 98611c8 into ARMmbed:master Jan 24, 2018
@fahimalavi fahimalavi mentioned this pull request Feb 1, 2018
Merged
@RobMeades RobMeades mentioned this pull request Feb 1, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@jamesbeyond jamesbeyond jamesbeyond approved these changes

@bulislaw bulislaw Awaiting requested review from bulislaw

Assignees
No one assigned
Labels
release-version: 5.8.0-rc1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@maciejbocianski @0xc0170 @bulislaw @mbed-ci @cmonr @jamesbeyond