Re: [Libguestfs] [PATCH nbdkit] s3: Only run the tests if boto3 is installed

On 05/17/22 10:55, Richard W.M. Jones wrote: > ERROR: S3 (unittest.loader._FailedTest) > ---------------------------------------------------------------------- > ImportError: Failed to import test module: S3 > Traceback (most recent call last): > File "/usr/lib64/python3.10/unittest/loader.py", line 154, in loadTestsFromName > module = __import__(module_name) > File "/home/rjones/d/nbdkit/plugins/S3/S3.py", line 41, in <module> > import boto3 > ModuleNotFoundError: No module named 'boto3' > > Reported-by: Laszlo Ersek > Fixes: commit 5130c43bc1f9 > --- > tests/test-S3-unit.sh | 1 + > tests/test-S3.sh | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/tests/test-S3-unit.sh b/tests/test-S3-unit.sh > index 6b6adf02..8a32dd4c 100755 > --- a/tests/test-S3-unit.sh > +++ b/tests/test-S3-unit.sh > @@ -37,6 +37,7 @@ set -x > > requires $PYTHON --version > requires $PYTHON -c 'import unittest' > +requires $PYTHON -c 'import boto3' > > # Python has proven very difficult to valgrind, therefore it is disabled. > if [ "$NBDKIT_VALGRIND" = "1" ]; then > diff --git a/tests/test-S3.sh b/tests/test-S3.sh > index de94751e..e51ef2ac 100755 > --- a/tests/test-S3.sh > +++ b/tests/test-S3.sh > @@ -36,6 +36,7 @@ set -x > > requires hexdump --version > requires $PYTHON --version > +requires $PYTHON -c 'import boto3' > requires nbdcopy --version > requires_plugin python > > This was my very first idea too, but does it not conflict with having a fake boto3 module too, at "tests/test-S3/boto3"?

In particular, "tests/test-S3.sh" already contains: # There is a fake boto3 module in test-S3/ which we use as a test # harness for the plugin. requires test -d test-S3 export PYTHONPATH=$srcdir/test-S3:$PYTHONPATH In other words, if you added the new line requires $PYTHON -c 'import boto3' to the same file, but under the PYTHONPATH extension, then I *think* the new "requires" would succeed, but the test case would fail the same way -- because the fake module does not expose the new exception type. Of course I'm also 100% fine if we make both test cases dependent on the *real* boto3 module, but should we then eliminate the "test-S3" directory? (The fake module, that is?)