Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for screen-recording media type #1585

Open
oPromessa opened this issue Jun 17, 2024 · 6 comments
Open

Support for screen-recording media type #1585

oPromessa opened this issue Jun 17, 2024 · 6 comments
Labels
feature request New feature or request

Comments

@oPromessa
Copy link
Contributor

oPromessa commented Jun 17, 2024

Is your feature request related to a problem? Please describe.

  • Support for screen-recording media type.
  • Found that Photos also classified Videos screen recordings recorded on iPhone (and possibly Mac).
  • On the Database it shows as subtype of 103 on the database.

Describe the solution you'd like

  • Tried to make the code changes.. but I'm stuck on a few tests.
  • It's also missing documentation update.
  • @RhetTbull: If you give me a few pointers I'll try to fix it prior to submitting a change request.

Describe alternatives you've considered
N/A

Additional context
Errors:

(...)
_________________________________________________________________________________ test_iphoto_info[photo_dict12] 

iphotodb = osxphotos.iPhotoDB(dbfile='/Users/MSP/Documents/GitHub/osxphotos/tests/Test-iPhoto-9.6.1.photolibrary/Database/apdb/Library.apdb')
photo_dict = {'adjustments': {}, 'album_info': [{'folder_list': ['dmCBXs4sSXaZ8nEJV+AM%A', 'NtsvmxOMR6WM3nYJy5X3ow'], 'folder_names...er_names': [], 'parent': None, 'sort_order': 0, ...}], 'albums': ['AlbumInFolder', 'Last Import'], 'burst': False, ...}

    @pytest.mark.usefixtures("set_tz_pacific")
    @pytest.mark.parametrize("photo_dict", photo_info())
    def test_iphoto_info(iphotodb: iPhotoDB, photo_dict: dict[str, Any]):
        """Test iPhotoPhotoInfo properties"""
    
        # rather than test each property individually, just compare json output
        # to the test data
        # when called with shallow=False, the json output will contain the output
        # of iPhotoAlbumInfo, iPhotoPersonInfo, etc. so each class will be tested
    
        uuid = photo_dict["uuid"]
        photo = iphotodb.get_photo(uuid)
        for key, value in json.loads(photo.json(shallow=False)).items():
            if key != "fingerprint":
                # fingerprint not implemented on linux
>               assert value == photo_dict[key]
E               KeyError: 'screen_recording'

tests/test_iphoto.py:194: KeyError
==================================================================================== short test summary info
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict0] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict1] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict2] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict3] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict4] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict5] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict6] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict7] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict8] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict9] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict10] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict11] - KeyError: 'screen_recording'
FAILED tests/test_iphoto.py::test_iphoto_info[photo_dict12] - KeyError: 'screen_recording'
============================================================== 13 failed, 2003 passed, 153 skipped, 4 xfailed in 286.63s (0:04:46) ===
@oPromessa oPromessa added the feature request New feature or request label Jun 17, 2024
@RhetTbull
Copy link
Owner

@oPromessa I'm impressed with how thorough your branch is -- nice work! A couple pointers:

The "screen_shot" key should be added inasdict() for photo PhotoInfo and the iPhoto PhotoInfo and it must be added in the if not shallow: block. This prevents new keys from messing up the comparison in osxphotos export to the stored JSON data for a photo. Only keys in the shallow=True dictionary are compared when exporting.

Additionally, in the iPhoto test, you'll need to update the test data at tests/iphoto_test_data.json to add the new key so the tests works.

@oPromessa
Copy link
Contributor Author

oPromessa commented Jun 18, 2024

  • Yeah! Basically RTFM 😉 Test Data -- BTW I've edited it a bit.
  • All tests pass now! YAY 💯
  • Ran build.sh... some warnings related with latex and pdf (guess I don't have the tools). stderr: build.txt
  • On docsrc/README.md is there a typo: cd docs should be cd docsrc ? Did not want to change it, in case I was wrong. Also build.sh generates the docs. PS: Did not install MacTeX

@RhetTbull
Copy link
Owner

Ran build.sh... some warnings related with latex and pdf (guess I don't have the tools)

The docs building needs work. Don't worry about this as I don't build the docs until I do the release. For individual PRs, just update the appropriate files but don't build the docs as they will get re-built as part of my release process.

@RhetTbull
Copy link
Owner

On docsrc/README.md is there a typo: cd docs should be cd docsrc ?

Yes, you're right. I'll fix it.

@RhetTbull
Copy link
Owner

I think I mostly got it. But have it but still question what to do in these code lines (now commented. 'cause if I uncomment, tests/test_cli.py fail 😢 on assert with where 1 = <Result KeyError('screen_recording')>.exit_code

The test_cli.py is a monster....on my to do list to break this up into smaller chunks as it has become very unwieldy.

Basically, for the tests that are failing, you need to track down where they're getting the "expected" data from. For example, in test_query_uuid(), it's reading CLI_OUTPUT_QUERY_UUID which is defined elsewhere in the file. This is a JSON string that will be reconstituted and compared to the JSON output of the query command. This JSON string will need to be modified to contain the screen_recording key.

There may be other places that need to be modified.

@oPromessa
Copy link
Contributor Author

Had to set property screen-recording to None for <= _PHOTOS_4_VERSION).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants