Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve "stat" compatibility #6933

Merged
merged 8 commits into from
Dec 10, 2024
Merged

Improve "stat" compatibility #6933

merged 8 commits into from
Dec 10, 2024

Conversation

sylvestre
Copy link
Contributor

No description provided.

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 7, 2024

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/stat/stat-fmt is no longer failing!

@sylvestre sylvestre force-pushed the stats branch 3 times, most recently from 655c7cc to a945717 Compare December 7, 2024 00:53
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 7, 2024

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/stat/stat-fmt is no longer failing!

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 7, 2024

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/stat/stat-fmt is no longer failing!
Congrats! The gnu test tests/stat/stat-printf is no longer failing!

@sylvestre
Copy link
Contributor Author

Regressed this test. I will have a look later.

TRY 3 STDERR:        coreutils::tests test_stat::test_printf ---
thread 'test_stat::test_printf' panicked at tests/by-util/test_stat.rs:250:38:
assertion failed: `(left == right)`

Diff < left / right > :
<123?
"\��ev                   /�wZJ
>123?
"\���                   /�wZJ

@sylvestre sylvestre force-pushed the stats branch 3 times, most recently from c82adaa to 6d0db47 Compare December 7, 2024 21:19
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 7, 2024

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/stat/stat-fmt is no longer failing!
Congrats! The gnu test tests/stat/stat-printf is no longer failing!

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 7, 2024

GNU testsuite comparison:

Congrats! The gnu test tests/stat/stat-fmt is no longer failing!
Congrats! The gnu test tests/stat/stat-printf is no longer failing!

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 8, 2024

GNU testsuite comparison:

Congrats! The gnu test tests/stat/stat-fmt is no longer failing!
Congrats! The gnu test tests/stat/stat-printf is no longer failing!

cakebaker
cakebaker reviewed Dec 8, 2024
View reviewed changes
src/uu/stat/src/stat.rs Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 8, 2024

GNU testsuite comparison:

Congrats! The gnu test tests/stat/stat-fmt is no longer failing!
Congrats! The gnu test tests/stat/stat-printf is no longer failing!
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

cakebaker
cakebaker reviewed Dec 9, 2024
View reviewed changes
src/uu/stat/src/stat.rs Outdated Show resolved Hide resolved
cakebaker
cakebaker reviewed Dec 9, 2024
View reviewed changes
src/uu/stat/src/stat.rs Outdated Show resolved Hide resolved
cakebaker
cakebaker reviewed Dec 9, 2024
View reviewed changes
src/uu/stat/src/stat.rs
Comment on lines +333 to +334
display_name: &str,
file: &OsString,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is display_name necessary? It looks to me like you could get it from file with file.to_string_lossy().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems legit but it isn't:
https://github.com/uutils/coreutils/blob/main/src/uu/stat/src/stat.rs#L639-L640
as you can see here, we are keeping the original name and changing :)

cakebaker
cakebaker reviewed Dec 9, 2024
View reviewed changes
src/uu/stat/src/stat.rs Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2024

GNU testsuite comparison:

Congrats! The gnu test tests/stat/stat-fmt is no longer failing!
Congrats! The gnu test tests/stat/stat-printf is no longer failing!
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@cakebaker cakebaker merged commit c60203d into uutils:main Dec 10, 2024
63 of 64 checks passed
@cakebaker
Copy link
Contributor

Congrats! The gnu test tests/stat/stat-fmt is no longer failing!
Congrats! The gnu test tests/stat/stat-printf is no longer failing!

Cool, two tests fixed with one PR, good job :)

@sylvestre sylvestre deleted the stats branch December 10, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cakebaker cakebaker cakebaker left review comments

@tertsdiepraam tertsdiepraam Awaiting requested review from tertsdiepraam

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sylvestre @cakebaker