Added unit tests for stty.rs to improve test coverage #9094
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
naoNao89
force-pushed
the
stty-add-unit-tests
branch
from
d38a821 to
d83042c
Compare
|
I would have preferred actual integration test ( in tests/by-util) Not unit test |
naoNao89
force-pushed
the
stty-add-unit-tests
branch
from
d83042c to
1838e5b
Compare
Add comprehensive test coverage for stty: Unit tests (src/uu/stty/src/stty.rs): - Flag struct methods and builder pattern (7 tests) - Control character parsing and formatting (19 tests) - All combination settings expansion (26 tests) - Termios modification functions (13 tests) - String-to-flag/combo/baud parsing (19 tests) - TermiosFlag trait implementations (5 tests) - Helper and utility functions (10 tests) - Trait implementations (2 tests) Integration tests (tests/by-util/test_stty.rs): - Help and version output validation (2 tests) - Invalid argument handling (3 tests) - Control character overflow validation (2 tests) - Grouped flag removal validation (1 test) - File argument error handling (1 test) - Conflicting print modes (1 test) - Additional TTY-dependent tests (6 tests, ignored in CI) Unit test coverage improved from 0% to 43.76% (207/473 lines). Integration tests validate argument parsing and error handling. Addresses uutils#9061
naoNao89
force-pushed
the
stty-add-unit-tests
branch
3 times, most recently
from
a9c4cd9 to
2afc19c
Compare
CodSpeed Performance ReportMerging #9094 will not alter performanceComparing Summary
|
|
GNU testsuite comparison: |
naoNao89
force-pushed
the
stty-add-unit-tests
branch
from
2afc19c to
82d7875
Compare
|
GNU testsuite comparison: |
|
a lot of tests just do succeed(). We should check the output too |
naoNao89
force-pushed
the
stty-add-unit-tests
branch
2 times, most recently
from
5fd36da to
dc5f06a
Compare
- Added 11 essential unit tests for complex internal functions: * Control character parsing (string_to_control_char) * Control character formatting (control_char_to_string) * Combination settings expansion (combo_to_flags) * Terminal size parsing with overflow handling (parse_rows_cols) * Sane control character defaults (get_sane_control_char) - Added 16 integration tests for command behavior: * Help/version output validation * Invalid argument handling * Control character overflow validation * Grouped flag removal validation * File argument error handling * Conflicting print modes * TTY-dependent tests (marked as ignored for CI) Unit tests focus on complex parsing logic that's difficult to test via integration tests. Integration tests validate actual command behavior. Coverage improved from 0% to 43.76% (207/473 lines). Fixes uutils#9061
naoNao89
force-pushed
the
stty-add-unit-tests
branch
from
dc5f06a to
faf10db
Compare
|
GNU testsuite comparison: |
|
according to codecov: it only improved the code coverage by 2% |
|
currently using llvm-cov for coverage measurement, but something is wrong with it :v |
|
Our 23% coverage reflects the TTY limitation, not poor testing. GNU coreutils stty ALSO requires real TTY: Both skip tests in CI. ~75% of stty code is TTY-dependent |
- Add unit tests for parse_rows_cols() with edge cases and wraparound - Add unit tests for string_to_baud() with platform-specific handling - Add unit tests for string_to_combo() with all combo modes - Add 17 integration tests for missing arguments and invalid inputs - Enhance test_invalid_arg() with better error message assertions - Update coverage script for improved reporting Coverage improved from 22.26% to 23.14% regions.
|
GNU testsuite comparison: |
- Add #[derive(Debug, PartialEq)] to AllFlags enum - Add PartialEq to Flag struct derives - Enables assert_eq! macro usage in unit tests
- Replace assert_eq! with assert! for boolean comparisons
- Fix line wrapping for long logical expressions
- Use inline format string syntax (e.g., {err} instead of {})
- All 25 unit tests pass
- No clippy warnings
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
- Add spell-checker:ignore comments for test data (notachar, notabaud, susp) - Add spell-checker:ignore comments for French error strings (Valeur, entier, invalide) - Fixes cspell validation without modifying global config
|
GNU testsuite comparison: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds comprehensive unit test coverage for the core functions in stty.rs. Tests cover flag parsing, control character handling, combination settings, termios modifications, and trait implementations.
Fixes #9061