mgunyho / tere

Terminal file explorer
European Union Public License 1.2
1.67k stars 36 forks source link

Integration test failures during Arch Linux build #93

Closed orhun closed 5 months ago

orhun commented 1 year ago

Integration tests were added in the last release, more specifically: https://github.com/mgunyho/tere/commit/70d2686e3ad5bd950e6137dd755a0d7e59b42dc1

I'm getting the following test failures in a clean chroot build:

running 4 tests
test basic_run ... FAILED
test first_run_prompt_accept ... FAILED
test output_on_exit_without_cd ... FAILED
test first_run_prompt_cancel ... FAILED

failures:

---- basic_run stdout ----
thread 'basic_run' panicked at 'assertion failed: `(left == right)`
  left: `"Error: Io(Os { code: 11, kind: WouldBlock, message: \"Resource temporarily unavailable\" })\r\n"`,
 right: `"/tmp/.tmpmJWT4L\r\n"`', tests/cli.rs:61:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- first_run_prompt_accept stdout ----
thread 'first_run_prompt_accept' panicked at 'assertion failed: ptn.find(&output).is_some()', tests/cli.rs:126:5

---- output_on_exit_without_cd stdout ----
thread 'output_on_exit_without_cd' panicked at 'assertion failed: `(left == right)`
  left: `"Error: Io(Os { code: 11, kind: WouldBlock, message: \"Resource temporarily unavailable\" })\r\n"`,
 right: `"tere: Exited without changing folder\r\n"`', tests/cli.rs:76:5

---- first_run_prompt_cancel stdout ----
thread 'first_run_prompt_cancel' panicked at 'assertion failed: ptn.find(&output).is_some()', tests/cli.rs:96:5

failures:
    basic_run
    first_run_prompt_accept
    first_run_prompt_cancel
    output_on_exit_without_cd

test result: FAILED. 0 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.12s

error: test failed, to rerun pass `--test cli`

I couldn't figure out why this is happening so I thought it'd better to report here.

As a workaround, I tried skipping the integration tests and using --lib but got error: no library targets found in package tere.

mgunyho commented 1 year ago

Huh, thanks for the report. So seems like that is coming from the stderr of the app, and not from any of the test code. Is it possible to run just the app in the chroot environment with something like

# so that the environment is similar to the tests
$ mkdir /tmp/test
$ cd /tmp/test
# The PWD can maybe left out? Not sure. But this is effectively what I do in the test.
$ PWD=/tmp/test /path/to/tere &> test-out.txt
<press ctrl-c or esc to exit if necessary>

$ PWD=/tmp/test /path/to/tere 2> test-out-stderr.txt 1> test-out-stdout.txt
<press ctrl-c or esc to exit if necessary>

Could you do this and tell me what's in the txt files, including control codes? Or what is the output of the app if you just run it without redirecting the output?

Another option is to try to add a test to tests/cli.rs that prints the full output without stripping the alternate screen:

#[test]
fn dummy() -> Result<(), RexpectError> {
    let mut proc = run_app_with_cmd(get_cmd_no_first_run_prompt());
    proc.send_control('c')?;
    proc.writer.flush()?;
    let output = proc.exp_eof()?;
    println!("{:?}", output);
    assert!(false);
}

And check the stdout of that, does it have some more info than "Error: Io(Os { code: 11, ... } )"

mgunyho commented 1 year ago

I searched for "chroot pty error", and found some stackoverflow posts like this one or this one suggesting that something related to /dev/pts has to be mounted in order to open a pty (which is what the integration test does), are you mounting that? I'm not familiar at all with chrooting.

orhun commented 1 year ago

Could you do this and tell me what's in the txt files, including control codes?

$ PWD=/tmp/test tere &> test-out.txt # exit code: 1

$ cat test-out.txt
Cancelled.
$ PWD=/tmp/test tere 2> test-out-stderr.txt 1> test-out-stdout.txt # exit code: 1

$ cat test-out-stderr.txt
Cancelled.

$ cat test-out-stdout.txt
# empty

Or what is the output of the app if you just run it without redirecting the output?

It runs normally.

Another option is to try to add a test to tests/cli.rs that prints the full output without stripping the alternate screen

