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

Add a Unity macro to assert on platform error code difference #8646

Merged
merged 1 commit into from
Nov 16, 2018

Conversation

davidsaada
Copy link
Contributor

Description

Current platform error codes, defined in mbed_error.h header file, consist of the error code along with other information such as the module ID. This means that if we have a test that checks for the error code, an unexpected result would show a very large (negative) number, requiring user parsing in order to understand the real error.
This PR adds TEST_ASSERT_EQUAL_ERROR_CODE and TEST_ASSERT_EQUAL_ERROR_CODE_MESSAGE macros, checking only the status code (lower part) of the full error code, showing the interesting part of the error in case of a mismatch.

Pull request type

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

@0xc0170 0xc0170 requested a review from a team November 5, 2018 13:52
@deepikabhavnani
Copy link

@SenRamakri - Please review

Copy link

@deepikabhavnani deepikabhavnani 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 👍

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 8, 2018

@ARMmbed/mbed-os-test Please review

@NirSonnenschein
Copy link
Contributor

@ARMmbed/mbed-os-test reminder: Please review

@mikisch81
Copy link
Contributor

@ARMmbed/mbed-os-test another reminder: Please review

@cmonr
Copy link
Contributor

cmonr commented Nov 14, 2018

/morph build

*-------------------------------------------------------*/
// Use these to check whether error code equals what we expect.
// Only display error code (without other information)
#define TEST_ASSERT_EQUAL_ERROR_CODE(expected, actual) TEST_ASSERT_EQUAL(expected & MBED_ERROR_STATUS_CODE_MASK, actual & MBED_ERROR_STATUS_CODE_MASK)
Copy link
Contributor

@SenRamakri SenRamakri Nov 14, 2018

Choose a reason for hiding this comment

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

Hi David, Instead of using the error mask directly can you please use the macro MBED_GET_ERROR_CODE() macro. The reason is depending on error type the error code is encoded differently(this is due to support for Posix errors). Using the macro makes the code more portable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@cmonr
Copy link
Contributor

cmonr commented Nov 14, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 14, 2018

Build : FAILURE

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

@cmonr
Copy link
Contributor

cmonr commented Nov 14, 2018

@davidsaada Please take a look at the build failure. Looks like there's something wrong with the PR's implementation when it's compiled with IAR.

@davidsaada
Copy link
Contributor Author

davidsaada commented Nov 15, 2018

IAR build problem was bypassed by removing the include to "mbed_error.h". This should be OK, as the only change here was in a macro, which should be used by users of mbed errors anyway.
@SenRamakri The reason for this build problem lies within "mbed_error.h": This header file includes "mbed_retarget.h", which declares the fdopen function. Now, this conflicts with the one declared in the internal IAR libraries. Tried to fix it without success. I'm leaving this one to you as many PRs depend on this one and as the problem has nothing to do with the PR itself.
If you wish to reproduce this problem, you can add the include to "mbed_error.h" in "unity.h" and then run the command:

mbed test --compile -m ARCH_PRO -t IAR -DMBED_HEAP_STATS_ENABLED=1 -DMBED_STACK_STATS_ENABLED=1 -DMBED_TRAP_ERRORS_ENABLED=1 -DMBED_ALL_STATS_ENABLE -n FEATURES-STORAGE-NVSTORE-TESTS-NVSTORE-FUNCTIONALITY

@NirSonnenschein
Copy link
Contributor

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2018

Build : SUCCESS

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

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2018

@mbed-ci
Copy link

mbed-ci commented Nov 15, 2018

@cmonr
Copy link
Contributor

cmonr commented Nov 15, 2018

Sporadic NRF52 RTC test issue.

Will retest when able.

@cmonr
Copy link
Contributor

cmonr commented Nov 15, 2018

Info: This PR has now been bundled into a rollup PR (#8760).

The CI failures that this PR saw do not appear to be the fault of the PR. To save time in retesting this PR, along with other PRs that experienced similar issues, it has been bundled into a rollup PR.

If any more commits are made in this PR, this PR will remain open and have to go through CI on its own.

@cmonr
Copy link
Contributor

cmonr commented Nov 16, 2018

Info: This PR has been re-bundled into a new rollup PR (#8763).

The previous rollup found an issue with a bundled PR after new devices were added into master. The PR needing work has been removed.

No further work is needed here, as once that PR is merged, this PR will also be closed and marked as merged.
If any more commits are made in this PR, this PR will remain open and have to go through CI on its own.

@0xc0170 0xc0170 merged commit 3995779 into ARMmbed:master Nov 16, 2018
@0xc0170 0xc0170 removed the needs: CI label Nov 16, 2018
@davidsaada davidsaada deleted the david_unity_error_code branch December 6, 2018 13:04
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.

None yet

9 participants