-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add a flake check for testing the stdlib #2061
Conversation
Bencher Report
Click to view all benchmark results
|
Yeah, it's the same for the actual stdlib code - often functions refer to other function of the same module directly, but use |
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.
Glad to see this moving forward!
flake.nix
Outdated
|
||
stdlibTests = pkgs.runCommandLocal "stdlib-test" { } | ||
'' | ||
${pkgs.lib.getExe self.packages."${system}".default} test ${./core/stdlib/std.ncl}; |
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.
Isn't the ;
at the end going to make the command pass whatever is the actual result? Maybe a sanity check would be to push a commit with a deliberate error in the stdlib somewhere, just to be sure.
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.
I don't know exactly why (maybe runCommandLocal
does a set -xeu
or something?) but it seems to fail if I make it fail.
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.
But why do you need the ;
? Can't we use a &&
? Do we also have to necessarily create $out
?
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.
Sure, I can use &&
instead; the behavior seems to be the same.
Yes, without making $out
then nix flake check
fails with
error: builder for '/nix/store/kyr0xs7ncr05i3frnphvvvr1q8p8ja1c-stdlib-test.drv' failed to produce output path for output 'out' at '/nix/store/kyr0xs7ncr05i3frnphvvvr1q8p8ja1c-stdlib-test.drv.chroot/root/nix/store/yil1zc0j0vy5p3sjsp0c1kqi8r0h6krj-stdlib-test'
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.
Thanks, looks good! I think &&
is indeed better as it doesn't rely on assumptions about how runCommandLocal
works internally 👍
This reverts commit 07c34e5.
An amusing little thing that I think is harmless: when a test in the stdlib uses the
std
name (likestd.contract.not
) then it's testing the copy of the stdlib that was compiled into the nickel binary. But when it uses a "local" name (likeHasField
) then it's testing the copy of the stdlib that it gets from the path./core/stdlib/std.ncl
.