-
-
Notifications
You must be signed in to change notification settings - Fork 184
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
treewide: add application testbeds #612
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we require new modules to provide a testbed once this PR merges, or is it still too early considering the overall state of #319?
For example, certain applications cannot be properly tested without mock files. While terminals should be fine with the GNU coreutils installed, Zathura requires a PDF and text editors would benefit from better files than /etc/shells
. It would be cool to install the Stylix source tree (e.g. inputs.self
) and a PDF version of the Stylix documentation somewhere into the VM for this reason.
From 195e32407441033407a6ea7ef531133c8e2bedb0 Mon Sep 17 00:00:00 2001 From: Daniel Thwaites <[email protected]> Date: Fri, 8 Nov 2024 18:00:26 +0000 Subject: [PATCH 01/13] stylix: add template for application testbeds
It would be beneficial to include the informative cover-letter (initial PR message) in the message body of the initial commit. Consider copy-pasting the following slightly adapted version into [PATCH 01/13] stylix: add template for application testbeds
:
Extend testbeds with individual GUI applications beyond DEs, offering the
following benefits:
- Preview PRs without changing your real configuration by running:
nix \
run \
github:«owner»/«repository»/«branch»#testbed-«application»-«polarity»'
- Unlike local installations, testbeds reset their filesystem between rebuilds,
ensuring each test simulates a fresh installation to identify cases requiring
manual steps for module effectiveness.
- Enhance CI testing with successful evaluation. The extent of confirmation
varies based on the configuration's validity.
The application testbeds are currently based on GNOME but could be replaced with
a lightweight compositor like Cage [1] in the future. GNOME was chosen for the
time being due to preconfigured services, such as a secret service [2], which
some applications require.
[1]: https://github.com/cage-kiosk/cage?tab=readme-ov-file#cage-a-wayland-kiosk
[2]: https://specifications.freedesktop.org/secret-service-spec/latest
From 6c67f75101679ce3c7d415995e090c61944e3bed Mon Sep 17 00:00:00 2001 From: Daniel Thwaites <[email protected]> Date: Fri, 8 Nov 2024 18:00:44 +0000 Subject: [PATCH 02/13] firefox: add testbed --- modules/firefox/testbed.nix | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 modules/firefox/testbed.nix diff --git a/modules/firefox/testbed.nix b/modules/firefox/testbed.nix new file mode 100644 index 00000000..770c5901 --- /dev/null +++ b/modules/firefox/testbed.nix @@ -0,0 +1,21 @@ +{ pkgs, ... }: + +let package = pkgs.firefox; + +in { + stylix.testbed.application = { + enable = true; + name = "firefox"; + inherit package; + }; + + home-manager.sharedModules = [{ + programs.firefox = { + enable = true; + inherit package; + profiles.stylix.isDefault = true; + }; + + stylix.targets.firefox.profileNames = [ "stylix" ]; + }]; +}
Is it possible to simplify this testbed by having the /modules/firefox/hm.nix
module imply the stylix
profile as the default? Or is there a reason it does not already do that?
Profiles don't only affect the theme; many users will be coming with existing Home Manager configurations where they used a different name for their default profile. Changing the name of an existing profile requires manually renaming the files related to that profile. |
LGTM. What about this:
Feel free to improve the commit message:
|
I don't think we should block PRs on the basis of not having a testbed: this may load additional work onto contributors who don't have time or motivation to do it, which delays merging and discourages them from contributing again in the future. For simple cases, it may be quicker to write a follow-up PR ourselves than to request changes on the original PR. However, I think it's good to aim to having a testbed for as many modules as possible. It could be something we recommend in this guide for example. |
This follows the discussion at #612 (comment)
Sounds good to me. Do you mind rewording the first commit of this PR or should we just merge as-is with a merge commit:
|
I merged this with the adapted message in the body of the merge commit, so it's available in the Git history without needing to force-push the original commit. |
Successfully created backport PR for |
This PR extends the current testbeds to also include individual GUI applications, rather than just desktop environments. This is useful for a number of reasons (which also relate to the testbed system as a whole):
nix run github:«owner»/«repository»/«branch»#testbed-«application»-«polarity»
.The application testbeds are currently based on GNOME. This could be replaced with a lightweight compositor such as Cage in the future. I chose GNOME for the time being since it comes preconfigured with various services, such as a secret service, which some applications require.