tests/more: run all tests with terminal_simulation

[tertsdiepraam]

Closes #6100

@cakebaker @BenWiederhake, could you check whether this solves the problem for you too?

I think that with this change, we can run the tests in the CI too, but I first want to check whether the issue is fixed. I could also clean this up, but I went with a quick and dirty solution first.

[BenWiederhake]

Huh, this is weird. The terminal is fixed (even running it several times and in various ways, the terminal never breaks), but now test_more::test_more_no_arg is failing:

---- test_more::test_more_no_arg stdout ----
run: /home/nattiff/workspace/coreutils-rs/target/debug/coreutils more
thread 'test_more::test_more_no_arg' panicked at 'Command was expected to fail.


 stderr = ', tests/by-util/test_more.rs:13:14


failures:
    test_more::test_more_no_arg

[cakebaker]

I get the same result as @BenWiederhake: the terminal is fixed and test_more_no_arg fails.

[tertsdiepraam]

Looks like the terminal simulation is broken then, I'll take another look...

[tertsdiepraam]

@cre4ture since you implemented the terminal simulation, do you know what could be wrong with stdin? It looks like it keeps hanging.

This is also a bug in more by the way. The util-linux more can exit immediately, but uutils more waits for ctrl+d.

[cre4ture]

@tertsdiepraam I will take a look into the tests. Afterwards, maybe I can help.

[cre4ture]

after investigation, I think the issue is on uu_more side and not the test-utils.

I compared the behaviour of GNU with uutils:

uli@hp13-ulix:~/dev_rust/coreutils$ echo -n "" | more ; echo $?
0
uli@hp13-ulix:~/dev_rust/coreutils$ echo -n "" | target/debug/coreutils more ; echo $?
more: bad usage
Try 'target/debug/coreutils more --help' for more information.
1
uli@hp13-ulix:~/dev_rust/coreutils$ 

I did some more tests in a similar direction (using /dev/null as input, pipe outputs to tee, ...).
I even used the GNU more in our test-suite to compare behaviour.
I concluded that the only condition that more needs to put out the error message is that stdin is connected to a terminal.

The following patch fixes the issue for me. I would have pushed it directly to this PR, but I don't know how to do this.

diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs
index 0b8c838f2..987b48998 100644
--- a/src/uu/more/src/more.rs
+++ b/src/uu/more/src/more.rs
@@ -5,7 +5,7 @@
 
 use std::{
     fs::File,
-    io::{stdin, stdout, BufReader, Read, Stdout, Write},
+    io::{stdin, stdout, BufReader, IsTerminal, Read, Stdout, Write},
     panic::set_hook,
     path::Path,
     time::Duration,
@@ -158,10 +158,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         }
         reset_term(&mut stdout);
     } else {
-        stdin().read_to_string(&mut buff).unwrap();
-        if buff.is_empty() {
+        if stdin().is_terminal() {
             return Err(UUsageError::new(1, "bad usage"));
         }
+        stdin().read_to_string(&mut buff).unwrap();
         let mut stdout = setup_term();
         more(&buff, &mut stdout, false, None, None, &mut options)?;
         reset_term(&mut stdout);

[tertsdiepraam]

@cre4ture Sounds good! Could you put that up as a PR?

I do think that somethings up with the terminal simulation though. Or at least some additional configuration might be necessary, where it's possible to simulate a terminal only for output or something like that. Would that make sense?

[cre4ture]

where it's possible to simulate a terminal only for output or something like that. Would that make sense?

true, especially for tools like more it does makes sense. Here the condition is that the only the output is allowed to be a terminal. I will create another PR for adding some destiction possibilitiy.

[github-actions]

GNU testsuite comparison:

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

[sylvestre]

inactive for a while, closing. cc @tertsdiepraam