Page MenuHomeSoftware Heritage
Differential D8756

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

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

Details

Reviewers
olasd
Group Reviewers
Reviewers
Maniphest Tasks
T2309: Add support for other hash algo than sha1 in current objstorage implementation
Commits
rDOBJSdf4be2d87c30: azure: Add tests based on Azurite in addition to mocks
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

vlorentz created this revision.Oct 24 2022, 2:45 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptOct 24 2022, 2:45 PM
vlorentz added a task: T2309: Add support for other hash algo than sha1 in current objstorage implementation.Oct 24 2022, 2:45 PM
swh-public-ci added a comment.Oct 24 2022, 2:49 PM

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!
vlorentz updated this revision to Diff 31580.Oct 24 2022, 6:24 PM

fix typo in tox.ini

swh-public-ci added a comment.Oct 24 2022, 6:27 PM

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.

Harbormaster completed remote builds in B32534: Diff 31580.Oct 24 2022, 6:27 PM
vlorentz requested review of this revision.Oct 24 2022, 6:27 PM
olasd accepted this revision.Oct 25 2022, 1:45 PM
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
vlorentz updated this revision to Diff 31604.Oct 25 2022, 2:13 PM

use AZURITE_PATH as a dir name, and switch to skipif

vlorentz edited the test plan for this revision. (Show Details)Oct 25 2022, 2:13 PM
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

swh-public-ci added a comment.Oct 25 2022, 2:16 PM

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.

Harbormaster completed remote builds in B32558: Diff 31604.Oct 25 2022, 2:16 PM
vlorentz mentioned this in D8765: Install Azurite in base docker image.Oct 25 2022, 2:31 PM
olasd added inline comments.Oct 25 2022, 2:32 PM
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).

olasd added inline comments.Oct 25 2022, 2:40 PM
swh/objstorage/tests/test_objstorage_azure.py
139

s/connection_url/container_url/, of course

vlorentz added inline comments.Oct 25 2022, 2:41 PM
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

vlorentz updated this revision to Diff 31606.Oct 25 2022, 2:41 PM

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

olasd added inline comments.Oct 25 2022, 2:43 PM
pytest.ini
2–3

this should go away now

tox.ini
13

this should go away now

swh-public-ci added a comment.Oct 25 2022, 2:44 PM

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 updated this revision to Diff 31607.Oct 25 2022, 2:44 PM

Begone, markers

Harbormaster completed remote builds in B32560: Diff 31606.Oct 25 2022, 2:44 PM
vlorentz marked 2 inline comments as done.Oct 25 2022, 2:45 PM
vlorentz marked an inline comment as done.
swh-public-ci added a comment.Oct 25 2022, 2:46 PM

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

Harbormaster failed remote builds in B32561: Diff 31607!Oct 25 2022, 2:46 PM
swh-public-ci added a comment.Oct 25 2022, 2:51 PM

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.

Harbormaster completed remote builds in B32561: Diff 31607.Oct 25 2022, 2:51 PM
swh-public-ci added a comment.Oct 25 2022, 3:01 PM

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.

vlorentz updated this revision to Diff 31612.Oct 25 2022, 3:05 PM

remove leftover marker

swh-public-ci added a comment.Oct 25 2022, 3:09 PM

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.

Harbormaster completed remote builds in B32565: Diff 31612.Oct 25 2022, 3:09 PM
Closed by commit rDOBJSdf4be2d87c30: azure: Add tests based on Azurite in addition to mocks (authored by vlorentz). · Explain WhyOct 25 2022, 3:12 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDOBJSdf4be2d87c30: azure: Add tests based on Azurite in addition to mocks.
vlorentz edited the test plan for this revision. (Show Details)Oct 25 2022, 3:13 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
2 lines--
swh/
objstorage/
backends/
37 lines95%
Loading...
tests/
95 lines88%
Loading...
3 lines--

Diff 31580

View Options

pytest.ini

Loading...
View Options

swh/objstorage/backends/azure.py

Loading...
View Options

swh/objstorage/tests/test_objstorage_azure.py

Loading...
View Options

tox.ini

Loading...
About · Developer information · Legal