[Fix] Infinite while loop when called on root folder, add tests #29
Conversation
[Fix] Get parent folder when called on non-existent drive on windows
| /** | ||
| * Linux max file path length is 4096 characters. | ||
| * With / separators and 1 letter folder names, this gives us a max of ~2048 folders to traverse. | ||
| * This is much less error prone than a while loop. | ||
| */ |
There was a problem hiding this comment.
This limit is not valid on every file system
https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
So... I do not know if I want to add that to this loop.
There was a problem hiding this comment.
Is there a number we can change it to that would accomplish this goal? Or another way to avoid a while loop/provide a failsafe if we miss another edge case?
There was a problem hiding this comment.
Ultimately this is of course your call and I'm happy to revert to the while loop if you would like
|
|
||
| test('win32: Gets parent to C:\\HyperPlay', async t => { | ||
| const parentPath = normalize('C:\\') | ||
| const dependencies = getDependencies(parentPath) |
There was a problem hiding this comment.
I suggest to define the pathSep somewhere here.
There was a problem hiding this comment.
I am not following. Is this still relevant after the changes I committed?
BrettCleary
changed the title
|
@Alex-D apologies for the windows specific runner tests. it was wrong of me to assume that was a goal of this project. The main issue is the infinite while loop when called on a root directory, which is platform agnostic and this PR fixes and the new tests confirm this. I've updated the logic and tests to match what you were doing previously with win32 tests in |
When calling
getFirstExistingParentPathon the parent folder of a non-existent root drive on Windows, the while loop proceeds indefinitely, crashing the calling programThis is fixed by early return if one iteration of the loop does not change the parent folder
This also changes the while loop to a for loop proceeding up until the max # of folders (with the same condition to stop as the while loop). This is much less error prone
Tests have been added and two other tests have been modified to run on windows as well