Skip to content

test_cp.rs: fix error message of ficlone on musl - #14273

Merged
sylvestre merged 1 commit into
uutils:mainfrom
oech3:patch-2
Aug 30, 2026
Merged

test_cp.rs: fix error message of ficlone on musl#14273
sylvestre merged 1 commit into
uutils:mainfrom
oech3:patch-2

Conversation

@oech3

@oech3 oech3 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Closes #14274

@github-actions

Copy link
Copy Markdown

Binary size comparison:

Individual binary size comparison VS main (threshold: >=5% AND >=4 KB).

Total size of compared binaries: 152.18 MB (+1004 KB, +0.65%)

Significant per-binary changes:
  comm     1.12 MB ->    2.33 MB  (+1.21 MB, +107.29%)

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/tail-n0f (passes in this run but fails in the 'main' branch)

Comment thread tests/by-util/test_cp.rs
.fails()
.no_stdout()
.stderr_contains("Invalid cross-device link");
.stderr_contains("ross-device link"); // cover both of glibc and musl

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.

Suggested change
.stderr_contains("ross-device link"); // cover both of glibc and musl
.stderr_contains(if cfg!(target_env = "musl") {
"ross-device link"
} else {
"Invalid cross-device link"
});

I'd prefer to be more explicit here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think there is worth to increase complexity at here just for test.

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.

I think the explicit cfg!(target_env = "musl") is worthwhile here. The test is covering a known difference in the diagnostic, so checking the expected string for each environment makes that clearer. The added complexity is small and limited to the test.

@oech3 oech3 Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It does not cover the case musl started aligning message with GNU at some version (or inverse).

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.

If the platform message changes, I think it's useful for the test to fail. It flags the change and lets us decide if anything needs updating.

Comment thread tests/by-util/test_cp.rs

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.

To be honest I think we should split test into separate test_cp_reflink_always_invalid_argument and test_cp_reflink_always_cross_device tests.

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.

don't hesitate to submit a new PR :)

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.

@sylvestre
sylvestre merged commit 6cb1d5e into uutils:main Aug 30, 2026
96 checks passed
@oech3
oech3 deleted the patch-2 branch August 30, 2026 17:42
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.

test_cp_reflink_always_failure failure

3 participants