Skip to content

Conversation

@pjbull
Copy link
Member

@pjbull pjbull commented Nov 30, 2024

Live tests for #483

* feat: added md5_hash property in GSClient._get_metadata, added md5 property inside GSPath

* docs: update supported methods and properties table in root README

* chore: running make format

* feat: updating code since linter wouldnt let me do it all in one line like the etag property

* chore: rename variable to match the variable naming in GSPath.stat()

* feat: update LocalGSPath to mimic local/implementations/azure.py for md5 property

* feat: updating MockBlob to include md5_hash property

* docs: updated history.md

* chore: reverting to pre PEP604 syntax

* test: added md5 test case, update MockBlob to take its value from an environment variable

* chore: applying format and lint
@github-actions
Copy link
Contributor

github-actions bot commented Nov 30, 2024

@github-actions github-actions bot temporarily deployed to pull request November 30, 2024 00:13 Inactive
@codecov
Copy link

codecov bot commented Nov 30, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.8%. Comparing base (0c8d0c4) to head (0d799d2).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
cloudpathlib/gs/gspath.py 85.7% 1 Missing ⚠️
cloudpathlib/local/implementations/gs.py 66.6% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #490     +/-   ##
========================================
- Coverage    94.0%   93.8%   -0.2%     
========================================
  Files          23      23             
  Lines        1788    1797      +9     
========================================
+ Hits         1682    1687      +5     
- Misses        106     110      +4     
Files with missing lines Coverage Δ
cloudpathlib/gs/gsclient.py 91.4% <ø> (ø)
cloudpathlib/gs/gspath.py 92.4% <85.7%> (-1.2%) ⬇️
cloudpathlib/local/implementations/gs.py 97.1% <66.6%> (-2.9%) ⬇️

... and 1 file with indirect coverage changes

@pjbull
Copy link
Member Author

pjbull commented Jan 29, 2025

@fafnirZ The hashes do not match in the live tests. Do you know why that would be?

=================================== FAILURES ===================================
_______ test_md5_property[hello world-5eb63bbbe01eeed093cb22bb8f5acdc3] ________
[gw1] linux -- Python 3.11.10 /home/runner/.venv/bin/python

content = 'hello world', expected_hash = '5eb63bbbe01eeed093cb22bb8f5acdc3'
gs_rig = <tests.conftest.CloudProviderTestRig object at 0x7f4eacb26e10>
monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7f4ead623e50>

    @pytest.mark.parametrize(
        "content, expected_hash",
        [
            ("hello world", "5eb63bbbe01eeed093cb22bb8f5acdc3"),
            ("another test case", "4f8182cd9856777ebe3c4f5dc58dacea"),
        ],
    )
    def test_md5_property(content, expected_hash, gs_rig, monkeypatch):
        # if USE_LIVE_CLOUD this doesnt have any effect
        monkeypatch.setenv("MOCK_EXPECTED_MD5_HASH", expected_hash)
    
        p: GSPath = gs_rig.create_cloud_path("dir_0/file0_0.txt")
        p.write_text(content)
>       assert p.md5 == expected_hash
E       AssertionError: assert 'XrY7u+Ae7tCTyyK7j1rNww==' == '5eb63bbbe01eeed093cb22bb8f5acdc3'
E         
E         - 5eb63bbbe01eeed093cb22bb8f5acdc3
E         + XrY7u+Ae7tCTyyK7j1rNww==

tests/test_gs_specific.py:67: AssertionError
---------------------------- Captured stdout setup -----------------------------
Test directory name is: RkUnPnTDZxRHxQeE72hydb-test_gs_specific-test_md5_property
____ test_md5_property[another test case-4f8182cd9856777ebe3c4f5dc58dacea] _____
[gw1] linux -- Python 3.11.10 /home/runner/.venv/bin/python

content = 'another test case'
expected_hash = '4f8182cd9856777ebe3c4f5dc58dacea'
gs_rig = <tests.conftest.CloudProviderTestRig object at 0x7f4ead3eca10>
monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7f4eace25650>

    @pytest.mark.parametrize(
        "content, expected_hash",
        [
            ("hello world", "5eb63bbbe01eeed093cb22bb8f5acdc3"),
            ("another test case", "4f8182cd9856777ebe3c4f5dc58dacea"),
        ],
    )
    def test_md5_property(content, expected_hash, gs_rig, monkeypatch):
        # if USE_LIVE_CLOUD this doesnt have any effect
        monkeypatch.setenv("MOCK_EXPECTED_MD5_HASH", expected_hash)
    
        p: GSPath = gs_rig.create_cloud_path("dir_0/file0_0.txt")
        p.write_text(content)
>       assert p.md5 == expected_hash
E       AssertionError: assert 'T4GCzZhWd36+PE9dxY2s6g==' == '4f8182cd9856777ebe3c4f5dc58dacea'
E         
E         - 4f8182cd9856777ebe3c4f5dc58dacea
E         + T4GCzZhWd36+PE9dxY2s6g==

tests/test_gs_specific.py:67: AssertionError

@fafnirZ
Copy link
Contributor

fafnirZ commented Feb 11, 2025

Hi @pjbull apologies for this, I have just created a fix for this here: 3a40dae

How would you like this change incorporated? do I submit a new PR rebased off this branch onto master?

@github-actions github-actions bot temporarily deployed to pull request February 11, 2025 20:49 Inactive
@pjbull
Copy link
Member Author

pjbull commented Feb 11, 2025

Thanks @fafnirZ, I just grabbed the change directly. Will merge if tests pass

@pjbull pjbull merged commit c0d20fa into master Feb 11, 2025
28 checks passed
@pjbull pjbull deleted the 483-local-tests branch February 11, 2025 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants