[Libguestfs] S3 plugin test case breaks test suite / development workflow...

[Laszlo Ersek]

Hi,

I'm writing this about a specific problem and about a general problem.

* The specific problem is that commit 5130c43bc1f9 ("S3 plugin: add
support for accessing multiple objects", 2022-05-12) introduced a
dependency on the "botocore" python module, and now "make check" fails
for me, because this module is unavailable on my system.

> Traceback (most recent call last):
>   File "[...]/nbdkit/plugins/S3/nbdkit-S3-plugin", line 43, in <module>
>     from botocore.exceptions import ClientError
> ModuleNotFoundError: No module named 'botocore'

Now, while the README file says, from commit 6715c3d8b3e6 ("New plugin:
S3 plugin for accessing disks stored in AWS S3 and Ceph.", 2020-11-14):

> For the Python plugin:
>
>  - python interpreter (version 3 only)
>
>  - python development libraries
>
>  - python unittest to run the test suite
>
>  - boto3 is required to run the S3 plugin written in Python

the "boto3" dependency had never been a "hard" one until commit
5130c43bc1f9. The language suggests that running the S3 plugin -- even
for testing -- is optional.

Therefore, commit 5130c43bc1f9 should have updated the test cases
"test-S3.sh" and "test-S3-unit.sh" with a proper "requires" line (I
assume anyway).

FWIW, the following trick in "test-S3.sh":

> # There is a fake boto3 module in test-S3/ which we use as a test
> # harness for the plugin.

Does not work.

... So, after all, the bug in commit 5130c43bc1f9 may have been that it
did not add the ClientError exception type to the fake module in
"tests/test-S3/boto3". I'm unsure; but please fix it anyway.


* The generic problem is that I need to write this separate error report
email, rather than commenting directly under the submission -- on the
mailing list -- or the pull request -- on gitlab -- that ended up as
commit 5130c43bc1f9. For the life of me, I just can't figure out *where*
commit 5130c43bc1f9 was originally reviewed.

I think that's wrong.

... Ultimately, I've found the patch in the MR listing at
<https://gitlab.com/nbdkit/nbdkit/-/merge_requests?scope=all&state=all>,
namely in MR#9 -- <https://gitlab.com/nbdkit/nbdkit/-/merge_requests/9>.

But this merge request has status *closed*, not *merged*. So even though
a commit is given, I can't find the associated discussion because (a)
MR#9 is not listed in the list of *merged* merge requests (only the
"rejected" ones), (b) the commit message itself does not reference MR#9.

I think we should improve our process here.

Thanks,
Laszlo