Skip to content

Fix MemoryExtensions.TrimStart#58587

Merged
jcouv merged 2 commits intodotnet:mainfrom
jcouv:trim-start
Jan 7, 2022
Merged

Fix MemoryExtensions.TrimStart#58587
jcouv merged 2 commits intodotnet:mainfrom
jcouv:trim-start

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented Jan 4, 2022

Fixes #58074

@jcouv jcouv self-assigned this Jan 4, 2022
@ghost ghost added the Area-Compilers label Jan 4, 2022
@jcouv jcouv added this to the 17.1 milestone Jan 4, 2022
@jcouv jcouv marked this pull request as ready for review January 4, 2022 06:40
@jcouv jcouv requested a review from a team as a code owner January 4, 2022 06:40
}

[Fact]
public void WhitespaceInDefine()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

WhitespaceInDefine

Would it make sense to test MemoryExtensions.TrimStart() directly?

Copy link
Copy Markdown
Contributor

@cston cston Jan 4, 2022

Choose a reason for hiding this comment

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

Consider testing TrimStart() with a string that contains only spaces, and with an empty string.

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.

I considered direct testing, but it's not what we commonly do for internal APIs. We typically verify bugs via their impact on public APIs.

Added a test with only spaces.

For define: with an empty string, that is already covered by test InvalidDefineSwitch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implementation of extension method causes ArgumentOutOfRangeException

4 participants