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

Image file name changed and useless 32bit images after framework update #78

Closed
liuxuan30 opened this issue Apr 10, 2019 · 2 comments
Closed

Comments

@liuxuan30
Copy link

liuxuan30 commented Apr 10, 2019

Hi there,

Not sure if you can see my comments in ChartsOrg/Charts#3874, so I posted here again. sorry for any inconvenience.

the image file name pattern has been changed, such as from testDefaultValues_iOS_375.0_667.0@2x.png to testDefaultValues_iOS_375_0_667_0@2x.png
note 375.0_667.0 to 375_0_667_0
there is a ReferenceImages_32 folder added, while we only have ReferenceImages_64.
Since this seems caused by the test case framework update itself, I'm going to ask @alanzeino for a quick help: is it configurable so we don't have to change the name pattern? Can we disable 32bit images? I don't think we need a 32bit arch, iPhone 5 is long dead now. Basically I don't want to touch the images, it should work by itself.

@reidmain
Copy link

Sorry it has taken so long to respond to this @liuxuan30.

Regarding the change in the filename pattern I am actually quite surprised to hear that. We did add a fileNameOptions property to customize what was included but I was under the impression that it was backwards compatible. I apologize for the breaking change but I think regenerating the snapshots and having git recognize them as a move is probably the best thing.

The useless "ReferenceImages_32" folder was also mentioned here #42 and I agree with you 100%. I also see you found the workaround of explicitly setting the suffixes to only contain "_64". #42 also recommends changing the suffixes so that it only includes the architecture that is being testing and I don't see any reason why we can't make that change.

@liuxuan30
Copy link
Author

ah ha, I already forgot I filed this :)
closing. Thanks for replying! I think we are cool now. re-generate the image is the best workaround so far.

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

No branches or pull requests

2 participants