Output ``` running 1 test test dummy ... FAILED failures: ---- dummy stdout ---- "\u{1b}[?25l\u{1b}[1;1H\u{1b}[2K\u{1b}[1;1H\u{1b}[0m\u{1b}[1m\u{1b}[4m/build/tere/src/tere-1.5.1\u{1b}[0m\u{1b}[1;1H\u{1b}[2K\u{1b}[1;1H\u{1b}[0m\u{1b}[1m\u{1b}[4m/build/tere/src/tere-1.5.1\u{1b}[0m\u{1b}[23;1H\u{1b}[2K\u{1b}[23;1H\u{1b}[0m\u{1b}[1mtere 1.5.1 - Type something to search, press '?' to view help or Esc to exit.\u{1b}[0m\u{1b}[24;1H\u{1b}[2K\u{1b}[24;26H\u{1b}[0m\u{1b}[1mgap search from start - smart case - sort:name - 2 / 13\u{1b}[0m\u{1b}[24;1H\u{1b}[0m\u{1b}[1msearch: \u{1b}[0m\u{1b}[2;1H\u{1b}[0m\u{1b}[0m\u{1b}[1m\u{1b}[24m\u{1b}[49m\u{1b}[39m.\u{1b}[24m\u{1b}[49m\u{1b}[39m.\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[3;1H\u{1b}[0m\u{1b}[0m\u{1b}[1m\u{1b}[24m\u{1b}[48;5;7m\u{1b}[38;5;0m.\u{1b}[24m\u{1b}[48;5;7m\u{1b}[38;5;0mc\u{1b}[24m\u{1b}[48;5;7m\u{1b}[38;5;0ma\u{1b}[24m\u{1b}[48;5;7m\u{1b}[38;5;0mr\u{1b}[24m\u{1b}[48;5;7m\u{1b}[38;5;0mg\u{1b}[24m\u{1b}[48;5;7m\u{1b}[38;5;0mo\u{1b}[0m\u{1b}[48;5;7m \u{1b}[0m\u{1b}[0m\u{1b}[4;1H\u{1b}[0m\u{1b}[0m\u{1b}[1m\u{1b}[24m\u{1b}[49m\u{1b}[39md\u{1b}[24m\u{1b}[49m\u{1b}[39me\u{1b}[24m\u{1b}[49m\u{1b}[39mm\u{1b}[24m\u{1b}[49m\u{1b}[39mo\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[5;1H\u{1b}[0m\u{1b}[0m\u{1b}[1m\u{1b}[24m\u{1b}[49m\u{1b}[39ms\u{1b}[24m\u{1b}[49m\u{1b}[39mr\u{1b}[24m\u{1b}[49m\u{1b}[39mc\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[6;1H\u{1b}[0m\u{1b}[0m\u{1b}[1m\u{1b}[24m\u{1b}[49m\u{1b}[39mt\u{1b}[24m\u{1b}[49m\u{1b}[39ma\u{1b}[24m\u{1b}[49m\u{1b}[39mr\u{1b}[24m\u{1b}[49m\u{1b}[39mg\u{1b}[24m\u{1b}[49m\u{1b}[39me\u{1b}[24m\u{1b}[49m\u{1b}[39mt\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[7;1H\u{1b}[0m\u{1b}[0m\u{1b}[1m\u{1b}[24m\u{1b}[49m\u{1b}[39mt\u{1b}[24m\u{1b}[49m\u{1b}[39me\u{1b}[24m\u{1b}[49m\u{1b}[39ms\u{1b}[24m\u{1b}[49m\u{1b}[39mt\u{1b}[24m\u{1b}[49m\u{1b}[39ms\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[8;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[24m\u{1b}[49m\u{1b}[39m.\u{1b}[24m\u{1b}[49m\u{1b}[39mg\u{1b}[24m\u{1b}[49m\u{1b}[39mi\u{1b}[24m\u{1b}[49m\u{1b}[39mt\u{1b}[24m\u{1b}[49m\u{1b}[39mi\u{1b}[24m\u{1b}[49m\u{1b}[39mg\u{1b}[24m\u{1b}[49m\u{1b}[39mn\u{1b}[24m\u{1b}[49m\u{1b}[39mo\u{1b}[24m\u{1b}[49m\u{1b}[39mr\u{1b}[24m\u{1b}[49m\u{1b}[39me\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[9;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[24m\u{1b}[49m\u{1b}[39mb\u{1b}[24m\u{1b}[49m\u{1b}[39mu\u{1b}[24m\u{1b}[49m\u{1b}[39mi\u{1b}[24m\u{1b}[49m\u{1b}[39ml\u{1b}[24m\u{1b}[49m\u{1b}[39md\u{1b}[24m\u{1b}[49m\u{1b}[39m-\u{1b}[24m\u{1b}[49m\u{1b}[39mr\u{1b}[24m\u{1b}[49m\u{1b}[39me\u{1b}[24m\u{1b}[49m\u{1b}[39ml\u{1b}[24m\u{1b}[49m\u{1b}[39me\u{1b}[24m\u{1b}[49m\u{1b}[39ma\u{1b}[24m\u{1b}[49m\u{1b}[39ms\u{1b}[24m\u{1b}[49m\u{1b}[39me\u{1b}[24m\u{1b}[49m\u{1b}[39m.\u{1b}[24m\u{1b}[49m\u{1b}[39ms\u{1b}[24m\u{1b}[49m\u{1b}[39mh\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[10;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[24m\u{1b}[49m\u{1b}[39mC\u{1b}[24m\u{1b}[49m\u{1b}[39ma\u{1b}[24m\u{1b}[49m\u{1b}[39mr\u{1b}[24m\u{1b}[49m\u{1b}[39mg\u{1b}[24m\u{1b}[49m\u{1b}[39mo\u{1b}[24m\u{1b}[49m\u{1b}[39m.\u{1b}[24m\u{1b}[49m\u{1b}[39ml\u{1b}[24m\u{1b}[49m\u{1b}[39mo\u{1b}[24m\u{1b}[49m\u{1b}[39mc\u{1b}[24m\u{1b}[49m\u{1b}[39mk\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[11;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[24m\u{1b}[49m\u{1b}[39mC\u{1b}[24m\u{1b}[49m\u{1b}[39ma\u{1b}[24m\u{1b}[49m\u{1b}[39mr\u{1b}[24m\u{1b}[49m\u{1b}[39mg\u{1b}[24m\u{1b}[49m\u{1b}[39mo\u{1b}[24m\u{1b}[49m\u{1b}[39m.\u{1b}[24m\u{1b}[49m\u{1b}[39mt\u{1b}[24m\u{1b}[49m\u{1b}[39mo\u{1b}[24m\u{1b}[49m\u{1b}[39mm\u{1b}[24m\u{1b}[49m\u{1b}[39ml\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[12;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[24m\u{1b}[49m\u{1b}[39mC\u{1b}[24m\u{1b}[49m\u{1b}[39mH\u{1b}[24m\u{1b}[49m\u{1b}[39mA\u{1b}[24m\u{1b}[49m\u{1b}[39mN\u{1b}[24m\u{1b}[49m\u{1b}[39mG\u{1b}[24m\u{1b}[49m\u{1b}[39mE\u{1b}[24m\u{1b}[49m\u{1b}[39mL\u{1b}[24m\u{1b}[49m\u{1b}[39mO\u{1b}[24m\u{1b}[49m\u{1b}[39mG\u{1b}[24m\u{1b}[49m\u{1b}[39m.\u{1b}[24m\u{1b}[49m\u{1b}[39mm\u{1b}[24m\u{1b}[49m\u{1b}[39md\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[13;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[24m\u{1b}[49m\u{1b}[39mL\u{1b}[24m\u{1b}[49m\u{1b}[39mI\u{1b}[24m\u{1b}[49m\u{1b}[39mC\u{1b}[24m\u{1b}[49m\u{1b}[39mE\u{1b}[24m\u{1b}[49m\u{1b}[39mN\u{1b}[24m\u{1b}[49m\u{1b}[39mS\u{1b}[24m\u{1b}[49m\u{1b}[39mE\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[14;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[24m\u{1b}[49m\u{1b}[39mR\u{1b}[24m\u{1b}[49m\u{1b}[39mE\u{1b}[24m\u{1b}[49m\u{1b}[39mA\u{1b}[24m\u{1b}[49m\u{1b}[39mD\u{1b}[24m\u{1b}[49m\u{1b}[39mM\u{1b}[24m\u{1b}[49m\u{1b}[39mE\u{1b}[24m\u{1b}[49m\u{1b}[39m.\u{1b}[24m\u{1b}[49m\u{1b}[39mm\u{1b}[24m\u{1b}[49m\u{1b}[39md\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[15;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[16;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[17;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[18;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[19;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[20;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[21;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[22;1H\u{1b}[0m\u{1b}[0m\u{1b}[2m\u{1b}[0m\u{1b}[0m\u{1b}[K\u{1b}[0m\u{1b}[0m\u{1b}[?25h\u{1b}[?1049ltere: Exited without changing folder\r\n" thread 'dummy' panicked at 'assertion failed: false', tests/cli.rs:147:5 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace failures: dummy test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 4 filtered out; finished in 0.22s ```

/dev/pts has to be mounted in order to open a pty

Ah, good catch. However, it is mounted inside chroot:

$ findmnt

├─/dev                                tmpfs                                                             tmpfs   rw,nosuid,size=4096k,nr_inodes=65536,mode=755,inode64
│ ├─/dev/shm                          tmpfs                                                             tmpfs   rw,nosuid,nodev,size=26378176k,nr_inodes=409600,inode64
│ ├─/dev/pts                          devpts                                                            devpts  rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=666
mgunyho commented 1 year ago

Thanks for testing this! I'll look into it. One thing to note that cat test-out-stderr.txt will not show the control codes, since the output is like <control code to switch to alternate screen><a bunch of drawing commands><switch back to regular screen>Cancelled. cat will print to the terminal, which will obey the control codes, so you won't see what was on the alt screen. So I would need to see the actual txt file contents (you can see the control codes by opening it in vim for example). But I think I can debug this based on the print in the dummy test.

mgunyho commented 1 year ago

I searched a bit more and found some discussion suggesting that this error may show up in Rust if the thread limit is reached or there isn't enough memory. Is it possible that the no. of threads in chroot mode is limited? Does the error go away if you run cargo test -j 1 ?

mgunyho commented 1 year ago

Also, what is the output of the failing test if you change this line to assert!(ptn.find(&output).is_some(), "{:?}", output); ?

