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/i3wm: fix terminal title (test is broken on master) #51792

Merged
merged 2 commits into from Dec 9, 2018
Merged

nixos/tests/i3wm: fix terminal title (test is broken on master) #51792

merged 2 commits into from Dec 9, 2018

Conversation

hedning
Copy link
Contributor

@hedning hedning commented Dec 9, 2018

Motivation for this change

#51678 changed terminal titles, breaking the i3wm test. For some reason the gnome3 test is okay even though it looks for the same window title.

cc @yegortimoshenko who did the bash title change

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@hedning
nixos/tests/i3wm: fix terminal title
373be82 
Probably due to NixOS#51678 which makes bash set the terminal title.
@hedning
Copy link
Contributor Author

hedning commented Dec 9, 2018

I can update the gnome3 test if it's wanted. I'm guessing it works due to gnome-terminal starting up with Terminal as a title and takes long enough to change for the waitForWindow to pick it up.

@lukateras
Copy link
Member

lukateras commented Dec 9, 2018 •
edited

Sorry :-(

nixpkgs/nixos/tests/gnome3.nix

Line 36 in 373be82

$machine->waitForWindow(qr/Terminal/);

Agree that GNOME test should be updated:

$machine->sleep(2);
$machine->waitForWindow(qr/alice.*machine/);

@hedning
nixos/tests/gnome3: fix terminal title
80fdafb 
The tests passes, but that's just because a race condition where the window is
titled `Terminal` long enough.
@hedning
Copy link
Contributor Author

hedning commented Dec 9, 2018

Well it's a bit weird that changing the bash title would break desktop manager tests 😆 Would be nice getting it decoupled in the long run. Perhaps getting bash to use a predictable title in the tests would be good?

Took a look at the other waitForWindow tests and xfce and plasma should be fine, they both contain the correct strings. The gnome3-gdm test will be fixed by #44497 so I'd prefer not making a merge conflict for no reason.

@lukateras lukateras merged commit b0a4013 into NixOS:master Dec 9, 2018
@lukateras
Copy link
Member

Thank you!

@hedning hedning deleted the fix-i3wm-test branch October 15, 2019 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukateras lukateras lukateras approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hedning @lukateras @GrahamcOfBorg