Skip to content

mv: Adding fixes for i-3 GNU tests related to tty output when file is not writeable#9599

Merged
sylvestre merged 7 commits intouutils:mainfrom
ChrisDryden:mv_prompt
Dec 27, 2025
Merged

mv: Adding fixes for i-3 GNU tests related to tty output when file is not writeable#9599
sylvestre merged 7 commits intouutils:mainfrom
ChrisDryden:mv_prompt

Conversation

@ChrisDryden
Copy link
Collaborator

@ChrisDryden ChrisDryden commented Dec 7, 2025

Discovered when automating the stty test discovery on the github CI that the reason this test was failing was because it required to be run with an interactive tty, the logic is that when the -i flag is not provided and a file is piped in but it is not writable the GNU implementation prints an output to the tty.

Its exciting that this is the last mv test thats not passing.

@github-actions
Copy link

github-actions bot commented Dec 7, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/mv/i-3 is no longer failing!

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 7, 2025

CodSpeed Performance Report

Merging #9599 will not alter performance

Comparing ChrisDryden:mv_prompt (101b9da) with main (3e71f63)

Summary

✅ 127 untouched
⏩ 6 skipped1

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions
Copy link

github-actions bot commented Dec 7, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/mv/i-3 is no longer failing!

@ChrisDryden ChrisDryden marked this pull request as ready for review December 7, 2025 22:12
@ChrisDryden
Copy link
Collaborator Author

Same as with the other PR's that codspeed is just being flaky. Should be ready for another round of review

@ChrisDryden ChrisDryden changed the title Adding fixes for i-3 GNU tests related to tty output when file is not writeable mv: Adding fixes for i-3 GNU tests related to tty output when file is not writeable Dec 8, 2025
@ChrisDryden
Copy link
Collaborator Author

I'm curious on your take with the cleaning up of the readability of the tests?

I'm finding that theres a mismatch of integration test styles in every folder and that its cleaner when you do something like:

    let (at, mut ucmd) = at_and_ucmd!();
        .stderr_contains("replace 'target', overriding mode 0000")

compared to:

    let scene = TestScenario::new(util_name!());
    let at = &scene.fixtures;
    let result = scene
        .ucmd()
        
        
    assert!(
        result
            .stderr_str()
            .contains("replace 'target', overriding mode 0000")
    );

But I know its frowned upon to do these types of large scale refactors on projects like this, would you mind if I go through the tests and make them match the top format with the stderr matching on the ucmd and adding the macros for creating the test env?

@ChrisDryden ChrisDryden force-pushed the mv_prompt branch 4 times, most recently from eec091e to 2d9b035 Compare December 10, 2025 18:03
@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/mv/i-3 is no longer failing!

1 similar comment
@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/mv/i-3 is no longer failing!

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/mv/i-3 is no longer failing!
Congrats! The gnu test tests/tail/inotify-dir-recreate is now passing!

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/tty/tty-eof. tests/tty/tty-eof is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/mv/i-3 is no longer failing!
Congrats! The gnu test tests/rm/fail-eperm is now passing!

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/mv/i-3 is no longer failing!

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/mv/i-3 is no longer failing!
Congrats! The gnu test tests/tail/follow-name is no longer failing!

@sylvestre sylvestre merged commit e6467b1 into uutils:main Dec 27, 2025
168 of 172 checks passed
cerdelen pushed a commit to cerdelen/coreutils that referenced this pull request Dec 28, 2025
… not writeable (uutils#9599)

* Adding fixes for i-3 GNU tests related to tty output when file is not writeable

* Removed translation zero stripping hack and fixed clippy and spelling errors

* Removed unused windows import

* Addressing comments and cleaning up tests

* Spellcheck fixes

* Replacing helper function in other places too

* mv: address review comments - reorder enum, cache mode, safer defaults

du: fix count_link only working with -a

du: fix count_link only working with -a (linux)
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