ChargePoint / xcparse

Command line tool & Swift framework for parsing Xcode 11+ xcresult
MIT License
390 stars 47 forks source link

Use attachment timestamp as modification date of exported attachment file #82

Open Boerworz opened 1 year ago

Boerworz commented 1 year ago

Change Description Hi!

First of all, thank you for building this library! It's been very useful for us in making screenshots available on Bitrise for our failing UI tests.

However, as you are aware, a lot of people (including us) would find it convenient to be able to view the exported screenshots in the order they were taken in the tests. I saw that there have been some discussions about this previously (#38, #42), but as far as I could tell nothing concrete has come off it, so I decided to take a stab at it!

Instead of solving the problem by changing the exported filename—which requires a lot of considerations as seen in #38—I opted to simply set the modification date of the exported file. Prior to this change the modification date would be the date of when xcparse wrote the exported file to disk. Setting the modification date instead to the attachment's timestamp means that the screenshots can be viewed in the order they were taken simply by sorting by the modification date. This is supported out-of-the box in Finder and using the -t flag for ls in the terminal.

Test Plan/Testing Performed It feels like quite a safe change, so I've only performed a little bit of manual testing on an xcresult bundle. However I'd be happy to get some help with writing unit tests and/or doing some more manual testing on other result bundles!

Boerworz commented 1 year ago

Welp, turns out that even though APFS has nanosecond precision for the file modification date, the zip format only has 1 second precision for each file IIUC. This means that my changes in this PR work great when testing locally, but does not work as great when the screenshots are zipped to make them available as a build artefact on Bitrise. If there are multiple screenshots taken during the same second then these might be incorrectly ordered after unzipping them.

@abotkin-cpi @mgorkani: Let me know what you want to do with my changes! I still think setting the modification date is a good default behaviour to have even if it didn't quite solve the problem for my personal setup.

abotkin-cpi commented 1 year ago

@rsukumar-cpi Could you help review within the next week?

abotkin-cpi commented 1 year ago

Plan is to merge this early next week and cut a new build of xcparse by Wednesday.