10 tests fail on Windows

[jednano]

This is a long-standing issue (years) that continues to plague me when working on the editorconfig-core-js project. These tests run fine in the Travis-CI environment, but fail on Windows in my local dev environment.

@xuhdev do you know if there's something special about these particular tests that would make them fail on Windows, specifically? It makes it pretty difficult to do development when I have to wait for the CI environment to finish each run.

The following tests FAILED:
          9 - brackets_close_inside (Failed)
         11 - brackets_nclose_inside (Failed)
         55 - braces_escaped_comma1 (Failed)
         58 - braces_escaped_brace1 (Failed)
         59 - braces_escaped_brace2 (Failed)
         60 - braces_escaped_brace3 (Failed)
        146 - escaped_semicolon_in_section (Failed)
        153 - escaped_octothorpe_in_section (Failed)
        168 - windows_separator (Failed)
        169 - windows_separator2 (Failed)

[xuhdev]

For 168 and 169, I can sort of imagine incorrect handling of path separator. I guess the rest of the failed tests may also be affect by that.

[jednano]

If you can give me some tips to debug and test just one test at a time, I think I might be able to figure it out. Also, it's a bit hard for me to navigate these tests and find the appropriate one. I'm not at all familiar w/ ctest.

[jednano]

Is this where the two Windows tests are written?

[jednano]

Or is this where I should be looking?

[xuhdev]

@jedmao I think these should be what you are looking for...

[jednano]

These tests are pretty hard to read. I can't tell what the input/output is or what the expected value should be. Definitely not what I'm used to.

[xuhdev]

You should run ctest -VV --output-on-failure . to see output. And the definition of the test is in CMakeLists.txt.

[jednano]

Yeah. I knew that. I'm just expecting to see something like:

expect(foo('bar')).toEqual('baz');

Where it's very clear that foo is the function being tested, "bar" is the input and "baz" is the expected output.

I don't know how to translate what I'm seeing into that mindset.

[jednano]

For example:

new_ec_test(windows_separator path_separator.in windows/separator "^[ \t\n\r]*$")

How do I read that?

[xuhdev]

• windows_separator is the name of the test.
• path_separator.in is the name of the input .editorconfig file (-f option).
• windows/separator is the file path passed into editorconfig.
• "^[ \t\n\r]*$" is the expected regex to match the output.

[jednano]

If I navigate to the filetree folder and run the following command:

editorconfig -f path_separator.in windows/separator

The output is:

key=value

Which I presume came from line 12. To me, this means the editorconfig command worked, even on Windows. I just don't see how the regex would match that output.

/^[ \t\n\r]*$/.test('key=value'); // false

Am I missing something? Or was it not supposed to hit line 12? Is there an assumption here that EditorConfig wouldn't actually work on Windows, but it is, which is throwing the test off?

[xuhdev]

@jedmao I can vaguely remember that we decided that all path separators in brackets must be /, even on Windows. @treyhunner Can you help confirm this?

[jednano]

I see, so it seems there actually is something wrong with the js core in that it shouldn't be accepting back slashes. That would make some sense.

The regex above is saying there should be no match, just white spaces, right?

[xuhdev]

Yes, that's correct.

[jednano]

@xuhdev, I think it would make sense to document our ini-like format in something that sounds more like English than these tests so we don't have similar confusions now or in the future.

[jednano]

I know for a fact that minimatch, the library that the js core has modified internally, accepts any variety of slashes, so that would do it.

[xuhdev]

OK, I fixed it in the doc: editorconfig/editorconfig.github.com@44b8cc1d390b72a4e9dcbfe7a1ae1f16e558573e

[xuhdev]

Is this issue still relevant today?

[jednano]

Yes. I think it warrants adding AppVeyor to the CI builds.

[cxw42]

Example Appveyor config, in case it helps: https://github.com/cxw42/editorconfig-core-vimscript/blob/master/appveyor.yml

[cxw42]

@jedmao If you have a chance, would you please test the JS core with the updates from #29? I would be interested to know if they help for JS, since they help the Vim and C cores pass on Windows.

[jednano]

Sure thing. I’ll try to find some time.

[jednano]

@cxw42 I put up a PR, but it breaks the build. I don't think the issue is with the new tests, however. I think there's actually a problem with the js core, which I was trying to fix a while back in this PR. Really, it's the INI parser that's the problem, from what I can tell. Perhaps we need something with more of a formal grammar.

[cxw42]

Thanks for giving it a shot! If I get any ideas about the build failure, I'll let you know.