orhun commented 1 year ago

cargo test -j 1

This time I got:

running 4 tests
test basic_run ... FAILED
test first_run_prompt_cancel ... FAILED
test output_on_exit_without_cd ... FAILED
test first_run_prompt_accept ... FAILED

failures:

---- basic_run stdout ----
thread 'basic_run' panicked at 'assertion failed: `(left == right)`
  left: `"Error: Io(Os { code: 2, kind: NotFound, message: \"No such file or directory\" })\r\n"`,
 right: `"/tmp/.tmpZs2oTz\r\n"`', tests/cli.rs:61:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- first_run_prompt_cancel stdout ----
thread 'first_run_prompt_cancel' panicked at 'assertion failed: ptn.find(&output).is_some()', tests/cli.rs:96:5

---- output_on_exit_without_cd stdout ----
thread 'output_on_exit_without_cd' panicked at 'assertion failed: `(left == right)`
  left: `"Error: Io(Os { code: 2, kind: NotFound, message: \"No such file or directory\" })\r\n"`,
 right: `"tere: Exited without changing folder\r\n"`', tests/cli.rs:76:5

---- first_run_prompt_accept stdout ----
thread 'first_run_prompt_accept' panicked at 'assertion failed: ptn.find(&output).is_some()', tests/cli.rs:126:5

failures:
    basic_run
    first_run_prompt_accept
    first_run_prompt_cancel
    output_on_exit_without_cd

test result: FAILED. 0 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.16s

error: test failed, to rerun pass `--test cli`

Also, what is the output of the failing test if you change this line

cargo test:

failures:

---- panic_guard::tests::test_nested_callback_hook_restored stdout ----
thread 'panic_guard::tests::test_nested_callback_hook_restored' panicked at 'assertion failed: `(left == right)`
  left: `["inner inner cleanup", "inner cleanup", "outer hook", "outer hook", "outer hook"]`,
 right: `["inner inner cleanup", "inner cleanup", "outer hook"]`', src/panic_guard.rs:214:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- panic_guard::tests::test_nested_callback_with_panic stdout ----
thread 'panic_guard::tests::test_nested_callback_with_panic' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:161:36

---- panic_guard::tests::test_callback_called_once_only_panic stdout ----
thread 'panic_guard::tests::test_callback_called_once_only_panic' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:99:36

---- panic_guard::tests::test_callback_called_on_drop stdout ----
thread 'panic_guard::tests::test_callback_called_on_drop' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:82:36

---- panic_guard::tests::test_callback_called_before_panic_hook stdout ----
thread 'panic_guard::tests::test_callback_called_before_panic_hook' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:119:36

failures:
    panic_guard::tests::test_callback_called_before_panic_hook
    panic_guard::tests::test_callback_called_on_drop
    panic_guard::tests::test_callback_called_once_only_panic
    panic_guard::tests::test_nested_callback_hook_restored
    panic_guard::tests::test_nested_callback_with_panic

test result: FAILED. 86 passed; 5 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.13s

cargo test -j 1:

failures:

---- panic_guard::tests::test_callback_called_before_panic_hook stdout ----
thread 'panic_guard::tests::test_callback_called_before_panic_hook' panicked at 'assertion failed: `(left == right)`
  left: `["cleanup", "hook", "hook", "hook"]`,
 right: `["cleanup", "hook"]`', src/panic_guard.rs:135:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- panic_guard::tests::test_callback_called_on_drop stdout ----
thread 'panic_guard::tests::test_callback_called_on_drop' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:82:36

---- panic_guard::tests::test_callback_called_once_only_panic stdout ----
thread 'panic_guard::tests::test_callback_called_once_only_panic' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:99:36

---- panic_guard::tests::test_nested_callback stdout ----
thread 'panic_guard::tests::test_nested_callback' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:140:36

---- panic_guard::tests::test_nested_callback_with_panic stdout ----
thread 'panic_guard::tests::test_nested_callback_with_panic' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:161:36

---- panic_guard::tests::test_nested_callback_hook_restored stdout ----
thread 'panic_guard::tests::test_nested_callback_hook_restored' panicked at 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }', src/panic_guard.rs:180:36

failures:
    panic_guard::tests::test_callback_called_before_panic_hook
    panic_guard::tests::test_callback_called_on_drop
    panic_guard::tests::test_callback_called_once_only_panic
    panic_guard::tests::test_nested_callback
    panic_guard::tests::test_nested_callback_hook_restored
    panic_guard::tests::test_nested_callback_with_panic

test result: FAILED. 85 passed; 6 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.20s
mgunyho commented 1 year ago

Huh, where is that "No such file or directory" error now suddenly coming from? Do you reliably get the results you had previously (WouldBlock error) without -j 1 and then this NotFoundError with -j 1? Can you add println!("{:?}", output); to line 59 of tests/cli.rs (before the strip_until_alternate_screen_exit) and show me the result with -j1 and without it? I am really at a loss here.

The second set of results you have with the panic_guard errors is unrelated to this, it's the same as #90 (which I still can't reliably reproduce and also have no idea how to fix). Are you sure you ran the CLI integration tests? Or maybe those are not run if there are failures in the unit tests?

orhun commented 9 months ago

Hey, I just took a look at this again and I'm totally lost as well. I don't remember the last thing that I tried but if I try to build again I get this failure:

---- app_state::tests::test_case_sensitive_mode_change stdout ----
thread 'app_state::tests::test_case_sensitive_mode_change' panicked at src/app_state.rs:1711:9:
assertion `left == right` failed
  left: [2]
 right: [1]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    app_state::tests::test_case_sensitive_mode_change

I think I'm just going to close my eyes and keep skipping tests. Sorry for this extremely annoying issue 😅

mgunyho commented 9 months ago

No worries. I thought about the async thing a little bit too but didn't get anywhere. However! Now I tried the tests as well and the same test fails for me, and it seems reproducible. So that's probably a real test failure.

mgunyho commented 9 months ago

I did a bisect and that test fails for me for all commits after reworking the tests to use real folders instead of just a list of strings in 6d7eb28d00277bc6239fb5c8960a4efa373efe76 and c90c49783b02f2abc57e2d51ec05844e0de286fc. I'm pretty sure I would have fixed this if it had shown up. So something has changed, maybe something with the compiler version or dependencies, both of which are kind of worrisome.

mgunyho commented 9 months ago

I now fixed it in 8cadf56, this app_state::tests::test_case_sensitive_mode_change shouldn't fail anymore. I'm not really sure what was going on there. I'm using a new computer and new distro compared to when I initially wrote it, so that might have something to do with it as well? But looking at the test, it did have a logic error. Hard to say. But thanks anyway for helping out!

ProducerMatt commented 6 months ago

NixOS updates finally hit this problem. I couldn't see any broken functionality so I commented out the failing tests. https://github.com/NixOS/nixpkgs/issues/298489

mgunyho commented 6 months ago

Thanks @ProducerMatt for the report! Which version of rustc/cargo do you have on Nix? I still can't reproduce this on Fedora 38 with rustc 1.75.0. With Nix it sounds like I might be able to reproduce this issue. Is there a way to set up a minimal environment without installing the full NixOS?

ProducerMatt commented 6 months ago

@mgunyho yes in fact, Nix runs on other Linux and MacOS. I will submit a PR with a Nix dev environment and how to use it. :)

ProducerMatt commented 6 months ago

Nix environment submitted. #100

mgunyho commented 5 months ago

With the help of @ProducerMatt's nix environment in #100, I managed to track down the issue.

It's the terminal_size_usize function, which calls this function from the crossterm library:


    let file = File::open("/dev/tty").map(|file| (FileDesc::new(file.into_raw_fd(), true)));
    let fd = if let Ok(file) = &file {
        file.raw_fd()
    } else {
        // Fallback to libc::STDOUT_FILENO if /dev/tty is missing
        STDOUT_FILENO
    };

    if wrap_with_result(unsafe { ioctl(fd, TIOCGWINSZ.into(), &mut size) }).is_ok() {
        return Ok(size.into());
    }

    Err(std::io::Error::last_os_error().into())

The first part sets fd either to the file descriptor of /dev/tty or stdout. Based on my testing in the Nix environment, File::open("/dev/tty") gives Err(Os { code: 6, kind: Uncategorized, message: "No such device or address" }), so it will fall back to stdout.

After this, it calls ioctl(fd, TIOCGWINSZ.into(), ...) where I believe the not very descriptive "File not found" error comes from.

So the problem is querying the "terminal size" inside the Nix environment / chroot. The next step would be to flood my search engine with queries like "nix environment tty" and similar.

orhun commented 5 months ago

Nice! I'm glad we now know the root cause of the issue. FWIW there are libraries such as faketty which might be helpful in this case.

mgunyho commented 5 months ago

@orhun Could you try to run the tests with script -c "cargo test" instead of just cargo test in the chroot? That might make them pass, but I'm not entirely sure if it's a good solution.

orhun commented 5 months ago

Good tip! I only get a single failure now:

failures:

---- app_state::tests::test_case_sensitive_mode_change stdout ----
thread 'app_state::tests::test_case_sensitive_mode_change' panicked at src/app_state.rs:1711:9:
assertion `left == right` failed
  left: [2]
 right: [1]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    app_state::tests::test_case_sensitive_mode_change

test result: FAILED. 90 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.01s

error: test failed, to rerun pass `--bin tere`

It might be better to use -e flag to use the exit code of the process as well.

mgunyho commented 5 months ago

That is actually another failure that has been fixed on the develop branch, and prevents the integration tests from running. Could you please retry on develop?

BTW, based on the readme of faketty, it does something very similar to script so that could be a solution as well.

mgunyho commented 5 months ago

I'm starting to think that it would be easiest to just skip the integration tests for now, this can be done by changing the test command to cargo test --bins.

orhun commented 5 months ago

Everything is good with the following command:

$ script -e -c "cargo test --frozen -- --skip test_case_sensitive_mode_change"

I didn't test develop branch yet but I'm assuming we don't need to skip it with the new release.

Using faketty in tests would be nice to avoid script command I think.

cargo test --bins

This also makes sense to me.

mgunyho commented 5 months ago

This is with the chroot? Good to hear if it works. faketty seems basically like a modern version of script, but on the other hand script might be more of a standard utility that is readily available, faketty would need to be added as a build dependency.

orhun commented 5 months ago

Yup, that's inside chroot!

I can take a stab at doing this with faketty if you want 🐻

mgunyho commented 5 months ago

Alright, sounds good! You can pick whichever of script or faketty is easier for you to integrate to the CI pipeline of Arch.

mgunyho commented 5 months ago

Okay, I found a solution that doesn't rely on faketty - seems like the integration test has a hidden dependency on tput (provided by ncurses package on nix, not sure about Arch), see https://github.com/mgunyho/tere/pull/100#issuecomment-2029618331. Could you @orhun try again without script/faketty and with tput available in the chroot?

orhun commented 5 months ago

Wow, nice discovery. And good news, yes that worked like a charm! 🎉

No script, just added ncurses to checkdepends and voila!

mgunyho commented 5 months ago

Alright, cool! I'll close this issue for now, let me know if anything else comes up. We can remove the ncurses dependency once the tput fallback is removed from crossterm, although I guess it's not much of a problem.

orhun commented 5 months ago

Thanks for bearing with me on this!