Page MenuHomeSoftware Heritage

azure: Add tests based on Azurite in addition to mocks
ClosedPublic

Authored by vlorentz on Oct 24 2022, 2:45 PM.

Details

Summary

In addition to add support for multi-hashes (T2309), we will need to use more advanced
features of the Azure Blob Storage, so using only mock-based tests will be
less reliable.

This commit adds the option to run the test suite with Azurite, a clone
of the Azure Blob Storage API, which is a standard tool used in tests.

See https://learn.microsoft.com/en-us/azure/storage/common/storage-use-azurite

Test Plan
npm install azurite
AZURITE_PATH=$HOME/node_modules/.bin/ pytest -m azurite

Diff Detail

Repository
rDOBJS Object storage
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 32534
Build 50957: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 50956: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D8756 (id=31570)

Rebasing onto 3718273e1e...

Current branch diff-target is up to date.
Changes applied before test
commit 24b561ab048dba0cf972eb94240f21ccf04e7c10
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 24 14:44:03 2022 +0200

    azure: Add tests based on Azurite in addition to mocks
    
    In addition to add support for multi-hashes, we will need to use more advanced
    features of the Azure Blob Storage, so using only mock-based tests will be
    less reliable.
    
    This commit adds the option to run the test suite with Azurite, a clone
    of the Azure Blob Storage API, which is a standard tool used in tests.

Link to build: https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/176/
See console output for more information: https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/176/console

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 24 2022, 2:49 PM
Harbormaster failed remote builds in B32524: Diff 31570!

Build is green

Patch application report for D8756 (id=31580)

Rebasing onto 3718273e1e...

Current branch diff-target is up to date.
Changes applied before test
commit 796de423f064263d129d8617586e1a7ce65db51c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 24 14:44:03 2022 +0200

    azure: Add tests based on Azurite in addition to mocks
    
    In addition to add support for multi-hashes, we will need to use more advanced
    features of the Azure Blob Storage, so using only mock-based tests will be
    less reliable.
    
    This commit adds the option to run the test suite with Azurite, a clone
    of the Azure Blob Storage API, which is a standard tool used in tests.

See https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/177/ for more details.

olasd added a subscriber: olasd.

This seems like a great idea.

I would suggest using the pytest.mark.skipif pattern, with a condition on whether the azurite executable is found. Something like:

azurite_exe = shutil.which(
    "azurite", path=os.environ.get("AZURITE_PATH", os.environ.get("PATH"))
)

pytest.mark.skipif(not azurite_exe, reason="azurite not found in AZURITE_PATH or PATH")
class ...:
    pass

(seems to work fine on my system)

You need to add AZURITE_PATH to tox.ini's passenv.

Then we should just npm install azurite container-wide on the CI system so these tests actually run on jenkins. (Alternatively, we can add a package.json / yarn.lock locally to this package, which should make jenkins automagically install it)

This revision is now accepted and ready to land.Oct 25 2022, 1:45 PM

use AZURITE_PATH as a dir name, and switch to skipif

In D8756#227875, @olasd wrote:

Then we should just npm install azurite container-wide on the CI system so these tests actually run on jenkins. (Alternatively, we can add a package.json / yarn.lock locally to this package, which should make jenkins automagically install it)

I'll go with the former, it will save 10-30 seconds of install; plus global install means we don't need to set $AZURITE_PATH

Build is green

Patch application report for D8756 (id=31604)

Rebasing onto 3718273e1e...

Current branch diff-target is up to date.
Changes applied before test
commit ddda77d4c6163525dfacd8bcb158fba57a779411
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 24 14:44:03 2022 +0200

    azure: Add tests based on Azurite in addition to mocks
    
    In addition to add support for multi-hashes, we will need to use more advanced
    features of the Azure Blob Storage, so using only mock-based tests will be
    less reliable.
    
    This commit adds the option to run the test suite with Azurite, a clone
    of the Azure Blob Storage API, which is a standard tool used in tests.

See https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/178/ for more details.

swh/objstorage/tests/test_objstorage_azure.py
30–35

Ugh. Just set AZURITE_PATH to your node_modules/.bin and avoid this .js ugliness?

