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.

Sign up for GitHub

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

Jump to bottom

wrapGAppsHook: add basic tests #85976

Merged
merged 3 commits into from Jul 26, 2020
Merged

wrapGAppsHook: add basic tests #85976

merged 3 commits into from Jul 26, 2020

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Apr 25, 2020 •
edited

Motivation for this change

Wanted to reproduce #85515

cc @worldofpeace

Ericson2314
Ericson2314 reviewed Apr 25, 2020
View reviewed changes
pkgs/build-support/setup-hooks/wrap-gapps-hook/tests/sample-project/Makefile
@@ -0,0 +1,23 @@
PREFIX = $(out)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other tests are in pkgs/test, BTW, though I am sympathetic to putting tests near what is being tested, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, did not notice that. It might be cleaner to move it there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python tests are in pkgs/development/interpreters/python/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about it more, I prefer having it near as well.

@jtojnar
Copy link
Contributor Author

jtojnar commented Apr 25, 2020 •
edited

One thing I do not like is that everything is in bash. Some testing library would be nice.

@FRidh
Copy link
Member

FRidh commented Apr 27, 2020 •
edited

I used pytest for the Python tests but I doubt you would gain much considering the site of the test. It could on the other hand invite one to write more tests.

@FRidh FRidh added this to Needs review in Staging Apr 27, 2020
@jtojnar
Copy link
Contributor Author

jtojnar commented Apr 30, 2020

I verified that #86416 with the following patch successfully passes typelib-self-user-has-gi-typelib-path test.

--- a/pkgs/build-support/setup-hooks/wrap-gapps-hook/default.nix
+++ b/pkgs/build-support/setup-hooks/wrap-gapps-hook/default.nix
@@ -13,13 +13,14 @@
 
 makeSetupHook {
   deps = lib.optional (!stdenv.isDarwin) dconf.lib ++ [
-    gtk3
-    librsvg
+    # gtk3
+    # librsvg
     makeWrapper
   ];
   substitutions = {
     passthru.tests = let
       sample-project = ./tests/sample-project;
+      gobject-introspection-setup-hook = makeSetupHook {} gobject-introspection.setupHook;
       skip = cond: lib.optionalString cond ''touch $out; echo "Skipping test $name"; exit'';
     in rec {
       basic = stdenv.mkDerivation {
@@ -58,7 +59,7 @@ makeSetupHook {
         src = sample-project;
 
         nativeBuildInputs = [
-          gobject-introspection
+          gobject-introspection-setup-hook
           wrapGAppsHook
         ];
 
@@ -84,7 +85,7 @@ makeSetupHook {
         src = sample-project;
 
         nativeBuildInputs = [
-          gobject-introspection
+          gobject-introspection-setup-hook
           wrapGAppsHook
         ];

@jtojnar jtojnar mentioned this pull request Apr 30, 2020
Merged
1 task
@jtojnar jtojnar marked this pull request as draft June 1, 2020 05:20
@jtojnar jtojnar force-pushed the wrap-gapps-hook-tests branch 4 times, most recently from 35e2972 to 745a663 Compare July 23, 2020 21:19
@jtojnar jtojnar marked this pull request as ready for review July 23, 2020 21:20
@jtojnar
Copy link
Contributor Author

jtojnar commented Jul 23, 2020 •
edited

I would like to merge this as is as I keep discovering new and new edge cases around the setup hooks. We can improve the test framework later.

@jtojnar jtojnar changed the base branch from master to staging July 23, 2020 22:36
jtojnar
jtojnar commented Jul 23, 2020
View reviewed changes
pkgs/build-support/setup-hooks/wrap-gapps-hook/default.nix
# We use the wrapProgram function.
makeWrapper
];
substitutions = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The substitutions make this a mass rebuild due to

nixpkgs/pkgs/build-support/trivial-builders.nix

Lines 382 to 384 in c61b6b1

'' + lib.optionalString (substitutions != {}) ''
substituteAll ${script} $out/nix-support/setup-hook
'');

@jtojnar jtojnar merged commit d489409 into NixOS:staging Jul 26, 2020
Staging automation moved this from Needs review to Done Jul 26, 2020
@jtojnar jtojnar deleted the wrap-gapps-hook-tests branch July 26, 2020 11:55
@jtojnar jtojnar mentioned this pull request Aug 11, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ericson2314 Ericson2314 Ericson2314 left review comments

@FRidh FRidh FRidh left review comments

Assignees
No one assigned
Labels
10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500
Projects
Staging
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jtojnar @FRidh @Ericson2314