Remove assertion for escapes in FileAccessTreeTests #122069
Conversation
The only real path separators are either forward or back slash. Trying to use something else like newline fails to even parse as a path on windows. This commit removes testing of other separators. closes elastic#121872
rjernst
added
>test
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
I wasn't testing newline as a separator. I was testing that a backslash is not accidentally interpreted as an escape character. |
|
Parsing of a path with a newline fails on windows. Did you mean to put a literal backslash followed by an |
|
No, I meant a newline. Short answer: let's delete this and forget the whole thing. 😂 Longer answer: I managed to scare myself that someone could put a backslash in one of our files someplace, and since it's so common that backslash is a separator, I worried that someone along the line would treat it as an escape. I wanted to assert that this won't happen. But I'm not sure that's a real risk, and even if it is, I'm not sure this is the right place to test for it. |
|
I'm still not sure the test makes sense. A backslash for escaping is handled by string parsing. The filesystem doesn't interpret the escape. |
prdoyle
left a comment
prdoyle
left a comment
There was a problem hiding this comment.
This was a bit misguided and is obviously doing more harm than good.
rjernst
changed the title
|
There are a lot of layers that could interpret backslashes along the chain from file contents to in-memory data structures, including the YAML parser, our own normalization logic, etc. My thinking was, putting the test here would ensure none of them trip over the backslash. But again... this was probably misguided so let's give it the axe. |
The only real path separators are either forward or back slash. Trying to use something else like newline fails to even parse as a path on windows. This commit removes testing of other separators. closes elastic#121872
The only real path separators are either forward or back slash. Trying to use something else like newline fails to even parse as a path on windows. This commit removes testing of other separators. closes elastic#121872
The only real path separators are either forward or back slash. Trying to use something else like newline fails to even parse as a path on windows. This commit removes testing of other separators. closes elastic#121872
3 participants
FileAccessTrees operate on string paths, but use the FileSystem to normalize the path. This test assertion is trying to check backslash's aren't interpreted as an escape, but escapes happen before the path is sent to FileSystem, so here we end up passing a newline as part of a path, which fails on windows. This commit removes the assertion.
closes #121872