ducminh-phan / reformat-gherkin

Formatter for Gherkin language
MIT License
22 stars 13 forks source link

On Windows, reformatting a file larger than 32 KB causes a "path too long for Windows" error #34

Closed rayjolt closed 4 years ago

rayjolt commented 4 years ago

On Windows, trying to reformat a file longer than 32 KB fails with the error "stat: path too long for Windows".

Steps to reproduce

  1. Download test.txt (size: 34 KB)
  2. Run reformat-gherkin on it with reformat-gherkin test.txt

Expected result

The file is reformatted

Actual result

The following error occurs:

Error: cannot format C:\path\to\test.txt: stat: path too long for Windows
All done! \U0001f4a5 \U0001f494 \U0001f4a5
1 file failed to reformat.

Environment

reformat-gherkin 1.1.0 Python 3.7.3 (32-bit) Windows 10

rayjolt commented 4 years ago

I tracked this down to the Gherkin TokenScanner implementation. When you pass a string to Gherkin's Parser.parse, a TokenScanner instance is created, and on creation it checks to see if that string is a path on the filesystem using os.path.exists(). If the path exists, then Gherkin parses the contents of that file; if not, it parses the original string.

The problem is that many Windows API path functions have a maximum length of 32,767 characters, so if a feature file is longer than that limit, then Windows will throw an error when trying to use it as a path in os.path.exists().

Ideally this would be fixed in Gherkin itself, but judging from my last bug report there that will take time - someone needs to volunteer to maintain the Python Gherkin fork and get all the tests to pass again before they will release a new version. As a workaround we can pass paths to Parser.parse() instead of feature file contents.

ducminh-phan commented 4 years ago

Passing paths to Parser.parse() is not feasible since we need to make sure that we have an equivalent AST after formatting.

To work around the issue, we can subclass TokenScanner to handle strings only, then pass it to Parser.parse().

rayjolt commented 4 years ago

Yes, that sounds like a good workaround until it can be fixed upstream.

ducminh-phan commented 4 years ago

@rayjolt Can you work on the issue? I don't have access to a Windows computer at the moment, so there's no way to reproduce and test but relying on Azure pipelines.

rayjolt commented 4 years ago

Sure, I'll have a look at it over the weekend.