tests issue #2805 - overgeneralised recognition of file as keyword in csharp lexer #2806
Conversation
Currently the added tests all show up as expected fails when running pytest; each test has a parameter that can be switched to `True` when a fix is implemented.
|
On second thoughts, I will mark this as a draft for now and actually try to implement some fixes first. |
This fixes the cases where 'file' is incorrectly tagged as a keyword. The tests for those cases have been switched from `pytest.xfail` to normal.
Adds tests for all cases covered by the new regex. Reformats the tests to make them more compact.
|
As far as I can see, the pull request now adequately deals with overgeneralised tagging of 'file' as a keyword. The one expected fail is due to a different issue, involving expressions of the form Anyway, I think that this is now ready for review; aside from the additional tests, only a couple of lines ended up being changed. |
|
Thanks for the PR! The fix looks good to me. In general, we'd prefer tests that check for desired token output in the existing "snippet" format to custom test functions/methods. Are you ok with changing that over? |
|
I'll certainly have a look at it. While I've made use of pygmentize for a while, my first dive into the code was just over a week ago. However, from what I remember, converting the tests to the snippet format shouldn't be too difficult. I mostly wrote the test the way I did because I wasn't sure how long it was going to take me to get around to working on the issue, and I wanted to write some test cases in advance; the warm fuzzy feeling that it gave me is probably less important than having something that fits in with the rest of the code :) Thanks for the quick response to the submission. I'm going to get some sleep before I work on anything, though. It's just past 2:30am where I am, and I have things to get done tomorrrow. |
This change replaces the test written using pytest with the file `tests/snippets/csharp/test_file_keyword.txt`. The change removes the ability to toggle between `xfail` and real failure for cases that are not expected to pass, but brings testing in line with the rest of the codebase. The tests involving `await` have been removed - which is fitting, as it was not the same issue.
|
The commit message really says it all. I've switched testing over to the "snippet" format and run the tests to ensure that everyting still works. Sorry about bailing last night, but I was really getting too tired to think properly. |
|
Very nice, thank you! |
…yword in csharp lexer (pygments#2806) * test: adds tests showing problems with 'file' Currently the added tests all show up as expected fails when running pytest; each test has a parameter that can be switched to `True` when a fix is implemented. * chore: removed extraneous material from test * fix: fixes test cases where 'file' isn't a keyword This fixes the cases where 'file' is incorrectly tagged as a keyword. The tests for those cases have been switched from `pytest.xfail` to normal. * test: adds test cases covering file as keyword Adds tests for all cases covered by the new regex. Reformats the tests to make them more compact. * fix: replaces pytest for issue pygments#2806 with snippet This change replaces the test written using pytest with the file `tests/snippets/csharp/test_file_keyword.txt`. The change removes the ability to toggle between `xfail` and real failure for cases that are not expected to pass, but brings testing in line with the rest of the codebase. The tests involving `await` have been removed - which is fitting, as it was not the same issue. --------- Co-authored-by: Josef Meyer <[email protected]>
3 participants
This PR adds some unit tests that point out instances where
pygments.lexers.dotnet.CSharpLexercurrently overgeneralises in recognising "file" as a keyword (issue #2805). These tests are currently set up as expected fails under pytest, but can be made active on a case-by-case basis by changing a flag. The issue of calls following "await" being incorrectly tagged withToken.Name.Functionrather thanToken.Nameis actually a different issue, but was considered as a candidate and left in as a problematic case.It is my intention to try fixing these and related issues at some point in the near future, but for now these tests identify cases that I think shold be addressed.