Skip to content

Fix line endings in Unicode diretion override tests #10628

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

Merged

Conversation

cameel
Copy link
Member

@cameel cameel commented Dec 16, 2020

One of the tests added in #10326 has Windows line endings. We probably have some line-ending conversion that kicks in only on non-CRLF platforms because the source locations in error messages end up being different on Windows because of this.

Details

See this failed run or t_win:

ASSERTION FAILURE:
- file   : boostTest.cpp
- line   : 125
- message: Test expectation mismatch.
Expected result:
  ParserError 8936: (128-139): Mismatching directional override markers in comment or string literal.
Obtained result:
  ParserError 8936: (124-135): Mismatching directional override markers in comment or string literal.

@cameel cameel requested review from chriseth and mijovic December 16, 2020 13:57
@cameel cameel self-assigned this Dec 16, 2020
mijovic
mijovic previously approved these changes Dec 16, 2020
Copy link
Contributor

@mijovic mijovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, let's see if windows is fine now

@axic
Copy link
Member

axic commented Dec 16, 2020

We probably have some line-ending conversion that kicks in only on non-CRLF platforms because the source locations in error messages end up being different on Windows because of this.

Well, should that be the case really?

@cameel
Copy link
Member Author

cameel commented Dec 16, 2020

Depends. That's pretty typical behavior in Python for example. I'm not sure if boost in C++ adds some conversion when you read a file in text mode. Might be worth investigating. Also, do we have any tests for different line endings?

@chriseth
Copy link
Contributor

The problem might also be related to git instead of C++ - the checkout routine on windows introduces different line breaks for text files.

@cameel
Copy link
Member Author

cameel commented Dec 16, 2020

But does it do it automatically? @axic says we don't have anything like this set up on purpose at least:

git has this feature to control line-ending conversion, but I don't think we have it set up

@chriseth chriseth force-pushed the fix-line-endings-in-unicode-direction-override-test branch from d970732 to 34e21c9 Compare December 16, 2020 14:24
@chriseth chriseth merged commit 33cc26f into develop Dec 16, 2020
@chriseth chriseth deleted the fix-line-endings-in-unicode-direction-override-test branch December 16, 2020 14:25
@axic
Copy link
Member

axic commented Dec 16, 2020

Some details about the git feature: https://stackoverflow.com/questions/21822650/disable-git-eol-conversions

Also, do we have any tests for different line endings?

I don't think so. Well we do have scanner tests to make sure they are parsed correctly, but no tests for the error reporter.

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.

4 participants