deno: enable tests by ofalvai · Pull Request #384838 · NixOS/nixpkgs

ofalvai · 2025-02-24T19:09:09Z

The project has an extensive tests suite in both Rust and Typescript, and while I couldn't enable all of them, this is one attempt to run at least a subset in the Nix build.

Things done

Add a 👍 reaction to pull requests you find important.

06kellyjac

It'd be nice to have tests back, good stuff.
They used to be a bit flakey so something to keep an eye on.

Added a few comments/questions

Also, by how much do you think this affects build times? Since the deno build is already very long due to the time spent linking.

pkgs/by-name/de/deno/package.nix

06kellyjac · 2025-02-24T20:13:07Z

pkgs/by-name/de/deno/patches/tests-no-chown.patch

Is it worth skipping these tests completely?
I cant remember the status of darwin sandboxing but it might just be worth keeping the platforms standard and skipping these

I'm also not fully familiar with the Darwin sandbox, but these tests do pass on Darwin for some reason. We could also completely drop them, I don't have a strong opinion.

Are they are passing with the sandbox on on darwin? nix recently got improved support for larger (in terms of amount of store paths involved) sandboxed builds on darwin.

Yes, these tests did pass on Darwin with sandbox = relaxed.

06kellyjac · 2025-02-24T20:15:01Z

pkgs/by-name/de/deno/patches/unvendor-sqlite.patch

Is this patch unused or did I just miss something?

It's used, but you made me realize this is not strictly related to tests, I just think it would be a good idea. Just like how we unvendored libffi after it caused a build failure a few months ago: #368102

Should I drop this change?

We could easily replace this patch with substituteInPlace

MisileLab · 2025-02-25T10:09:54Z

`nixpkgs-review` result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 384838

Logs: https://github.com/MisileLab/nixpkgs-review-gha/actions/runs/13516415642

`x86_64-linux`

❌ 1 package failed to build:

windmill

✅ 9 packages built:

deno
era
quarto
quartoMinimal
rstudio
rstudio-server
rstudioServerWrapper
rstudioWrapper
silverbullet

`aarch64-linux`

⏩ 4 packages marked as broken and skipped:

rstudio
rstudio-server
rstudioServerWrapper
rstudioWrapper

❌ 6 packages failed to build:

deno
era
quarto
quartoMinimal
silverbullet
windmill

`x86_64-darwin`

✅ 5 packages built:

deno
era
quarto
quartoMinimal
silverbullet

`aarch64-darwin`

✅ 5 packages built:

deno
era
quarto
quartoMinimal
silverbullet

ofalvai · 2025-02-25T15:13:53Z

Also, by how much do you think this affects build times? Since the deno build is already very long due to the time spent linking.

Build times on my MacBook M3 Pro:

without tests: 11m
with tests: 16m

We could also add requiredSystemFeatures = [ "big-parallel" ]; to this derivation, I think it's justified.

SuperSandro2000 · 2025-02-25T15:55:13Z

pkgs/by-name/de/deno/package.nix

Suggested change

./patches/unvendor-sqlite.patch

./patches/tests-no-chown.patch

./patches/tests-replace-hardcoded-paths.patch

./patches/tests-darwin-differences.patch

./unvendor-sqlite.patch

./tests-no-chown.patch

./tests-replace-hardcoded-paths.patch

./tests-darwin-differences.patch

I'm not sure I follow, do you suggest moving the patch files out of the subdirectory?

SuperSandro2000 · 2025-02-25T15:56:54Z

pkgs/by-name/de/deno/package.nix

We can probably combine those into one to not traverse everything 3 times.

SuperSandro2000 · 2025-02-25T15:57:59Z

pkgs/by-name/de/deno/package.nix

Suggested change

hash = "sha256-Ir/jBmXsiM9T3dY6kd8wsC+qB1alHm5JNAx+aq9hXHo=";

fetchSubmodules = true; # required for tests

fetchSubmodules = true; # required for tests

hash = "sha256-Ir/jBmXsiM9T3dY6kd8wsC+qB1alHm5JNAx+aq9hXHo=";

ofalvai · 2025-02-28T19:38:39Z

I fixed the review comments and rebased my commits to the latest Deno version, I think this PR is ready

MisileLab · 2025-03-01T09:24:31Z

`nixpkgs-review` result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 384838

Logs: https://github.com/MisileLab/nixpkgs-review-gha/actions/runs/13602439731

`x86_64-linux`

❌ 1 package failed to build:

windmill

✅ 9 packages built:

deno
era
quarto
quartoMinimal
rstudio
rstudio-server
rstudioServerWrapper
rstudioWrapper
silverbullet

`aarch64-linux`

❌ 10 packages failed to build:

deno
era
quarto
quartoMinimal
rstudio
rstudio-server
rstudioServerWrapper
rstudioWrapper
silverbullet
windmill

`x86_64-darwin`

✅ 9 packages built:

deno
era
quarto
quartoMinimal
rstudio
rstudio-server
rstudioServerWrapper
rstudioWrapper
silverbullet

`aarch64-darwin`

✅ 9 packages built:

deno
era
quarto
quartoMinimal
rstudio
rstudio-server
rstudioServerWrapper
rstudioWrapper
silverbullet

MisileLab · 2025-03-01T09:30:19Z

I have no idea why aarch64-linux failing

ofalvai · 2025-03-01T10:03:54Z

I have no idea either, the log is not helpful. But it builds for me on aarch64-linux, at this point I think it's a GHA-specific issue

ofalvai · 2025-03-12T18:51:55Z

Rebased commits on top of #387198

06kellyjac

Thanks for rebasing. 1 holdup, 2 comments.

06kellyjac · 2025-03-12T19:46:42Z

pkgs/by-name/de/deno/package.nix

This needs to live outside of canExecute please or cross-platform builds will end up with additional binaries.

Also this method is opt-out of extra bits, but do we want to be opt-in instead? It'd just mean we wouldn't accidentally package more testing utils etc without noticing. This part isn't a big blocker.

Good idea, thank you. Addressed these in my latest change

06kellyjac · 2025-03-12T19:47:56Z

pkgs/by-name/de/deno/package.nix

Might want to cleanup $HOME and unset the var in postCheck. Not a major blocker

On the same note it maybe preferable to add writableTmpDirAsHomeHook as a nativeBuildInput instead

Nice, I didn't know about writableTmpDirAsHomeHook. I just replaced the custom script with that, thank you

ofalvai · 2025-04-30T14:06:32Z

@06kellyjac @SuperSandro2000 I re-purposed this PR and added the latest version bump on top. Tests still pass on aarch64-darwin and I'm testing soon on aarch64-linux too. Let's merge this if there are no more objections please.

ofalvai · 2025-04-30T15:03:46Z

