On Tue, May 17, 2022 at 11:53:44AM +0200, Laszlo Ersek wrote:

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"?

Yeah I guess the real problem is that the mocked boto3 isn't working for some reason. FWIW this works (with mocked boto3) in nbdkit 1.30.

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?)

So I think we shouldn't ever be using the real boto3 module, because that might mean we try to connect to an S3 instance. Let's ditch this patch and I'll have another look. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

On 05/17/22 12:11, Richard W.M. Jones wrote:

OK I see what's going on.

test-S3.sh uses the mocked boto3 in tests/test-S3/ to do an end-to-end test (nbdcopy).

test-S3-unit.sh runs the unit tests within the plugin. It's basically testing the plugin as if it was a standalone Python script (without nbdkit being involved). This uses an internal MockS3Client class also inside the plugin.

The second one does actually need real boto3, just because there's an "import boto3" at the top and we don't set up PYTHONPATH to pick up the tests/test-S3/ subdirectory.

We could either fix the second one by skipping it if (real) boto3 isn't installed, or by making it use the mocked boto3 from tests/test-S3/

Either approach would work, but the second one (below) seems maybe better because we'd get more test coverage this way?

I think so, yes: from your description, "test-S3-unit.sh" seems "less demanding" than "test-S3.sh", so if the mocked module satisfies the latter, it should be good enough for the former too.

Rich.

diff --git a/tests/test-S3-unit.sh b/tests/test-S3-unit.sh index 6b6adf02..11718be3 100755 --- a/tests/test-S3-unit.sh +++ b/tests/test-S3-unit.sh @@ -44,6 +44,6 @@ if [ "$NBDKIT_VALGRIND" = "1" ]; then exit 77 fi

-export PYTHONPATH=$srcdir/../plugins/S3:$PYTHONPATH +export PYTHONPATH=$srcdir/../plugins/S3:$srcdir/test-S3:$PYTHONPATH

$PYTHON -m unittest S3

For this hunk above: Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo