nixosTests/cosmic: move core logic into a dedicated script by thefossguy · Pull Request #519448 · NixOS/nixpkgs

thefossguy · 2026-05-12T14:32:40Z

Things done

thefossguy · 2026-05-12T14:36:31Z

At the moment, the test fails because I added a new test for handling polkit degradation in the second commit.

But, once I apply https://github.com/pop-os/cosmic-osd/commit/cb9698c8536df05b7086115a071a3d3838b245f2.patch?full_index=1 to cosmic-osd, this new test passes.

thefossguy · 2026-05-12T14:37:54Z

Part of #508829

a-kenji · 2026-05-12T16:06:18Z

Maybe create a nixos/tests/cosmic/* directory in this PR or a follow up?

thefossguy · 2026-05-12T16:11:57Z

I'm up for whatever we decide. Do we have any official guidelines on this?

thefossguy · 2026-05-12T16:29:55Z

Additionally, I'd like to wait at least today before merging to see if upstream tags a new release. That way, we can get the cosmic-osd fix in, and can confidently verify that none of the tests fail.

salva09 · 2026-05-13T00:05:00Z

I created #519587 for the tagged release that should have the cosmic-osd fix. Is there a way I could run the new test?

a-kenji · 2026-05-13T15:00:48Z

I'm up for whatever we decide. Do we have any official guidelines on this?

I would guess its OK to just do it.

thefossguy · 2026-05-16T07:40:53Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 519448 --extra-nixpkgs-config '{ allowBroken = false; }' --additional-package nixosTests.cosmic-noxwayland --additional-package nixosTests.cosmic-autologin --additional-package nixosTests.cosmic --additional-package nixosTests.cosmic-autologin-noxwayland
Commit: be7d86b57f927b91333fc691590a3db898581a91

`x86_64-linux`

✅ 4 tests built:

nixosTests.cosmic
nixosTests.cosmic-autologin
nixosTests.cosmic-autologin-noxwayland
nixosTests.cosmic-noxwayland

thefossguy · 2026-05-16T07:57:50Z

For some reason, the tests now fail on aarch64-linux. Digging into it.

thefossguy · 2026-05-19T14:15:16Z

Aha, the tests were failing on aarch64 because software rending was too slow for cosmic-osd's pop-up dialog to render. Added a few more guardrails as well.

thefossguy · 2026-05-19T14:25:05Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 519448 --extra-nixpkgs-config '{ allowBroken = false; }' --additional-package nixosTests.cosmic-autologin --additional-package nixosTests.cosmic-noxwayland --additional-package nixosTests.cosmic-autologin-noxwayland --additional-package nixosTests.cosmic
Commit: 26087cf158dfc13702b84316f2593f13b726d128

`aarch64-linux`

✅ 4 tests built:

nixosTests.cosmic
nixosTests.cosmic-autologin
nixosTests.cosmic-autologin-noxwayland
nixosTests.cosmic-noxwayland

thefossguy · 2026-05-19T14:25:11Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 519448 --extra-nixpkgs-config '{ allowBroken = false; }' --additional-package nixosTests.cosmic-noxwayland --additional-package nixosTests.cosmic-autologin-noxwayland --additional-package nixosTests.cosmic --additional-package nixosTests.cosmic-autologin
Commit: 26087cf158dfc13702b84316f2593f13b726d128

`x86_64-linux`

✅ 4 tests built:

nixosTests.cosmic
nixosTests.cosmic-autologin
nixosTests.cosmic-autologin-noxwayland
nixosTests.cosmic-noxwayland

a-kenji · 2026-05-22T11:00:00Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 519448 --additional-package nixosTests.cosmic-autologin-noxwayland --additional-package nixosTests.cosmic-noxwayland --additional-package nixosTests.cosmic-autologin --additional-package nixosTests.cosmic
Commit: 7b412ccc0f87c410fbaeba763b085e9d912be95a

`x86_64-linux`

✅ 4 tests built:

nixosTests.cosmic
nixosTests.cosmic-autologin
nixosTests.cosmic-autologin-noxwayland
nixosTests.cosmic-noxwayland

a-kenji

Looks good. Tested.

thefossguy · 2026-05-27T01:28:15Z

I'd appreciate a review from you both, if possible, @alyssais @nyabinary.

alyssais · 2026-05-27T11:14:55Z

Maybe you could move the Nix file in a separate earlier commit, so git can recognize the rename?

thefossguy · 2026-05-27T16:16:38Z

This is a git mv. But due to a large enough delta, it doesn't register as a rename. Want me to split it into two commits instead? One that renames it and the second with the actual change.

alyssais · 2026-05-27T16:30:43Z

Yeah, that's what I meant. git mv doesn't do anything special.

thefossguy · 2026-05-27T18:01:42Z

Pushed the changes :)

alyssais

This is still really difficult to review. Not adding a new test in the same commit as porting the old ones might help, but even without all the polkit stuff I still have to wonder if this is really worth the explosion in length…

I haven't tried, so maybe it doesn't make sense, but would it be possible to avoid the long timeout by running a separate instance of a Python program for each application we want to test?

thefossguy · 2026-05-28T06:00:34Z

This is still really difficult to review. Not adding a new test in the same commit as porting the old ones might help

Sorry about that, I'll split it further.

but even without all the polkit stuff I still have to wonder if this is really worth the explosion in length...

The primary reason behind this migration of test logic from testScript to a dedicated python script that is auto-started is this:

The COSMIC DE session is different than the backdoor from testScript, forcing us to be creative in launching apps for our test.
This worked up until we needed to add polkit testing, where we really need to either recreate the COSMIC DE session or launch it inside the session.
I would be happy to keep polkit in testScript but I intend to add more tests in future (logout, re-login, lock, unlock, notifications, etc) and that complexity isn't worth it in testScript IMHO.

So yes, it isn't worth it right now given we haven't changed much, but I'd rather refactor now than later. The test for polkit was added as a matter of urgency since it is very much a core component that every user expects to work properly.

I haven't tried, so maybe it doesn't make sense, but would it be possible to avoid the long timeout by running a separate instance of a Python program for each application we want to test?

Do you mean some sort of multi-threading/parallelism with like a thread per app? We can't do this without also increasing our virtualisation.memorySize from 4GiB to something un-OOM-able.

nyabinary

lgtm :3

thefossguy · 2026-06-01T07:38:34Z

@alyssais do you have any current concerns unaddressed and/or any new concerns with this PR? I'd love to have this merged before #524670 so it can be tested with more coverage. :)

thefossguy · 2026-06-05T07:57:43Z

If there are no concerns, could someone please merge this?

Pandapip1

Mostly LGTM, just a few non-blocking nitpicks

Pandapip1 · 2026-06-05T08:12:07Z

+  emptyPDF = config.node.pkgs.stdenvNoCC.mkDerivation {
+    name = "empty-pdf";
+    dontUnpack = true;
+    nativeBuildInputs = [ config.node.pkgs.imagemagick ];
+    buildPhase = ''
+      magick xc:none -page Letter empty.pdf
+    '';
+    installPhase = ''
+      mkdir $out
+      mv empty.pdf $out/empty.pdf
+    '';
+  };


How about:

Suggested change

emptyPDF = config.node.pkgs.stdenvNoCC.mkDerivation {

name = "empty-pdf";

dontUnpack = true;

nativeBuildInputs = [ config.node.pkgs.imagemagick ];

buildPhase = ''

magick xc:none -page Letter empty.pdf

'';

installPhase = ''

mkdir $out

mv empty.pdf $out/empty.pdf

'';

};

emptyPDF = config.node.pkgs.stdenvNoCC.mkDerivation {

name = "empty.pdf";

dontUnpack = true;

dontConfigure = true;

nativeBuildInputs = [ config.node.pkgs.imagemagick ];

buildPhase = ''

runHook preBuild

magick xc:none -page Letter empty.pdf

runHook postBuild

'';

installPhase = ''

runHook preInstall

cp empty.pdf $out

runHook postInstall

'';

};

There's also an argument to be made for adding this to by-name.

Do we really need to do this for a derivation that simply creates an empty PDF? I'm fine either way, though.

Might as well imo

There's already precedent for helper/test derivations living in the main package set in the form of emptyFile and emptyDirectory.

Pandapip1 · 2026-06-05T08:12:54Z

+  cosmicTestDesktop = config.node.pkgs.makeDesktopItem {
+    name = "cosmicTest";
+    desktopName = "COSMIC NixOS VM test (${testName})";
+    exec = "${cosmicTest}/bin/cosmicTest";


Suggested change

exec = "${cosmicTest}/bin/cosmicTest";

exec = lib.getExe' cosmicTest "cosmicTest";

That being said, this might even be better off as

Suggested change

exec = "${cosmicTest}/bin/cosmicTest";

exec = "cosmicTest";

See #308324

Pandapip1 · 2026-06-05T08:15:35Z

+    ydotool_daemon_socket_path = f"{xdg_runtime_dir}/.ydotool_socket"
+    ydotool_daemon_process = subprocess.Popen(
+        [
+            f"{cli_args.ydotool_drv_store_path}/bin/ydotoold",


Any reason we're not relying on PATH here? It would simplify this script quite a bit.

I'm pretty sure that there was a reason during my testing but it may have changed after the cleanup. Let me test this out and get back.

Pandapip1 · 2026-06-05T08:15:55Z

+    notification_watcher_exists = False
+    while time.monotonic() < notification_watcher_wait_deadline:
+        busctl_process = subprocess.run(
+            ["busctl", "--user", "status", "com.system76.CosmicStatusNotifierWatcher"],


Why are we relying on the PATH here and not elsewhere?

Pandapip1 · 2026-06-05T08:16:55Z

+  cosmicTest = config.node.pkgs.writeShellScriptBin "cosmicTest" ''
+    exec ${config.node.pkgs.python3Minimal}/bin/python3 ${./test-script.py} \
+        --log-file-path ${logFilePath} \
+        --cosmic-reader-pdf ${emptyPDF}/empty.pdf \


Would be replaced with

Suggested change

--cosmic-reader-pdf ${emptyPDF}/empty.pdf \

--cosmic-reader-pdf ${emptyPDF} \

or

Suggested change

--cosmic-reader-pdf ${emptyPDF}/empty.pdf \

--cosmic-reader-pdf ${config.node.pkgs.emptyPDF} \

The 1st suggestion doesn't apply here because the installPhase copies the PDF to $out/empty.pdf. We could simply copy it to $out, will test that.

As for the 2nd suggestion, that doesn't work since emptyPDF is part of the let-in block, not part of nixpkgs' packages. And I don't see a reason to add it to pkgs/by-name for a derivation that doesn't provide an executable binary. I'm not opposed. Just not sure if adding it to nixkpgs is the right call here.

This is specifically if #519448 (comment) is applied

Pandapip1 · 2026-06-05T08:17:12Z

+  # Use `writeShellScriptBin` instead of `writeShellScript` so that the
+  # process name in the journald log appears as 'cosmicTest[$pid]'
+  cosmicTest = config.node.pkgs.writeShellScriptBin "cosmicTest" ''
+    exec ${config.node.pkgs.python3Minimal}/bin/python3 ${./test-script.py} \


Suggested change

exec ${config.node.pkgs.python3Minimal}/bin/python3 ${./test-script.py} \

exec ${lib.getExe config.node.pkgs.python3Minimal} ${./test-script.py} \

thefossguy · 2026-06-05T08:40:08Z

Thanks for the review @Pandapip1! I'll update this PR soon-ish.

Pandapip1 · 2026-06-05T19:32:18Z

Honestly given how much this PR has already been stalled, I'd probably prefer to see this merged first, then the improvements can be made in follow-up PRs.

thefossguy requested a review from a team May 12, 2026 14:32

thefossguy mentioned this pull request May 12, 2026

nixos/cosmic: remove unnecessary dependency of cosmic-session.target on xdg-desktop-autostart.target #519447

Draft

13 tasks

thefossguy marked this pull request as draft May 13, 2026 02:02

thefossguy force-pushed the cosmic-test-improvement branch 2 times, most recently from 11f2f8e to 4c4b064 Compare May 14, 2026 07:40

thefossguy changed the title ~~nixosTests/cosmic: move NixOS VM test's core logic from testScript into dedicated script~~ nixosTests/cosmic: move core logic into a dedicated script May 16, 2026

thefossguy force-pushed the cosmic-test-improvement branch from 4c4b064 to be7d86b Compare May 16, 2026 06:21

thefossguy marked this pull request as ready for review May 16, 2026 06:21

thefossguy marked this pull request as draft May 18, 2026 16:33

thefossguy force-pushed the cosmic-test-improvement branch 3 times, most recently from 449fde1 to 26087cf Compare May 19, 2026 14:07

thefossguy marked this pull request as ready for review May 19, 2026 14:15

thefossguy force-pushed the cosmic-test-improvement branch from 26087cf to 7b412cc Compare May 20, 2026 08:25

a-kenji reviewed May 22, 2026

View reviewed changes

Comment thread nixos/tests/cosmic/default.nix

thefossguy requested review from alyssais and nyabinary May 23, 2026 05:32

a-kenji approved these changes May 23, 2026

View reviewed changes

nixpkgs-ci Bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label May 23, 2026

thefossguy force-pushed the cosmic-test-improvement branch from 7b412cc to 7398dd7 Compare May 27, 2026 16:39

alyssais reviewed May 27, 2026

View reviewed changes

Comment thread nixos/tests/cosmic/default.nix Outdated

thefossguy force-pushed the cosmic-test-improvement branch 2 times, most recently from ed323aa to a6e422d Compare May 28, 2026 09:13

nixosTests/cosmic: move test into a dedicated directory

267d1f4

thefossguy force-pushed the cosmic-test-improvement branch from a6e422d to 010f24d Compare May 28, 2026 13:01

thefossguy added 2 commits May 28, 2026 19:10

nixosTests/cosmic: move core testing logic into a dedicated script

6589f01

nixosTests/cosmic: add test for polkit authentication

86fa08d

thefossguy force-pushed the cosmic-test-improvement branch from 010f24d to 86fa08d Compare May 28, 2026 13:41

thefossguy requested a review from alyssais May 29, 2026 09:22

nyabinary approved these changes May 29, 2026

View reviewed changes

nixpkgs-ci Bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels May 29, 2026

Pandapip1 reviewed Jun 5, 2026

View reviewed changes

	exec = "${cosmicTest}/bin/cosmicTest";
	exec = lib.getExe' cosmicTest "cosmicTest";

	--cosmic-reader-pdf ${emptyPDF}/empty.pdf \
	--cosmic-reader-pdf ${emptyPDF} \

	--cosmic-reader-pdf ${emptyPDF}/empty.pdf \
	--cosmic-reader-pdf ${config.node.pkgs.emptyPDF} \

	exec ${config.node.pkgs.python3Minimal}/bin/python3 ${./test-script.py} \
	exec ${lib.getExe config.node.pkgs.python3Minimal} ${./test-script.py} \

Uh oh!

Conversation

thefossguy commented May 12, 2026

Things done

Uh oh!

thefossguy commented May 12, 2026

Uh oh!

thefossguy commented May 12, 2026

Uh oh!

a-kenji commented May 12, 2026

Uh oh!

thefossguy commented May 12, 2026

Uh oh!

thefossguy commented May 12, 2026

Uh oh!

salva09 commented May 13, 2026

Uh oh!

a-kenji commented May 13, 2026

Uh oh!

thefossguy commented May 16, 2026

nixpkgs-review result

x86_64-linux

Uh oh!

thefossguy commented May 16, 2026

Uh oh!

thefossguy commented May 19, 2026

Uh oh!

thefossguy commented May 19, 2026

nixpkgs-review result

aarch64-linux

Uh oh!

thefossguy commented May 19, 2026

nixpkgs-review result

x86_64-linux

Uh oh!

a-kenji commented May 22, 2026

nixpkgs-review result

x86_64-linux

Uh oh!

Uh oh!

a-kenji left a comment

Choose a reason for hiding this comment

Uh oh!

thefossguy commented May 27, 2026

Uh oh!

alyssais commented May 27, 2026

Uh oh!

thefossguy commented May 27, 2026

Uh oh!

alyssais commented May 27, 2026 via email

Uh oh!

thefossguy commented May 27, 2026

Uh oh!

alyssais left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thefossguy commented May 28, 2026

Uh oh!

nyabinary left a comment

Choose a reason for hiding this comment

Uh oh!

thefossguy commented Jun 1, 2026

Uh oh!

thefossguy commented Jun 5, 2026

Uh oh!

Pandapip1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pandapip1 Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

`nixpkgs-review` result

`x86_64-linux`

`nixpkgs-review` result

`aarch64-linux`

`nixpkgs-review` result

`x86_64-linux`

`nixpkgs-review` result

`x86_64-linux`

Pandapip1 left a comment •

edited

Loading

Pandapip1 Jun 5, 2026 •

edited

Loading