139

As this includes the container_name, is the separate argument needed for get_objstorage?

If azurite supports that, it may be nicer to get a shared access signature and pass it through to the existing connection_url argument (which would introduce coverage of the code that retrieves shared access signatures as well).

swh/objstorage/tests/test_objstorage_azure.py
139

s/connection_url/container_url/, of course

swh/objstorage/tests/test_objstorage_azure.py
30–35

oooh! I was looking for node_modules/bin, that's why I didn't find it

I'll use azurite-blob instead of azurite itself, so it doesn't needlessly spawn other the queue and table services.

139

It's needed by ContainerClient.from_connection_string for some reason

Use 'azurite-blob' instead of 'azurite'/'azurite.js'

pytest.ini
2–3

this should go away now

tox.ini
13

this should go away now

Build is green

Patch application report for D8756 (id=31606)

Rebasing onto 3718273e1e...

Current branch diff-target is up to date.
Changes applied before test
commit ea6687830098a73f88d0d20e671dd5b01e3a17ac
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 24 14:44:03 2022 +0200

    azure: Add tests based on Azurite in addition to mocks
    
    In addition to add support for multi-hashes, we will need to use more advanced
    features of the Azure Blob Storage, so using only mock-based tests will be
    less reliable.
    
    This commit adds the option to run the test suite with Azurite, a clone
    of the Azure Blob Storage API, which is a standard tool used in tests.

See https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/179/ for more details.

vlorentz marked an inline comment as done.

Build has FAILED

Patch application report for D8756 (id=31607)

Rebasing onto 3718273e1e...

Current branch diff-target is up to date.
Changes applied before test
commit 51edf12a8c83744bd316cfb30891d6df70b150d8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 24 14:44:03 2022 +0200

    azure: Add tests based on Azurite in addition to mocks
    
    In addition to add support for multi-hashes, we will need to use more advanced
    features of the Azure Blob Storage, so using only mock-based tests will be
    less reliable.
    
    This commit adds the option to run the test suite with Azurite, a clone
    of the Azure Blob Storage API, which is a standard tool used in tests.

Link to build: https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/180/
See console output for more information: https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/180/console

Build is green

Patch application report for D8756 (id=31607)

Rebasing onto 3718273e1e...

Current branch diff-target is up to date.
Changes applied before test
commit 51edf12a8c83744bd316cfb30891d6df70b150d8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 24 14:44:03 2022 +0200

    azure: Add tests based on Azurite in addition to mocks
    
    In addition to add support for multi-hashes, we will need to use more advanced
    features of the Azure Blob Storage, so using only mock-based tests will be
    less reliable.
    
    This commit adds the option to run the test suite with Azurite, a clone
    of the Azure Blob Storage API, which is a standard tool used in tests.

See https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/181/ for more details.

Build is green

Patch application report for D8756 (id=31607)

Rebasing onto 3718273e1e...

Current branch diff-target is up to date.
Changes applied before test
commit 51edf12a8c83744bd316cfb30891d6df70b150d8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 24 14:44:03 2022 +0200

    azure: Add tests based on Azurite in addition to mocks
    
    In addition to add support for multi-hashes, we will need to use more advanced
    features of the Azure Blob Storage, so using only mock-based tests will be
    less reliable.
    
    This commit adds the option to run the test suite with Azurite, a clone
    of the Azure Blob Storage API, which is a standard tool used in tests.

See https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/182/ for more details.

Build is green

Patch application report for D8756 (id=31612)

Rebasing onto 3718273e1e...

Current branch diff-target is up to date.
Changes applied before test
commit df4be2d87c300940e8a118371acc8796e8b7e715
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 24 14:44:03 2022 +0200

    azure: Add tests based on Azurite in addition to mocks
    
    In addition to add support for multi-hashes, we will need to use more advanced
    features of the Azure Blob Storage, so using only mock-based tests will be
    less reliable.
    
    This commit adds the option to run the test suite with Azurite, a clone
    of the Azure Blob Storage API, which is a standard tool used in tests.

See https://jenkins.softwareheritage.org/job/DOBJS/job/tests-on-diff/183/ for more details.