nixosTests.sway: don't use ORC#238997
Conversation
github-actions
bot
added
the
6.topic: nixos
ofborg
bot
added
10.rebuild-darwin: 0
primeos
left a comment
primeos
left a comment
There was a problem hiding this comment.
It basically seems fine to me but would you mind briefly documenting in the commit message why we're making these changes (1-2 sentences and a link to #238642 should be enough)? Commit messages / documentation is important to me. I can also take care of it if you don't want.
And did you test if everything looks fine with a Nixpkgs revision where the OCR part fails? I'm asking because it could fail for good reasons. I'm also not keen on removing OCR tbh. It was never required but it always provided the extra security that the text rendering works properly (this already caused issues in the past - with Chromium as well). But we can certainly drop the OCR support for now as long as someone can confirm that only the OCR is the issue.
| }; | ||
|
|
||
| etc."gpg-agent.conf".text = '' | ||
| pinentry-timeout 86400 |
There was a problem hiding this comment.
It'd be great if we could document this change (either here via a comment or via the commit message) - otherwise it might not be clear / obvious enough in the future.
I also checked the man page but the timeout seems to come from pinentry-gnome3:
--pinentry-timeout n
This option asks the Pinentry to timeout after n seconds with no user input. The default value of 0 does not ask the pinentry to timeout, however a Pinentry may use its own default timeout
value in this case. A Pinentry may or may not honor this request.
There was a problem hiding this comment.
Oh yeah I tried that but it didn't work with 0 (which was with the first value I tried), but I didn't retry with a large positive value... I'll investigate
There was a problem hiding this comment.
Actually I tried this again and it just doesn't come up. Seems like the config file is really needed.
| import shlex | ||
| import json | ||
|
|
||
| q = shlex.quote |
There was a problem hiding this comment.
Hm, not sure about this but I can live with it.
from shlex import quote (or from shlex import quote as q) would probably be nicer.
| NODE_GROUPS = ["nodes", "floating_nodes"] | ||
|
|
||
|
|
||
| def swaymsg(command: str = "", succeed=True, type="command"): |
There was a problem hiding this comment.
Optional: A docstring would be nice (but the function is well readable so it's not required).
For the other two functions it'd be a bit more useful.
|
|
||
| return any(pattern in name for name in nodes) | ||
|
|
||
| retry(func) |
There was a problem hiding this comment.
Not sure how safe it is to rely on retry being available but it should be fine given that other tests also already rely on it.
| swaymsg("exec mkdir -p ~/.gnupg") | ||
| swaymsg("exec cp /etc/gpg-agent.conf ~/.gnupg") |
There was a problem hiding this comment.
That's a bit unfortunate but the package seems to use sysconfdir=${gnupg}/etc/gnupg so it's fine for now.
| swaymsg("exec mkdir -p ~/.gnupg") | ||
| swaymsg("exec cp /etc/gpg-agent.conf ~/.gnupg") | ||
|
|
||
| swaymsg("exec DISPLAY=INVALID gpg --no-tty --yes --quick-generate-key test", succeed=False) |
There was a problem hiding this comment.
Do we really need the succeed=False here and do you know why it returns an unsuccessful exit code?
| machine.wait_until_succeeds("pgrep --exact gpg") | ||
| machine.wait_for_text("Passphrase") | ||
| wait_for_window("gpg") | ||
| machine.succeed("pgrep --exact gpg") |
There was a problem hiding this comment.
Hm, it's fine but is this really useful again after waiting for the window (since we already ensured that the process is there before waiting for the window)?
| before = get_height() | ||
| machine.send_key("alt-shift-e") | ||
| machine.wait_for_text("You pressed the exit shortcut.") | ||
| retry(lambda _: get_height() < before) |
There was a problem hiding this comment.
It's fine but a bit of a weird test. Would seem better if we could use wait_for_window but I haven't tested if and how it gets listed there.
There was a problem hiding this comment.
It doesn't show up in the window list, this was the best thing I could think of.
|
Thanks. Given that EDIT: I think in-code comments are usually better than detailed commit messages. |
| @@ -45,6 +45,10 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: { | |||
| regular2 = foreground; | |||
There was a problem hiding this comment.
We can remove this now, right?
There was a problem hiding this comment.
Yeah, the entire foot config I think (unless we want to return to ORC? Not sure how common font rendering issues are vs. how finicky ORC is (doesn't work reliably in interactive mode, etc.))
4 participants
Description of changes
Factor out ORC. Also don't time out the gpg-agent. Should fix #238642
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)