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.

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

Integration test failures during Arch Linux build #93

Closed
orhun opened this issue Aug 25, 2023 · 30 comments
Closed

Integration test failures during Arch Linux build #93

orhun opened this issue Aug 25, 2023 · 30 comments

Comments

@orhun
Copy link
Contributor

orhun commented Aug 25, 2023

Integration tests were added in the last release, more specifically: 70d2686

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
Copy link
Owner

mgunyho commented Aug 25, 2023

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
Copy link
Owner

mgunyho commented Aug 25, 2023

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
Copy link
Contributor Author

orhun commented Sep 5, 2023

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
Copy link
Owner

mgunyho commented Sep 6, 2023

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
Copy link
Owner

mgunyho commented Sep 14, 2023

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
Copy link
Owner

mgunyho commented Sep 14, 2023

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

@orhun
Copy link
Contributor Author

orhun commented Sep 16, 2023

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
Copy link
Owner

mgunyho commented Sep 17, 2023

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
Copy link
Contributor Author

orhun commented Dec 21, 2023

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
Copy link
Owner

mgunyho commented Dec 21, 2023

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
Copy link
Owner

mgunyho commented Dec 21, 2023

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 6d7eb28 and c90c497. 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 added a commit that referenced this issue Dec 21, 2023
Not sure why it was broken in the first place...

See also discussion in Github issue #93
@mgunyho
Copy link
Owner

mgunyho commented Dec 21, 2023

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
Copy link
Contributor

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

@mgunyho
Copy link
Owner

mgunyho commented Mar 24, 2024

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
Copy link
Contributor

@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
Copy link
Contributor

Nix environment submitted. #100

@mgunyho
Copy link
Owner

mgunyho commented Mar 27, 2024

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
Copy link
Contributor Author

orhun commented Mar 27, 2024

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
Copy link
Owner

mgunyho commented Mar 30, 2024

@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
Copy link
Contributor Author

orhun commented Mar 30, 2024

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
Copy link
Owner

mgunyho commented Mar 31, 2024

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
Copy link
Owner

mgunyho commented Mar 31, 2024

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
Copy link
Contributor Author

orhun commented Mar 31, 2024

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
Copy link
Owner

mgunyho commented Mar 31, 2024

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
Copy link
Contributor Author

orhun commented Mar 31, 2024

Yup, that's inside chroot!

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

@mgunyho
Copy link
Owner

mgunyho commented Apr 1, 2024

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

@mgunyho
Copy link
Owner

mgunyho commented Apr 1, 2024

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 #100 (comment). Could you @orhun try again without script/faketty and with tput available in the chroot?

@orhun
Copy link
Contributor Author

orhun commented Apr 5, 2024

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

No script, just added ncurses to checkdepends and voila!

@mgunyho
Copy link
Owner

mgunyho commented Apr 5, 2024

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.

@mgunyho mgunyho closed this as completed Apr 5, 2024
@orhun
Copy link
Contributor Author

orhun commented Apr 5, 2024

Thanks for bearing with me on this!

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

No branches or pull requests

3 participants