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

treewide: add application testbeds #612

Merged
merged 13 commits into from
Dec 29, 2024
Merged

treewide: add application testbeds #612

merged 13 commits into from
Dec 29, 2024

Conversation

danth
Copy link
Owner

@danth danth commented Nov 8, 2024

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):

  • Easy to preview a pull request without changing your real configuration. Just type nix run github:«owner»/«repository»/«branch»#testbed-«application»-«polarity».
  • Compared to local installation, testbeds have their filesystem reset between rebuilds, so every test simulates a fresh installation. This can help to pick up on cases where there are manual steps required for a module to be effective.
  • CI tests for successful evaluation. The extent to which this actually confirms that the configuration is valid can vary.

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.

Copy link
Collaborator

@trueNAHO trueNAHO left a 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?

@danth
Copy link
Owner Author

danth commented Nov 15, 2024

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.

@trueNAHO
Copy link
Collaborator

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:

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.

Feel free to improve the commit message:

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

@danth
Copy link
Owner Author

danth commented Dec 10, 2024

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?

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.

danth pushed a commit that referenced this pull request Dec 10, 2024
@trueNAHO
Copy link
Collaborator

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.

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:

Feel free to improve the commit message:

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

@danth danth merged commit 71eea3f into master Dec 29, 2024
34 checks passed
@danth danth deleted the application-testbeds branch December 29, 2024 22:51
@danth
Copy link
Owner Author

danth commented Dec 29, 2024

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.

@stylix-automation
Copy link

Successfully created backport PR for release-24.11:

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.

2 participants