Skip to content

Enhance test runner scripts#11100

Closed
mspncp wants to merge 3 commits intoopenssl:masterfrom
mspncp:pr-enhance-test-runner-scripts
Closed

Enhance test runner scripts#11100
mspncp wants to merge 3 commits intoopenssl:masterfrom
mspncp:pr-enhance-test-runner-scripts

Conversation

@mspncp
Copy link
Copy Markdown
Contributor

@mspncp mspncp commented Feb 15, 2020

Two commits:

shlib_wrap.sh: add module and engine paths to environment if unset

The testsuite invokes many openssl commands using the shlib_wrap.sh script.
It used to be possible to copy those commands for testing them standalone
or run them in the debugger (see also description of issue #10628).
With providers this did not work anymore, because the script did not
set the OPENSSL_MODULES and OPENSSL_ENGINES environment variables,
they were already set by the _tests make target.

To make shlib_wrap.sh self-contained again, we set the OPENSSL_MODULES
and OPENSSL_ENGINES environment variables if they are unset. In fact,
that's just what opensslwrap.sh already does.

opensslwrap.sh: make implementation more consistent with shlib_wrap.sh

The testsuite invokes many openssl commands using the shlib_wrap.sh script.
It used to be possible to copy those commands for testing them standalone
or run them in the debugger (see also description of issue openssl#10628).
With providers this did not work anymore, because the script did not
set the `OPENSSL_MODULES` and `OPENSSL_ENGINES` environment variables,
they were already set by the `_tests` make target.

To make `shlib_wrap.sh` self-contained again, we set the `OPENSSL_MODULES`
and `OPENSSL_ENGINES` environment variables if they are unset. In fact,
that's just what `opensslwrap.sh` already does.

if [ -d "${HERE}../engines" -a "x$OPENSSL_ENGINES" = "x" ]; then
OPENSSL_ENGINES="${HERE}../engines"; export OPENSSL_ENGINES
if [ -z "$OPENSSL_ENGINES" ] && [ -d "${HERE}/engines" ] ; then
Copy link
Copy Markdown
Member

@levitte levitte Feb 15, 2020

Choose a reason for hiding this comment

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

What's wrong with -a? Is there a reason you want to create two processes when one can be enough?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Isn't it builtin anyway? The reason I changed it can be found in the TEST(1) manpage:

   NOTE:  Binary  -a  and -o are inherently ambiguous.  Use 'test EXPR1 &&
   test EXPR2' or 'test EXPR1 || test EXPR2' instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IIRC, the -a evaluates both expressions, no?

fi
if [ -z "$OPENSSL_MODULES" ] && [ -d "${THERE}/providers" ] ; then
OPENSSL_MODULES="${THERE}/providers"; export OPENSSL_MODULES
fi
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would rather not at this level. What if I want to test a newly built test program against the default config file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... too bad. It was such a convenient way to plug in the debugger into a test command. Do you have a better suggestion how to fix it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The variables could be set explicitly on the command line instead of implicitly by the _test target, but that is not really a satisfying solution either.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see much inconvenience with this

OPENSSL_MODULES=providers ./util/shlib_wrap.sh gdb test/evp_extra_test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Often it's more like

OPENSSL_MODULES=../../providers ../../util/shlib_wrap.sh gdb ../../apps/openssl  ...

I don't see much inconvenience with this

You mean inconvenience to add it manually, or inconvenience to add it to the test output?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

...sets OPENSSL_MODULES and OPENSSL_ENGINES automatically...

...if unset...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I view plug-ins (which our providers and engines are) as something different than shared libraries, even if the technology is the same. The difference is build-time linking vs run-time linking.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have an alternative idea that you might suitable, if you can have a little bit of patience? PR coming up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure. Surprise me... ;-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Have a look at #11110, I believe it fulfils what you want.

@mspncp
Copy link
Copy Markdown
Contributor Author

mspncp commented Feb 18, 2020

Closing in favour of #11110.

@mspncp mspncp closed this Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants