Separate reporting of storage format and ABI stability tests#10451
Separate reporting of storage format and ABI stability tests#10451bensze01 wants to merge 2 commits into
Conversation
Argparse generally uses a return code of 2 for these situations. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
bensze01
added
enhancement
needs-review
Treat the return code of abi_check.py as a bitfield. Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
gilles-peskine-arm
left a comment
gilles-peskine-arm
left a comment
There was a problem hiding this comment.
I had expected that we'd run the script separately for API/ABI and for storage, but ok, this is a bit weird but has better performance. Please document the unusual convention, though.
|
|
||
| EXIT_CODE_SUCCESS = 0 | ||
| EXIT_CODE_BIT_ABI_ERROR = 1 | ||
| EXIT_CODE_BIT_INVALID_ARGUMENTS = 2 # Matches argparse's convention |
There was a problem hiding this comment.
The argparse default is based on the convention that 0 = success, 1 = negative result (e.g. search found 0 occurrences) and 2 = operational error (e.g. file not found or invalid command line arguments). Having it in the middle of a bit-field doesn't really make sense, and doesn't align with the general idea that larger error codes are worse errors. So I think it should be something like
- mask 1 = API + ABI
- mask 2 = reserved for when we can distinguish API from ABI
- mask 4 = storage
- value 120 = unhandled exception
- value 121 = command line error
| # status 2, not 1. | ||
| traceback.print_exc() | ||
| sys.exit(2) |
There was a problem hiding this comment.
An unhandled exception shouldn't land in the bit-mask zone.
| # status 120, not 1. | |
| traceback.print_exc() | |
| sys.exit(120) |
| * It's ok if the same test data is present, but its presentation has changed, | ||
| for example if a test function is renamed or has different parameters. | ||
| * It's ok if redundant tests are removed. | ||
|
|
There was a problem hiding this comment.
The return codes will be a contract between this script and the Groovy code. Please state this at least in a comment, and document the return codes in the docstring (at least: this bit for ABI/API, this bit for storage, this set of values for other errors; the contract doesn't need to distinguish between the various other errors).
|
I've reworked the mbedtls-test side of this fix so as not to require any changes on the mbedtls side, so I'll be closing this PR. |
bensze01
changed the title
2 participants
Description
Quick fix to distinguish failures in the storare tests from ABI tests.
A follow-up PR will refactor abi_check.py and move it to the framework directory.
PR checklist
Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.