Skip to content

test: add StringSlice Append coverage - #504

Merged
tomasaschan merged 1 commit into
spf13:masterfrom
LY-19418:test-string-slice-append
Sep 6, 2026
Merged

test: add StringSlice Append coverage#504
tomasaschan merged 1 commit into
spf13:masterfrom
LY-19418:test-string-slice-append

Conversation

@LY-19418

Copy link
Copy Markdown

Add test coverage for StringSlice SliceValue.Append.

This follows the existing SliceValue Replace coverage pattern
and only adds tests without changing runtime behavior.

Test:
go test ./...

@CLAassistant

CLAassistant commented Aug 18, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@tomasaschan tomasaschan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you! I don't see any issues with those change, but I'm curious: any particular reason for sending us this patch?

@LY-19418

LY-19418 commented Sep 2, 2026

Copy link
Copy Markdown
Author

谢谢!我没看出这些改动有什么问题,但我很好奇:您发送这个补丁有什么特别的原因吗?

I was looking through the SliceValue implementations and their tests to better understand the codebase, and noticed that Append is implemented for stringSliceValue but didn't appear to have direct test coverage.
Since there is already coverage for Replace in TestSSAsSliceValue, I thought adding a small test for Append would help cover that existing behavior and serve as a regression test.
There isn't a bug report or issue behind it — I just noticed the missing coverage while exploring the codebase.

@tomasaschan
tomasaschan merged commit 5c5ea33 into spf13:master Sep 6, 2026
8 checks passed
@LY-19418
LY-19418 deleted the test-string-slice-append branch September 7, 2026 11:20
@LY-19418

LY-19418 commented Sep 7, 2026

Copy link
Copy Markdown
Author

Thanks for reviewing and merging! @tomasaschan

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.

3 participants