Visitar URL original
Cleanup, Tests | MultipartIdentifier by edwardneal · Pull Request #3891 · dotnet/SqlClient · GitHub
Skip to content

Cleanup, Tests | MultipartIdentifier - #3891

Merged
paulmedynski merged 21 commits into
dotnet:mainfrom
edwardneal:cleanup/multipartidentifier
Mar 9, 2026
Merged

Cleanup, Tests | MultipartIdentifier#3891
paulmedynski merged 21 commits into
dotnet:mainfrom
edwardneal:cleanup/multipartidentifier

Conversation

@edwardneal

@edwardneal edwardneal commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Description

This work performs a small cleanup of the MultipartIdentifier type, and moves its test into the UnitTests project. That simplifies some of the tech debt identified in #3890, since we can use InternalsVisibleTo to access the type directly rather than manually include the source file.

This can be reviewed commit-by-commit - I don't expect any behavioural changes. There's existing test coverage, and this passes, but everything can be verified statically. These changes essentially remove parameters which are always constant, but which were passed as a result of the MultipartIdentifier type being originally used for providers other than SqlClient.

Issues

None, but dovetails with #3890 - @benrr101 for awareness.

Testing

Existing automatic tests cover this.

@edwardneal
Remove leftQuote and rightQuote parameters
65b528e 
These were always passed with the same, constant, value.
@edwardneal
Remove constant-value separator parameter
62988ae
@edwardneal
Remove constant-value removeQuotes parameter
3f12787
@edwardneal
Remove unnecessary test case
b7f564a 
This tested a case which would never occur.
@edwardneal
Minor comment cleanup
5d084e3
@edwardneal
Remove constant-value rightQuote parameter
f797d3b 
Also remove now-unnecessary test case.
@edwardneal
Remove constant-value leftQuote parameter
ae5551b 
Also remove a now-unnecessary test case.
@edwardneal
Minor cleanup
2215652 
Remove unnecessary IsWhitespace method.
Replace always-true runtime checks against constant values with debug assertions.
@edwardneal
Comment and syntax cleanup
df55f47
@edwardneal
if-block cleanup
4f6bfd4
@edwardneal
Remove unnecessary test case
daa76c3 
This tested the exception thrown when limit is zero. This is always either 3 or 4.
@edwardneal
Move MultipartIdentifierTests to the UnitTests project, remove condit…
8830d5c 
…ional inclusion of MultipartIdentifier from FunctionalTests
@edwardneal
Cleanup warning in MultipartIdentifierTests
5c581fb
@edwardneal
Remove now-unnecessary ParseMultipartIdentifier overload
c79ac4b
@edwardneal
edwardneal requested a review from a team as a code owner January 14, 2026 06:57
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Jan 14, 2026
@paulmedynski paulmedynski moved this from To triage to In review in SqlClient Board Jan 14, 2026
@paulmedynski paulmedynski added the Area\Tests Issues that are targeted to tests or test projects label Jan 14, 2026
@paulmedynski

paulmedynski commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

azure-pipelines Bot commented Jan 14, 2026

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

paulmedynski
paulmedynski requested changes Jan 14, 2026
View reviewed changes

@paulmedynski paulmedynski left a comment

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.

Another good move of some true unit tests, and valuable cleanup along the way. A few suggestions to improve the cleanup even more.

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/MultipartIdentifier.cs Resolved
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/MultipartIdentifier.cs Resolved
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/MultipartIdentifier.cs Resolved
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/MultipartIdentifier.cs Resolved
Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/MultipartIdentifier.cs Resolved
Comment thread src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/Common/MultipartIdentifierTests.cs Resolved
@github-project-automation github-project-automation Bot moved this from In review to In progress in SqlClient Board Jan 14, 2026
@edwardneal
Move debug assertions to comments on the constants
36239ad
@edwardneal
Naming cleanup
149606f 
Eliminates some ambiguity between "name" and "identifier", "identifier part" or "property name".
@edwardneal
Enable nullability annotations
1f7cde9
@edwardneal
Sense-check names of tests
1b5acff
@edwardneal

edwardneal commented Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @paulmedynski - I've just replied to the first round of review comments.

@github-actions

github-actions Bot commented Feb 14, 2026

Copy link
Copy Markdown

This pull request has been marked as stale due to inactivity for more than 30 days.

If you would like to keep this pull request open, please provide an update or respond to any comments. Otherwise, it will be closed automatically in 7 days.

@github-actions github-actions Bot added the Stale The Issue or PR has become stale and will be automatically closed shortly if no activity occurs. label Feb 14, 2026
This was referenced Nov 6, 2026
Open
Open
Closed
Merged
Open
Closed
Closed
Closed
Open
Merged
Closed
Open
Open
@MZurmuehlen89 MZurmuehlen89 mentioned this pull request Nov 10, 2026
Closed
This was referenced Nov 10, 2026
Open
Closed
Open
Closed
Merged
Merged
Open
Open
Open
Open
Open
Open
Open
Open
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cheenamalhotra cheenamalhotra cheenamalhotra approved these changes
@paulmedynski paulmedynski paulmedynski approved these changes
+1 more reviewer
@Wraith2 Wraith2 Wraith2 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@paulmedynski paulmedynski

Labels

Area\Tests Issues that are targeted to tests or test projects

Projects

SqlClient Board
Status: Done

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@edwardneal @paulmedynski @Wraith2 @cheenamalhotra