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

RTOS: Mutex: Rework tests #4729

Merged
merged 2 commits into from Jul 27, 2017

Conversation

Projects
None yet
5 participants
@bulislaw
Member

bulislaw commented Jul 10, 2017

Add descriptions, fix small issues and timings

Status

READY

@bulislaw

This comment has been minimized.

Member

bulislaw commented Jul 10, 2017

/morph test

@bulislaw bulislaw force-pushed the bulislaw:mutex_tests branch 2 times, most recently Jul 10, 2017

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 10, 2017

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 750

Test Prep failed!

@bulislaw

This comment has been minimized.

Member

bulislaw commented Jul 10, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 10, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 752

All builds and test passed!

@pan-

Access to the variables change_counter, changing_counter and mutex_defect shall be synchronized (critical section seems a good candidate).

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 10, 2017

@bulislaw Could you address @pan- comment?

@bulislaw

This comment has been minimized.

Member

bulislaw commented Jul 11, 2017

They sort of are, it's an old test, that probably could be removed, but the variables you mentioned are manipulated inside the mutex protected zone and only if the mutex wouldn't work properly something 'bad' would happen.

@bulislaw bulislaw force-pushed the bulislaw:mutex_tests branch Jul 11, 2017

@pan-

This comment has been minimized.

Member

pan- commented Jul 11, 2017

@bulislaw Those tests target mutexes behaviors. It shouldn't be assumed that mutexes work otherwise, what is the point of testing them ?

bulislaw added some commits Jul 10, 2017

RTOS: Mutex: Rework tests
Add descriptions, fix small issues and timings.

@bulislaw bulislaw force-pushed the bulislaw:mutex_tests branch to 0935683 Jul 12, 2017

@bulislaw

This comment has been minimized.

Member

bulislaw commented Jul 17, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 17, 2017

LGTM

@pan- What do you think?

@pan-

pan- approved these changes Jul 19, 2017

Few ideas for new tests:

  • Test a continuous manipulation of a variable shared by many threads; without delay.

  • Add test of the thread state when a thread is blocked waiting on a mutex.

if (changing_counter == true) {
result = false;
mutex_defect = true;
}
changing_counter = true;
change_counter++;
core_util_critical_section_exit();

This comment has been minimized.

@pan-

pan- Jul 19, 2017

Member

Now, I'm thinking about, change_counter Is the variable under test. It is maybe not the best to protect it with a critical section like other flags.

@0xc0170 0xc0170 removed the needs: review label Jul 24, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 24, 2017

/morph test

@0xc0170 0xc0170 added the needs: CI label Jul 24, 2017

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 25, 2017

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 874

Build failed!

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 26, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 26, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 893

All builds and test passed!

@theotherjimmy theotherjimmy merged commit c5c470e into ARMmbed:master Jul 27, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment