Skip to content

stty: don't depend on the system stty in test_saved_state_valid_formats - #15346

Closed
ozenyunus35 wants to merge 6 commits into
uutils:mainfrom
ozenyunus35:stty-test-no-system-stty
Closed

ozenyunus35 wants to merge 6 commits into
uutils:mainfrom
ozenyunus35:stty-test-no-system-stty

Conversation

@ozenyunus35

Copy link
Copy Markdown
Contributor

Closes #15311

The saved state in test_saved_state_valid_formats came from the system (GNU) stty, so the test returned early wherever GNU stty isn't installed and otherwise depended on its output.

It now generates the state with our own stty -g, checks that restoring it succeeds with no output, and that stty -g afterwards gives the same state back. The comparison with GNU stty stays as an extra check when it's available.

I used an AI assistant to help draft parts of the code and tests; I reviewed the changes and ran the tests myself.

The saved state was generated with the system (GNU) stty, so the test
returned early where GNU stty isn't available and otherwise depended on
its output. Generate it with our own stty -g, check that restoring it
prints nothing and leaves the state unchanged, and keep the GNU comparison
as an extra check when GNU stty is available.

Closes uutils#15311
Comment thread tests/by-util/test_stty.rs Outdated
result.success().no_stderr();
result.success().no_output();

// Restoring the state leaves the terminal as it was

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this restores the state that is already set, so it can't really catch much. could you change a setting first, then restore and compare?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. It now sets -echo first, checks the saved state changed, then restores and compares.

@xtqqczze

Copy link
Copy Markdown
Collaborator

Needs to be rebased on 41f705b to compile.

@ozenyunus35

Copy link
Copy Markdown
Contributor Author

Thanks! I merged main and 41f705b into the branch instead of rebasing, to avoid a force-push. It builds now and the stty tests pass locally.

@xtqqczze

Copy link
Copy Markdown
Collaborator

@ozenyunus35 please rebase, we prefer a clean branch history

@ozenyunus35

Copy link
Copy Markdown
Contributor Author

Done: I opened a new PR with the change rebased onto 41f705b as a single commit (it links back here), since I'd rather not force-push. Closing this one in favour of it.

@xtqqczze xtqqczze reopened this Oct 15, 2026
@xtqqczze

Copy link
Copy Markdown
Collaborator

@ozenyunus35 Force-pushing to the PR branch is expected when necessary to update it, including after rebasing.

@ozenyunus35

Copy link
Copy Markdown
Contributor Author

Thanks, noted.

@ozenyunus35

Copy link
Copy Markdown
Contributor Author

Closing this, as I can't follow up on it further. Thanks for the reviews.

@xtqqczze xtqqczze self-assigned this Oct 15, 2026
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.

stty: Some Rust native test depends on system (GNU) coreutils

3 participants