Skip to content

Conversation

@naoNao89
Copy link
Contributor

Working on issue #1857 about nohup test coverage.

What's here:

  • Added 6 tests covering the main code paths that weren't tested
  • Tests validate output file creation, HOME fallback, stderr redirection, and exit codes

The uncovered lines:
Line 119 (and similar at L110, L124) are dup2() syscall error paths. These only trigger if the kernel fails to duplicate file descriptors - basically unreachable without fault injection tools. I documented this in the coverage analysis.

The remaining uncovered lines are defensive error handling that's realistically untestable in the current test environment. IMO they should stay as-is - better to have defensive code than risk removing safety checks.

Add comprehensive tests for nohup's file descriptor replacement logic
and error handling, increasing coverage of previously untested code paths.

New tests cover:
- Creating nohup.out in current directory
- Appending to existing nohup.out
- Stderr redirection to stdout
- Command not found error (exit codes 126/127)
- Fallback to $HOME/nohup.out when cwd is not writable (Linux/FreeBSD)
- POSIXLY_CORRECT environment variable handling (Linux/FreeBSD)

Tests use .terminal_simulation(true) to properly test TTY-dependent
behavior. Platform-specific tests are gated behind appropriate cfg
attributes.

Addresses uutils#1857
@naoNao89 naoNao89 force-pushed the test/nohup-coverage-1857 branch from 96fde9e to 361b517 Compare October 15, 2025 23:49
- Add 'nowrite' to cspell jargon dictionary
- Fix clippy::uninlined_format_args warnings in test_nohup.rs
- Remove nested #[test] function that caused unnameable-test-items error
- Format strings now use inline variable syntax
@naoNao89 naoNao89 force-pushed the test/nohup-coverage-1857 branch from e0ce292 to e6892ce Compare October 16, 2025 00:00
@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

@sylvestre
Copy link
Contributor

nice!

[codecov/project](https://github.com/uutils/coreutils/pull/8920/checks?check_run_id=52864993020)Successful in 1s — 86.61% (+0.04%) compared to 8d59e08

@sylvestre sylvestre merged commit af76671 into uutils:main Oct 16, 2025
121 checks passed
@naoNao89 naoNao89 deleted the test/nohup-coverage-1857 branch October 16, 2025 02:18
naoNao89 added a commit to naoNao89/coreutils that referenced this pull request Oct 25, 2025
…ils#8920)

* test(nohup): expand test coverage for fd replacement and error paths

Add comprehensive tests for nohup's file descriptor replacement logic
and error handling, increasing coverage of previously untested code paths.

New tests cover:
- Creating nohup.out in current directory
- Appending to existing nohup.out
- Stderr redirection to stdout
- Command not found error (exit codes 126/127)
- Fallback to $HOME/nohup.out when cwd is not writable (Linux/FreeBSD)
- POSIXLY_CORRECT environment variable handling (Linux/FreeBSD)

Tests use .terminal_simulation(true) to properly test TTY-dependent
behavior. Platform-specific tests are gated behind appropriate cfg
attributes.

Addresses uutils#1857

* fix: CI failures for cspell and clippy in nohup tests

- Add 'nowrite' to cspell jargon dictionary
- Fix clippy::uninlined_format_args warnings in test_nohup.rs
- Remove nested #[test] function that caused unnameable-test-items error
- Format strings now use inline variable syntax
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.

2 participants