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

Update core tests: fixes for CRLF in multiline tests, executable-not-found issue #62

Merged
merged 1 commit into from
Oct 22, 2019

Conversation

cxw42
Copy link
Member

@cxw42 cxw42 commented Aug 4, 2019

@cxw42
Copy link
Member Author

cxw42 commented Oct 20, 2019

@xuhdev Have you seen this failure mode before? --- https://travis-ci.org/editorconfig/editorconfig-core-c/jobs/600209206#L299 . pcre2 static-link failure on Xenial with GCC. I checked, and libpcre2-dev on xenial does have a static library.

Other than that, it seems like my latest test change may have helped - most of the jobs in that build passed.

@xuhdev
Copy link
Member

xuhdev commented Oct 20, 2019

Awesome! This is really good work! 🎆

No I haven't seen that error before. I'm enabling verbose output in #64 so we can see what's going on.

@xuhdev
Copy link
Member

xuhdev commented Oct 20, 2019

But path_separator_backslash_in_cmd_line fails now on Windows, which did not fail before...

I'll try to fix the static build issue (which shouldn't be relevant here). Something may have changed on Travis...

@cxw42
Copy link
Member Author

cxw42 commented Oct 20, 2019

@xuhdev Thanks for looking into the static-build issue!

I am continuing to investigate the backslashes. The problem with the Windows builds seems to be twofold:

  • CTest is checking its input strings for backslash escapes, and warning if they aren't valid, so we need \\.
  • But CTest is also passing the strings through unchanged to the shell, so we need \!

I am checking whether I can move the $BackSlashes variable where CMake won't process it but CTest will. Edit https://cmake.org/cmake/help/v3.12/prop_dir/TEST_INCLUDE_FILE.html#prop_dir:TEST_INCLUDE_FILE

Edit The backslashes differ between editorconfig-core-c and editorconfig-vim because the former was using add_test(NAME ... COMMAND ...) and the latter was using add_test(...). The former processes the arguments in a way the latter does not.

@cxw42
Copy link
Member Author

cxw42 commented Oct 20, 2019

@xuhdev I am logging off for now and plan to work on this again tomorrow. o/

@xuhdev
Copy link
Member

xuhdev commented Oct 20, 2019

Thanks! #69 should have resolved the static build issue -- feel free to rebase

@cxw42
Copy link
Member Author

cxw42 commented Oct 20, 2019

@xuhdev Yep, Travis build passed :) . Thanks for the quick fix!

@cxw42 cxw42 force-pushed the cxwtest branch 2 times, most recently from 80f3354 to 80d6d1b Compare October 20, 2019 22:59
@cxw42 cxw42 changed the title Changes for proposed test changes Update core tests: fixes for CRLF in multiline tests, executable-not-found issue Oct 20, 2019
@cxw42 cxw42 marked this pull request as ready for review October 22, 2019 01:31
@xuhdev xuhdev merged commit e70d90d into editorconfig:master Oct 22, 2019
@xuhdev
Copy link
Member

xuhdev commented Oct 22, 2019

🎆 🎆 🎆

@cxw42 cxw42 deleted the cxwtest branch October 22, 2019 12:01
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