Restrict fileset necessary for deployment tests #450

Merged
fricklerhandwerk merged 3 commits from Niols/Fediversity:restrict-filesets-2 into main 2025-07-09 22:57:54 +02:00
Owner

This PR is based on #448 and #449, and should only be merged after them. From Matrix:

Now that we won't depend on the flake.nix anymore, we won't depend on all the flake-part.nix files (necessary to evaluate flake.nix) and all the files they depend on etc., so the Nix dependencies of the tests will be drastically reduced, and I will be able to leverage that by introducing a more subtle src. This will make the test not need to re-run if only things outside that reduced src changed (and the previous run is in the Nix store).

And here it is. You only need to read be1fdc4643, the other commits being just the PRs we depend on.

This PR is based on #448 and #449, and should only be merged after them. From Matrix: > Now that we won't depend on the flake.nix anymore, we won't depend on all the flake-part.nix files (necessary to evaluate flake.nix) and all the files they depend on etc., so the Nix dependencies of the tests will be drastically reduced, and I will be able to leverage that by introducing a more subtle src. This will make the test not need to re-run if only things outside that reduced src changed (and the previous run is in the Nix store). And here it is. You only need to read https://git.fediversity.eu/Niols/Fediversity/commit/be1fdc4643ad3d3dfd4b9e721dd44263a6d0d21c, the other commits being just the PRs we depend on.
Niols added 3 commits 2025-07-09 11:24:12 +02:00
Restrict filesets necessary for deployment tests
Some checks failed
/ check-pre-commit (pull_request) Has been cancelled
/ check-data-model (pull_request) Has been cancelled
/ check-peertube (pull_request) Has been cancelled
/ check-panel (pull_request) Has been cancelled
/ check-deployment-basic (pull_request) Has been cancelled
/ check-deployment-cli (pull_request) Has been cancelled
/ check-deployment-panel (pull_request) Has been cancelled
59ded28863
fricklerhandwerk reviewed 2025-07-09 11:26:49 +02:00
@ -13,1 +18,4 @@
extraSourceFileset = lib.fileset.unions [
# REVIEW: I would like to be able to grab all of `/deployment` minus
# `/deployment/check`, but I can't because there is a bunch of other files

But you can literally lib.fileset.subtract <this> <deployment/check> -- today!

But you can literally `lib.fileset.subtract <this> <deployment/check>` -- today!
Author
Owner

I know, this is not what the comment is about!

but I can't because there is a bunch of other files in /deployment.

If I take /deployment minus /deployment/check, I grab all the data model stuff as well, and I don't want that.

I know, this is not what the comment is about! > but I can't because there is a bunch of other files in `/deployment`. If I take `/deployment` minus `/deployment/check`, I grab all the data model stuff as well, and I don't want that.

Ah got it

Ah got it
fricklerhandwerk marked this conversation as resolved
fricklerhandwerk reviewed 2025-07-09 11:28:08 +02:00
@ -56,0 +65,4 @@
extraSourceFileset = mkOption {
## FIXME: type? what is the type of a fileset? cf
## https://moduscreate.slack.com/archives/C0T4L4QPR/p1752048299160429

it's a slightly-more-than-slightly magical attrset, let's just take it for granted and link to reference documentation. in any case it doesn't have a module system type

it's a slightly-more-than-slightly magical attrset, let's just take it for granted and link to reference documentation. in any case it doesn't have a module system type
Owner

don't have access to modus slack, sad 😸

don't have access to modus slack, sad 😸
Author
Owner

I know, sorry about this :-/

I know, sorry about this :-/
Author
Owner

This was just me asking on the Nix team how to make a type for file sets, but in the end it is just a discussion between Valentin and I, so it could have stayed on Matrix :p

This was just me asking on the Nix team how to make a type for file sets, but in the end it is just a discussion between Valentin and I, so it could have stayed on Matrix :p
Author
Owner

I can revert it, of course, but what do you guys think of 128ddc60b3?

I can revert it, of course, but what do you guys think of https://git.fediversity.eu/Fediversity/Fediversity/commit/128ddc60b37a115aad002a08d2d86703ed38fec1?
kiara approved these changes 2025-07-09 11:53:58 +02:00
Niols added 1 commit 2025-07-09 12:11:58 +02:00
Give a proper type to the sourceFileset option
Some checks failed
/ check-pre-commit (pull_request) Has been cancelled
/ check-data-model (pull_request) Has been cancelled
/ check-peertube (pull_request) Has been cancelled
/ check-panel (pull_request) Has been cancelled
/ check-deployment-basic (pull_request) Has been cancelled
/ check-deployment-cli (pull_request) Has been cancelled
/ check-deployment-panel (pull_request) Has been cancelled
2686006e6a
Niols force-pushed restrict-filesets-2 from 2686006e6a to 128ddc60b3 2025-07-09 12:12:53 +02:00 Compare
Niols added 1 commit 2025-07-09 15:22:24 +02:00
Restrict filesets even more by avoiding test files that are unecessary inside the test
Some checks failed
/ check-pre-commit (pull_request) Successful in 13s
/ check-data-model (pull_request) Successful in 29s
/ check-panel (pull_request) Has been cancelled
/ check-deployment-basic (pull_request) Has been cancelled
/ check-deployment-cli (pull_request) Has been cancelled
/ check-deployment-panel (pull_request) Has been cancelled
/ check-peertube (pull_request) Has been cancelled
a4bb891231
Niols force-pushed restrict-filesets-2 from a4bb891231 to 44e3e5db4c 2025-07-09 15:23:17 +02:00 Compare
Niols added 1 commit 2025-07-09 15:23:44 +02:00
[TO REVERT] Trigger CI, demonstrate improvement
All checks were successful
/ check-pre-commit (pull_request) Successful in 14s
/ check-data-model (pull_request) Successful in 30s
/ check-peertube (pull_request) Successful in 20s
/ check-panel (pull_request) Successful in 1m31s
/ check-deployment-basic (pull_request) Successful in 32s
/ check-deployment-cli (pull_request) Successful in 42s
/ check-deployment-panel (pull_request) Successful in 1m48s
2f1d898dad
Author
Owner

Alright, all dependencies have been merged, we're ready for this one. I have pushed:

  • A version rebased on recent main, 44e3e5db4c. Its CI run is here.
  • A tiny commit, 2f1d898dad, to trigger CI and showcase that the tests only run Nix evaluation but don't need to rebuild for just a small change. The CI run is here.

There only remains to wait for CI to get green, and to validate a840a8cabb, a commit introducing a type for filesets.

Alright, all dependencies have been merged, we're ready for this one. I have pushed: - A version rebased on recent `main`, https://git.fediversity.eu/Fediversity/Fediversity/commit/44e3e5db4c395502fc85dd9447552df296251cba. Its CI run is [here](https://git.fediversity.eu/Fediversity/Fediversity/actions/runs/971). - A tiny commit, https://git.fediversity.eu/Fediversity/Fediversity/commit/2f1d898dad3fbeb843b9e201c61e2bdf58739743, to trigger CI and showcase that the tests only run Nix evaluation but don't need to rebuild for just a small change. The CI run is [here](https://git.fediversity.eu/Fediversity/Fediversity/actions/runs/972). There only remains to wait for CI to get green, and to validate https://git.fediversity.eu/Fediversity/Fediversity/commit/a840a8cabb29fe7f5a9a6ed6cc5f36be132fecae, a commit introducing a type for filesets.
kiara reviewed 2025-07-09 15:58:17 +02:00
@ -55,1 +48,4 @@
extraTestScript = mkOption { };
sourceFileset = mkOption {
## REVIEW: Upstream to nixpkgs?
Owner

looks great! 🎉

looks great! 🎉
Author
Owner

Yaaaay. CI is all green. On 44e3e5db4c:

image

And on 2f1d898dad, running right after:

image

It is still a bit crazy that CI takes two minutes to realise that there is nothing to do, but OK, this pushes CI down to ~5 minutes when nothing touches the deployment code or tests, instead of nearly two hours.

Yaaaay. CI is all green. On https://git.fediversity.eu/Fediversity/Fediversity/commit/44e3e5db4c395502fc85dd9447552df296251cba: ![image](/attachments/3b5e7da9-6960-4014-9dfb-0011939e1d82) And on https://git.fediversity.eu/Fediversity/Fediversity/commit/2f1d898dad3fbeb843b9e201c61e2bdf58739743, running right after: ![image](/attachments/cf4cc9c7-786e-49c8-a882-3b2d4db46d9e) It is still a bit crazy that CI takes two minutes to realise that there is nothing to do, but OK, this pushes CI down to ~5 minutes when nothing touches the deployment code or tests, instead of nearly two hours.
Niols force-pushed restrict-filesets-2 from 2f1d898dad to 44e3e5db4c 2025-07-09 17:19:54 +02:00 Compare
Author
Owner

This force push is only here to remove 2f1d898dad. As far as I am concerned, this PR can be merged, that is if you guys are happy with a840a8cabb.

This force push is only here to remove https://git.fediversity.eu/Fediversity/Fediversity/commit/2f1d898dad3fbeb843b9e201c61e2bdf58739743. As far as I am concerned, this PR can be merged, that is if you guys are happy with https://git.fediversity.eu/Fediversity/Fediversity/commit/a840a8cabb29fe7f5a9a6ed6cc5f36be132fecae.
fricklerhandwerk reviewed 2025-07-09 22:56:31 +02:00
@ -56,0 +53,4 @@
name = "fileset";
description = "fileset";
descriptionClass = "noun";
check = (x: (builtins.tryEval (fileset.unions [ x ])).success);

clever!

clever!
fricklerhandwerk approved these changes 2025-07-09 22:56:59 +02:00
fricklerhandwerk merged commit b4e1c5b5b3 into main 2025-07-09 22:57:54 +02:00
fricklerhandwerk deleted branch restrict-filesets-2 2025-07-09 22:57:54 +02:00
Sign in to join this conversation.
No description provided.