I had to disable two more tests (I'm not sure why they passed on Darwin, they both access the network), but aarch64-linux also builds now

phaer · 2025-05-04T09:12:01Z

I'm not sure why they passed on Darwin, they both access the network)

You might have the sandbox off on darwin? I think that's still the default and would allow full networking during builds.

phaer · 2025-05-04T09:15:13Z

@ofalvai Do we know what's up with the x86_64-darwin failure? Is that due to a test? Did you check? Your aarch64-darwin machine should be able to build that, right? Does it do with the sandbox?

I'll try to test tomorrow and would be happy to merge once we have an explanation for that failing check.

ofalvai · 2025-05-04T09:18:58Z

Ofborg says it's a simple timeout, which is understandable, this project already takes a long time to build, and now I enabled tests as well.
I tested both Darwin platforms (via Rosetta) with sandbox = relaxed (that's why I added __darwinAllowLocalNetworking = true; to the derivation in this PR)

wolfgangwalther · 2025-06-01T14:24:43Z

merge conflict!

ofalvai · 2025-06-02T09:50:24Z

@wolfgangwalther it's fixed now. Tested on aarch64-darwin and aarch64-linux again, tests are still green.

wolfgangwalther · 2025-06-02T16:21:30Z

So I can build both linux fine, but darwin fails with this:

test repl::pty_complete_imports_no_panic_empty_specifier ... FAILED

failures:

---- repl::pty_complete_imports_no_panic_empty_specifier stdout ----
command /private/tmp/nix-build-deno-2.3.5.drv-0/source/target/aarch64-apple-darwin/release/deno repl -A
command cwd /private/tmp/nix-build-deno-2.3.5.drv-0/deno-cli-test8Iz5Gq
------ Start Full Text ------
"Deno 2.3.5\r\nexit using ctrl+d, ctrl+c, or close()\r\n\u{1b}[?2004h\r\u{1b}[K> \r\u{1b}[2C\r\u{1b}[K> i\r\u{1b}[3C\r\u{1b}[K> im\r\u{1b}[4C\r\u{1b}[K> imp\r\>
------- End Full Text -------
Next text: "Deno 2.3.5\r\nexit using ctrl+d, ctrl+c, or close()\r\n\r> \r\r> i\r\r> im\r\r> imp\r\r> impo\r\r> impor\r\r> import\r\r> import \r\r> import '\r"

thread 'repl::pty_complete_imports_no_panic_empty_specifier' panicked at tests/integration/repl_tests.rs:189:15:
Timed out.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    repl::pty_complete_imports_no_panic_empty_specifier

test result: FAILED. 780 passed; 1 failed; 10 ignored; 0 measured; 45 filtered out; finished in 183.84s

error: test failed, to rerun pass `-p cli_tests --test integration_tests`

(repeatedly)

Edit: That's with sandbox, though, on the community-builder. Is this expected?

ofalvai · 2025-06-02T16:45:49Z

Interesting, this is the test case that timed out for me on Linux, and so I added to the skip list. But it has never hanged for me on macOS. And I'm also building with sandbox = relaxed.

The project has an extensive tests suite in both Rust and Typescript, and while I couldn't enable all of them, this is one attempt to run at least a subset in the Nix build.

ofalvai · 2025-06-03T16:30:54Z

@wolfgangwalther that test is now excluded for all platforms. I looked at the test source and have no idea why it hangs while other REPL tests do not.

wolfgangwalther · 2025-06-11T17:34:54Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 384838 --package deno.tests --package deno
Commit: 1161ba59a7be43418e09dd101a05489df3e461c7

`x86_64-linux`

✅ 7 packages built:

deno
deno.tests.basic
deno.tests.fail-read-file
deno.tests.import-json
deno.tests.import-ts
deno.tests.read-file
deno.tests.version

`aarch64-linux`

✅ 7 packages built:

deno
deno.tests.basic
deno.tests.fail-read-file
deno.tests.import-json
deno.tests.import-ts
deno.tests.read-file
deno.tests.version

`x86_64-darwin`

✅ 7 packages built:

deno
deno.tests.basic
deno.tests.fail-read-file
deno.tests.import-json
deno.tests.import-ts
deno.tests.read-file
deno.tests.version

`aarch64-darwin`

✅ 7 packages built:

deno
deno.tests.basic
deno.tests.fail-read-file
deno.tests.import-json
deno.tests.import-ts
deno.tests.read-file
deno.tests.version

Pin Deno to version 2.2.12 as this is the last version that successfully built via Hydra. All newer versions revert to source-building which takes way too long for every developer. References: NixOS/nixpkgs#417331 References: NixOS/nixpkgs#384838 Signed-off-by: Fletcher Nichol <fletcher@systeminit.com>

ofalvai requested review from 06kellyjac and MisileLab February 24, 2025 19:09

github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 24, 2025

06kellyjac reviewed Feb 24, 2025

View reviewed changes

ofalvai force-pushed the deno-enable-tests branch from b667203 to 6e96270 Compare February 24, 2025 20:51

SuperSandro2000 reviewed Feb 25, 2025

View reviewed changes

ofalvai force-pushed the deno-enable-tests branch from 6e96270 to bb6ccdf Compare February 28, 2025 18:34

ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 28, 2025

ofalvai force-pushed the deno-enable-tests branch from bb6ccdf to 01d7ae0 Compare February 28, 2025 19:17

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 28, 2025

ofalvai force-pushed the deno-enable-tests branch from 01d7ae0 to 2fd7874 Compare February 28, 2025 19:37

ofalvai requested review from 06kellyjac and SuperSandro2000 February 28, 2025 19:38

MisileLab approved these changes Mar 1, 2025

View reviewed changes

wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 1, 2025

ofalvai force-pushed the deno-enable-tests branch from 2fd7874 to e5a28c4 Compare March 12, 2025 18:50

github-actions bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 12, 2025

06kellyjac requested changes Mar 12, 2025

View reviewed changes

wegank removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 12, 2025

ofalvai changed the title ~~deno: 2.2.12 -> 2.3.0, enable tests~~ deno: 2.2.12 -> 2.3.1, enable tests Apr 30, 2025

ofalvai force-pushed the deno-enable-tests branch from 28db71c to 52dbcf8 Compare April 30, 2025 15:03

06kellyjac approved these changes May 1, 2025

View reviewed changes

wolfgangwalther added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 1, 2025

ofalvai force-pushed the deno-enable-tests branch from 52dbcf8 to 34bcc6f Compare June 2, 2025 09:31

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 2, 2025

github-actions bot added the 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. label Jun 2, 2025

ofalvai changed the title ~~deno: 2.2.12 -> 2.3.1, enable tests~~ deno: enable tests Jun 2, 2025

deno: enable tests

1161ba5

The project has an extensive tests suite in both Rust and Typescript, and while I couldn't enable all of them, this is one attempt to run at least a subset in the Nix build.

ofalvai force-pushed the deno-enable-tests branch from 34bcc6f to 1161ba5 Compare June 3, 2025 16:29

ofalvai closed this Jun 3, 2025

ofalvai reopened this Jun 3, 2025

wolfgangwalther merged commit 5831013 into NixOS:master Jun 11, 2025
26 of 31 checks passed

kachick mentioned this pull request Jun 16, 2025

Cannot update nixos-unstable with the failing deno build on hydra kachick/wait-other-jobs#1106

Closed

limwa mentioned this pull request Jun 16, 2025

Build failure: deno #417331

Closed

3 tasks

ofalvai mentioned this pull request Jun 24, 2025

deno: 2.3.6 -> 2.3.7 #419511

Merged

13 tasks

fnichol mentioned this pull request Jul 19, 2025

Fnichol/updates 2025 w29 systeminit/si#6724

Merged

-    ./patches/unvendor-sqlite.patch
-    ./patches/tests-no-chown.patch
-    ./patches/tests-replace-hardcoded-paths.patch
-    ./patches/tests-darwin-differences.patch
+    ./unvendor-sqlite.patch
+    ./tests-no-chown.patch
+    ./tests-replace-hardcoded-paths.patch
+    ./tests-darwin-differences.patch

-    hash = "sha256-Ir/jBmXsiM9T3dY6kd8wsC+qB1alHm5JNAx+aq9hXHo=";
-    fetchSubmodules = true; # required for tests
+    fetchSubmodules = true; # required for tests
+    hash = "sha256-Ir/jBmXsiM9T3dY6kd8wsC+qB1alHm5JNAx+aq9hXHo=";

Uh oh!

Conversation

ofalvai commented Feb 24, 2025

Things done

Uh oh!

06kellyjac left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

MisileLab commented Feb 25, 2025

nixpkgs-review result

x86_64-linux

aarch64-linux

x86_64-darwin

aarch64-darwin

Uh oh!

ofalvai commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

ofalvai commented Feb 28, 2025

Uh oh!

MisileLab commented Mar 1, 2025

nixpkgs-review result

x86_64-linux

aarch64-linux

x86_64-darwin

aarch64-darwin

Uh oh!

MisileLab commented Mar 1, 2025

Uh oh!

ofalvai commented Mar 1, 2025

Uh oh!

ofalvai commented Mar 12, 2025

Uh oh!

06kellyjac left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ofalvai commented Apr 30, 2025

Uh oh!

ofalvai commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phaer commented May 4, 2025

Uh oh!

phaer commented May 4, 2025

Uh oh!

`nixpkgs-review` result

`x86_64-linux`

`aarch64-linux`

`x86_64-darwin`

`aarch64-darwin`

ofalvai commented Feb 25, 2025 •

edited

Loading

`nixpkgs-review` result

`x86_64-linux`

`aarch64-linux`

`x86_64-darwin`

`aarch64-darwin`

ofalvai commented Apr 30, 2025 •

edited

Loading

wolfgangwalther commented Jun 2, 2025 •

edited

Loading

ofalvai commented Jun 2, 2025 •

edited

Loading

`nixpkgs-review` result

`x86_64-linux`

`aarch64-linux`

`x86_64-darwin`

`aarch64-darwin`