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

nixos/tests/keymap: Workaround xterm not in focus #33719

Closed
wants to merge 1 commit into from

Conversation

srhb
Copy link
Contributor

@srhb srhb commented Jan 10, 2018

Motivation for this change

Attempt to alleviate the failing keymap tests discussed in #32020

Things done

xdotool now brings the xterm into focus before sending keys. Tested by starting xterm with -iconic, which works.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

@srhb
Copy link
Contributor Author

srhb commented Jan 10, 2018

@GrahamcOfBorg test keymap.colemak

@srhb srhb requested a review from aszlig January 10, 2018 20:25
@srhb
Copy link
Contributor Author

srhb commented Jan 10, 2018

@aszlig Unless you have a fix in the pipeline, I think this would help! :)

Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: x86_64-linux

syncing
machine: running command: sync
machine: exit status 0
2 out of 2 tests succeeded
test script finished in 13.78s
cleaning up
killing machine (pid 596)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/tmp/nix-build-vm-test-run-keymap-colemak.drv-0/vde1.ctl': Directory not empty
/nix/store/l4grja9g7b95lqgw9icvpg6hv3rf05r2-vm-test-run-keymap-colemak

@pbogdan
Copy link
Member

pbogdan commented Jan 10, 2018

subtest: Xorg keymap
machine: must succeed: DISPLAY=:0 xterm -class testterm -fullscreen -e /nix/store/mz5w2zvd6wr0hffxd490kwsgfhf24knx-test-input-reader 'homerow' 'a' 'a' 'homerow' 's' 'r' 'homerow' 'd' 's' 'homerow' 'f' 't' 'homerow' 'j' 'n' 'homerow' 'k' 'e' 'homerow' 'l' 'i' 'homerow' 'semicolon' 'o' &
machine: exit status 0
machine: must succeed: for i in $(seq 600); do if [ -e '/tmp/reader.ready' ]; then cat '/tmp/reader.ready' && rm -f '/tmp/reader.ready' && exit 0; fi; sleep 0.1; done; echo timed out after 60 seconds >&2; exit 1
machine# [   21.098118] homerow[1037]: Waiting for 'a' to be typed
machine: exit status 0
machine: must succeed: /nix/store/80n0ywl2wak1hy2jp0hxnd7zk6i47086-xdotool-3.20160805.1/bin/xdotool search --class testterm windowactivate --sync
machine# XGetWindowProperty[_NET_WM_DESKTOP] failed (code=1)
machine# XGetWindowProperty[_NET_ACTIVE_WINDOW] failed (code=1)
machine: exit status 0
machine: sending monitor command: sendkey a
machine: must succeed: for i in $(seq 600); do if [ -e '/tmp/reader.ready' ]; then cat '/tmp/reader.ready' && rm -f '/tmp/reader.ready' && exit 0; fi; sleep 0.1; done; echo timed out after 60 seconds >&2; exit 1
machine# timed out after 60 seconds
machine: exit status 1
machine: output: 
error: command `for i in $(seq 600); do if [ -e '/tmp/reader.ready' ]; then cat '/tmp/reader.ready' && rm -f '/tmp/reader.ready' && exit 0; fi; sleep 0.1; done; echo timed out after 60 seconds >&2; exit 1' did not succeed (exit code 1)
collecting coverage data
machine: running command: test -e /sys/kernel/debug/gcov
machine: exit status 1
syncing
machine: running command: sync
machine: exit status 0
1 out of 2 tests succeeded

I'm honestly not sure but perhaps we would need to sleep for a moment after the test xterm is launched before trying to bring it into focus? Or just disable xterm desktop manager for these tests so there aren't two of them to begin with 😆

@srhb
Copy link
Contributor Author

srhb commented Jan 11, 2018

Ugh, how annoying. If we can disable the extra xterm, let's do that instead. It just looked like it was hard to reach agreement in.

@srhb
Copy link
Contributor Author

srhb commented Jan 11, 2018

@pbogdan Giving that a go in #33755 ! :)

@srhb srhb closed this Jan 11, 2018
@pbogdan
Copy link
Member

pbogdan commented Jan 11, 2018

Thanks! :-) I also have a small change on top of this one that so far remains resilient to my attempts to break the tests 😆 (but not sure how it would fare on Hydra). That may leave us an option to revive this PR if there are objections to disabling xterm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants