Skip to content

Test selection issue with normalized newlines#4773

Merged
stevenaw merged 3 commits into
nunit:mainfrom
stevenaw:4584-selection-issue
Aug 4, 2024
Merged

Test selection issue with normalized newlines#4773
stevenaw merged 3 commits into
nunit:mainfrom
stevenaw:4584-selection-issue

Conversation

@stevenaw
Copy link
Copy Markdown
Member

@stevenaw stevenaw commented Aug 2, 2024

Fixes #4584

An initial pass to turn off line ending normalization while we decide on final approach. Still room for improvement and optimizations

Comment thread src/NUnitFramework/tests/Issue4584.cs Outdated
@stevenaw stevenaw marked this pull request as draft August 2, 2024 03:00
@manfred-brands manfred-brands force-pushed the 4584-selection-issue branch 2 times, most recently from af94ab4 to f43e62c Compare August 2, 2024 03:48
Copy link
Copy Markdown
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

Your fix makes the code compatible with v3 with the minimum amount of changes.

return display;
}

public static string EscapeControlChars(string s)
Copy link
Copy Markdown
Member Author

@stevenaw stevenaw Aug 2, 2024

Choose a reason for hiding this comment

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

@manfred-brands Was this still needed? I don't see it referenced.

EDIT: Oh, sorry, I see it's used in the test you added. Just seeking clarity then, did you mean to add it here instead of in the tests in order to serve as a helper for others?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@stevenaw I added it initially to allow escaping characters for all TestNames. Similar like what is done in case the parameter is a string. As we haven't decided on an approach, I added a commit to remove it.
That allows this branch to be merged and 4.2 to be released.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks @manfred-brands . Definitely nice to get this merged in case it's blocking others from upgrading

public class Issue4584
{
[Test]
public void DiscoverAndExecution()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Looks great. Thanks @manfred-brands !

@stevenaw stevenaw marked this pull request as ready for review August 2, 2024 23:50
@stevenaw stevenaw changed the title WIP - Test selection issue with normalized newlines Test selection issue with normalized newlines Aug 2, 2024
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.

Nunit 4.0.x Test case selection is incorrect with certain test data

2 participants