Navigation Menu

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

Jump to bottom

nixos/tests: add integration tests for terminal emulators #103733

Merged
merged 4 commits into from Mar 20, 2022
Merged

nixos/tests: add integration tests for terminal emulators #103733

merged 4 commits into from Mar 20, 2022

Conversation

JJJollyjim
Copy link
Member

@JJJollyjim JJJollyjim commented Nov 13, 2020 •
edited

Motivation for this change

The terminal emulator is a pretty key program for a lot of (most?) desktop users, so it seems nice to have some certainty that they work before updating your system.

Terminal emulators all present a pretty similar interface, which gives us an opportunity to easily test their basic functionality with a single codebase.

My goal here was consistency -- I believe these tests should strike a good balance of thoroughness and simplicity, by not doing text input to the terminals, thereby avoiding much possibility of race conditions or focus issues.

There are two tests run on each terminal emulator

  • can it successfully execute a command passed on the cmdline?
  • can it successfully display a colour?

The latter is used as a proxy for "can it display text?", without going through all the intricacies of OCR.

256-colour terminal mode is used to display the test colour, since it has a universally-applicable palette (unlike 8- and 16- colour, where the colours are implementation-defined), and it is widely supported (unlike 24-bit colour).

I definitely don't have coverage of every terminal emulator -- I will address these in future PRs

Things done
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)

(note: termite fails to build pending #103704) fixed 👍

@JJJollyjim JJJollyjim changed the title Term tests nixos/tests: add integration tests for terminal emulators Nov 13, 2020
@ofborg ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 6.topic: qt/kde 6.topic: xfce The Xfce Desktop Environment 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Nov 13, 2020
@SuperSandro2000
Copy link
Member

Can you solve the merge conflicts?

@ttuegel
Copy link
Member

ttuegel commented Dec 5, 2020

This is a cool idea! I don't feel comfortable reviewing the entire pull request, but for my part: the changes to Konsole look good! 👍

@stale
Copy link

stale bot commented Jun 9, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 9, 2021
@ttuegel
Copy link
Member

ttuegel commented Aug 7, 2021

This is still very useful, just a little stalled.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 7, 2021
@JJJollyjim
Copy link
Member Author

Done! Merge conflicts resolved, references to renamed packages fixed, python import errors fixed, all tests pass.

@JJJollyjim
Copy link
Member Author

Ready for merging if you're willing @ttuegel @SuperSandro2000

@JJJollyjim
Copy link
Member Author

@SuperSandro2000 I've rebased this and added several remaining missing emulators.

@JJJollyjim
Copy link
Member Author

/marvin opt-in
/status needs_reviewer

@marvin-mk2
Copy link

marvin-mk2 bot commented Mar 18, 2022

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

@JJJollyjim JJJollyjim mentioned this pull request Mar 20, 2022
Merged
13 tasks
@SuperSandro2000
Copy link
Member

Please fix those two little things and then I am just going to merge this and hope for the best.

@JJJollyjim
nixos/tests: add passthru.tests to all tested terminal emulators
6948623
@JJJollyjim
vte: add tested vte-based terminals to passthru.tests
2708692
@JJJollyjim
nixos/tests/terminal-emulators: fix test for st
c9e1475 
some change in the last 24 hours altered the behaviour of st such that
it now dies with a non-zero exit code when the shell exits, so kill is
now necessary
@JJJollyjim
Copy link
Member Author

@SuperSandro2000 updated the commit with those changes incorporated, cheers

@SuperSandro2000
Copy link
Member

Here goes nothing! Thanks.

@SuperSandro2000 SuperSandro2000 merged commit bf78375 into NixOS:master Mar 20, 2022
@JJJollyjim JJJollyjim mentioned this pull request Mar 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@ttuegel ttuegel Awaiting requested review from ttuegel ttuegel is a code owner

@jtojnar jtojnar Awaiting requested review from jtojnar jtojnar is a code owner

@hedning hedning Awaiting requested review from hedning

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 6.topic: qt/kde 6.topic: xfce The Xfce Desktop Environment 10.rebuild-darwin: 0 10.rebuild-linux: 0 awaiting_reviewer marvin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JJJollyjim @SuperSandro2000 @ttuegel