Add new option to enable unit testing for replace default mode #331
Conversation
…tly initializing the openssl default provider
scripts/utils-openssl.sh
Outdated
| fi | ||
|
|
||
| # Patch libcrypto.num for replace-default-testing mode | ||
| if [ "$WOLFPROV_REPLACE_DEFAULT_TESTING" = "1" ]; then |
There was a problem hiding this comment.
Can we also require "$WOLFPROV_REPLACE_DEFAULT" = "1" here?
| --replace-default) | ||
| WOLFPROV_REPLACE_DEFAULT=1 | ||
| ;; | ||
| --enable-replace-default-testing) |
There was a problem hiding this comment.
I think we should error out if --replace-default is not also set in the cmd line options. Alternatively, force it on whenever this new arg is set.
There was a problem hiding this comment.
Now errors if replace default not also set
| WOLFPROV_REPLACE_DEFAULT=1 | ||
| ;; | ||
| --enable-replace-default-testing) | ||
| WOLFPROV_REPLACE_DEFAULT_TESTING=1 |
There was a problem hiding this comment.
In the C code this is called WP_ENABLE_REPLACE_DEFAULT_UNIT_TEST. In the scripts, we have WOLFPROV_REPLACE_DEFAULT_TESTING. Can we make the names more closely aligned?
Fwiw: there are no other WP_ENABLE_* defines...
There was a problem hiding this comment.
Name updated to be consistent
| # Final warning for replace-default-testing builds | ||
| if [ "$WOLFPROV_REPLACE_DEFAULT_TESTING" = "1" ]; then | ||
| printf "\n" | ||
| printf "╔══════════════════════════════════════════════════════════════════════════╗\n" |
There was a problem hiding this comment.
Consider deleting/cleaning the outputs at this point to ensure this version doesn't remain beyond this point. I believe the sole purpose of this build is to run the unit tests.
|
Can we add the new mode to a CI workflow? Maybe |
|
Do we have customers wanting to do this? |
Well since I came up with replace-default, every single prospective customer has wanted the replace default option. Without this PR, we have no way to unit test wolfProvider against an openssl built with replace default. We are still in early stages with many of them, but they are already starting to ask about testing/verification. I thought this was a decent way to keep our original testing strategy against the openssl that most closely resembles what they will run in production. |
|
Document this then in the README.md then. |
3 participants
Add new option to enable unit testing for replace default mode, directly initializing the openssl default provider