Open oPromessa opened 2 weeks ago
@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.
tests/test_cli.py
fail 😢 on assert with where 1 = <Result KeyError('screen_recording')>.exit_code
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 MacTeXRan 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.
On docsrc/README.md is there a typo: cd docs should be cd docsrc ?
Yes, you're right. I'll fix it.
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.
Had to set property screen-recording to None for <= _PHOTOS_4_VERSION).
Is your feature request related to a problem? Please describe.
Describe the solution you'd like
Describe alternatives you've considered N/A
Additional context Errors: