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

Truncate file names for screenshots #2635

Merged
merged 9 commits into from
Oct 31, 2018

Conversation

lilaconlee
Copy link
Contributor

@lilaconlee lilaconlee commented Oct 22, 2018

Opted to set a max length of 242 for the original string so it's 255 with (failed).png

@lilaconlee lilaconlee force-pushed the issue-2403-truncate-screenshot-name branch from f564b26 to 7b1f765 Compare October 30, 2018 15:41
@@ -313,14 +314,24 @@ getPath = (data, ext, screenshotsFolder) ->
else
names = [data.titles.map(replaceInvalidChars).join(RUNNABLE_SEPARATOR)]

# truncate file names to be less than 220 characters
# to accomodate filename size limits
maxFileNameLength = 220
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit funky. From testing outside of Cypress, seems like this value should be 255 - length of (failed) (N).png, but that still causes a name too long error on CI. That doesn't happen when this value is a bit smaller 🤷‍♀️

@brian-mann
Copy link
Member

I think ideally we would not truncate ahead of time, but instead recover from thrown errors like ENAMETOOLONG or whatever it is.

I'm merging this in regardless, but in case we wanted to tighten up later we could.

@brian-mann brian-mann merged commit 5da5bd2 into develop Oct 31, 2018
@lilaconlee lilaconlee deleted the issue-2403-truncate-screenshot-name branch November 6, 2018 19:15
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.

2 participants