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.

Sign up for GitHub

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
+322 −16

Conversation

JJJollyjim
Copy link
Member

@JJJollyjim JJJollyjim commented Nov 13, 2020 •
edited
Loading

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 👍

Sorry, something went wrong.

@JJJollyjim JJJollyjim requested a review from ttuegel as a code owner November 13, 2020 19:09
@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 Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: qt/kde 6.topic: xfce The Xfce Desktop Environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 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! 👍

@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 11, 2020
@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.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 1, 2021
@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
nixos/tests: add integration tests for terminal emulators

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
842e547
@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.

@marvin-mk2 marvin-mk2 bot added marvin This PR was reviewed by Marvin, a discontinued bot: https://github.com/timokau/marvin-mk2 needs_reviewer (old Marvin label, do not use) labels Mar 18, 2022
@marvin-mk2 marvin-mk2 bot requested a review from SuperSandro2000 March 18, 2022 07:20
@marvin-mk2 marvin-mk2 bot added awaiting_reviewer (old Marvin label, do not use) and removed needs_reviewer (old Marvin label, do not use) labels Mar 18, 2022
@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

@ttuegel ttuegel

@jtojnar jtojnar

@hedning hedning

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: qt/kde 6.topic: xfce The Xfce Desktop Environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux awaiting_reviewer (old Marvin label, do not use) marvin This PR was reviewed by Marvin, a discontinued bot: https://github.com/timokau/marvin-mk2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JJJollyjim @SuperSandro2000 @ttuegel