From f6fa2e16c0aa81b26ea4df949e81037479a64981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 13 Sep 2025 09:36:49 +0200 Subject: [PATCH 1/3] tests: Gitea test nitpicks - Add proper waitpid() for child process cleanup - Simplify file existence check loop with early exit - Rename variables for clarity ($uri -> $request_uri, remove unused $i) --- t/Hydra/Plugin/gitea.t | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/t/Hydra/Plugin/gitea.t b/t/Hydra/Plugin/gitea.t index 3de4ea8f..64f41429 100644 --- a/t/Hydra/Plugin/gitea.t +++ b/t/Hydra/Plugin/gitea.t @@ -58,24 +58,23 @@ if (!defined($pid = fork())) { ok(sendNotifications(), "Sent notifications"); kill('INT', $pid); + waitpid($pid, 0); } # We expect $ctx{jobsdir}/server.py to create the file at $filename, but the time it # takes to do so is non-deterministic. We need to give it _some_ time to hopefully # settle -- but not too much that it drastically slows things down. for my $i (1..10) { - if (! -f $filename) { - diag("$filename does not yet exist"); - sleep(1); - } + last if -f $filename; + diag("$filename does not yet exist"); + sleep(1); } open(my $fh, "<", $filename) or die ("Can't open(): $!\n"); -my $i = 0; -my $uri = <$fh>; +my $request_uri = <$fh>; my $data = <$fh>; -ok(index($uri, "gitea/api/v1/repos/root/foo/statuses") != -1, "Correct URL"); +ok(index($request_uri, "gitea/api/v1/repos/root/foo/statuses") != -1, "Correct URL"); my $json = JSON->new; my $content; -- 2.51.0 From b832cab12c9d0fcfb40a8a0737eea3d398b4872f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Fri, 12 Sep 2025 21:02:51 +0200 Subject: [PATCH 2/3] Avoid shadowing internal run function by renaming it to runCommand see https://github.com/NixOS/hydra/issues/1520 --- src/lib/Hydra/Controller/Build.pm | 2 +- src/lib/Hydra/Helper/Nix.pm | 6 +++--- src/lib/Hydra/Plugin/DarcsInput.pm | 2 +- src/lib/Hydra/Plugin/GitInput.pm | 14 +++++++------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm index ac31cc5b..ca9a22cf 100644 --- a/src/lib/Hydra/Controller/Build.pm +++ b/src/lib/Hydra/Controller/Build.pm @@ -212,7 +212,7 @@ sub checkPath { sub serveFile { my ($c, $path) = @_; - my $res = run(cmd => ["nix", "--experimental-features", "nix-command", + my $res = runCommand(cmd => ["nix", "--experimental-features", "nix-command", "ls-store", "--store", getStoreUri(), "--json", "$path"]); if ($res->{status}) { diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm index b9354092..6eb0aa27 100644 --- a/src/lib/Hydra/Helper/Nix.pm +++ b/src/lib/Hydra/Helper/Nix.pm @@ -44,7 +44,7 @@ our @EXPORT = qw( readNixFile registerRoot restartBuilds - run + runCommand $MACHINE_LOCAL_STORE ); @@ -466,7 +466,7 @@ sub readIntoSocket{ -sub run { +sub runCommand { my (%args) = @_; my $res = { stdout => "", stderr => "" }; my $stdin = ""; @@ -506,7 +506,7 @@ sub run { sub grab { my (%args) = @_; - my $res = run(%args, grabStderr => 0); + my $res = runCommand(%args, grabStderr => 0); if ($res->{status}) { my $msgloc = "(in an indeterminate location)"; if (defined $args{dir}) { diff --git a/src/lib/Hydra/Plugin/DarcsInput.pm b/src/lib/Hydra/Plugin/DarcsInput.pm index bb8962a6..79e3d7bb 100644 --- a/src/lib/Hydra/Plugin/DarcsInput.pm +++ b/src/lib/Hydra/Plugin/DarcsInput.pm @@ -32,7 +32,7 @@ sub fetchInput { my $stdout = ""; my $stderr = ""; my $res; if (! -d $clonePath) { # Clone the repository. - $res = run(timeout => 600, + $res = runCommand(timeout => 600, cmd => ["darcs", "get", "--lazy", $uri, $clonePath], dir => $ENV{"TMPDIR"}); die "Error getting darcs repo at `$uri':\n$stderr" if $res->{status}; diff --git a/src/lib/Hydra/Plugin/GitInput.pm b/src/lib/Hydra/Plugin/GitInput.pm index 0de02128..dee6be57 100644 --- a/src/lib/Hydra/Plugin/GitInput.pm +++ b/src/lib/Hydra/Plugin/GitInput.pm @@ -137,8 +137,8 @@ sub fetchInput { my $res; if (! -d $clonePath) { # Clone everything and fetch the branch. - $res = run(cmd => ["git", "init", $clonePath]); - $res = run(cmd => ["git", "remote", "add", "origin", "--", $uri], dir => $clonePath) unless $res->{status}; + $res = runCommand(cmd => ["git", "init", $clonePath]); + $res = runCommand(cmd => ["git", "remote", "add", "origin", "--", $uri], dir => $clonePath) unless $res->{status}; die "error creating git repo in `$clonePath':\n$res->{stderr}" if $res->{status}; } @@ -146,9 +146,9 @@ sub fetchInput { # the remote branch for whatever the repository state is. This command mirrors # only one branch of the remote repository. my $localBranch = _isHash($branch) ? "_hydra_tmp" : $branch; - $res = run(cmd => ["git", "fetch", "-fu", "origin", "+$branch:$localBranch"], dir => $clonePath, + $res = runCommand(cmd => ["git", "fetch", "-fu", "origin", "+$branch:$localBranch"], dir => $clonePath, timeout => $cfg->{timeout}); - $res = run(cmd => ["git", "fetch", "-fu", "origin"], dir => $clonePath, timeout => $cfg->{timeout}) if $res->{status}; + $res = runCommand(cmd => ["git", "fetch", "-fu", "origin"], dir => $clonePath, timeout => $cfg->{timeout}) if $res->{status}; die "error fetching latest change from git repo at `$uri':\n$res->{stderr}" if $res->{status}; # If deepClone is defined, then we look at the content of the repository @@ -156,16 +156,16 @@ sub fetchInput { if (defined $deepClone) { # Is the target branch a topgit branch? - $res = run(cmd => ["git", "ls-tree", "-r", "$branch", ".topgit"], dir => $clonePath); + $res = runCommand(cmd => ["git", "ls-tree", "-r", "$branch", ".topgit"], dir => $clonePath); if ($res->{stdout} ne "") { # Checkout the branch to look at its content. - $res = run(cmd => ["git", "checkout", "--force", "$branch"], dir => $clonePath); + $res = runCommand(cmd => ["git", "checkout", "--force", "$branch"], dir => $clonePath); die "error checking out Git branch '$branch' at `$uri':\n$res->{stderr}" if $res->{status}; # This is a TopGit branch. Fetch all the topic branches so # that builders can run "tg patch" and similar. - $res = run(cmd => ["tg", "remote", "--populate", "origin"], dir => $clonePath, timeout => $cfg->{timeout}); + $res = runCommand(cmd => ["tg", "remote", "--populate", "origin"], dir => $clonePath, timeout => $cfg->{timeout}); print STDERR "warning: `tg remote --populate origin' failed:\n$res->{stderr}" if $res->{status}; } } -- 2.51.0 From b1b3440041e9e18d8e49a2bbc336b8c57dae0792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Fri, 12 Sep 2025 21:37:56 +0200 Subject: [PATCH 3/3] add regression test for download api --- t/Hydra/Controller/Build/download.t | 74 +++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 t/Hydra/Controller/Build/download.t diff --git a/t/Hydra/Controller/Build/download.t b/t/Hydra/Controller/Build/download.t new file mode 100644 index 00000000..1839f172 --- /dev/null +++ b/t/Hydra/Controller/Build/download.t @@ -0,0 +1,74 @@ +use strict; +use warnings; +use Setup; +use Test2::V0; +use Catalyst::Test (); +use HTTP::Request::Common; + +my %ctx = test_init(); + +Catalyst::Test->import('Hydra'); + +my $db = Hydra::Model::DB->new; +hydra_setup($db); + +my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"}); + +# Create a simple Nix expression that uses the existing build-product-simple.sh +my $jobsdir = $ctx{jobsdir}; +my $nixfile = "$jobsdir/simple.nix"; +open(my $fh, '>', $nixfile) or die "Cannot create simple.nix: $!"; +print $fh <<"EOF"; +with import ./config.nix; +{ + simple = mkDerivation { + name = "build-product-simple"; + builder = ./build-product-simple.sh; + }; +} +EOF +close($fh); + +# Create a jobset that uses the simple build +my $jobset = createBaseJobset("simple", "simple.nix", $ctx{jobsdir}); + +ok(evalSucceeds($jobset), "Evaluating simple.nix should succeed"); +is(nrQueuedBuildsForJobset($jobset), 1, "Should have 1 build queued"); + +my $build = (queuedBuildsForJobset($jobset))[0]; +ok(runBuild($build), "Build should succeed"); + +$build->discard_changes(); + +subtest "Test downloading build products (regression test for #1520)" => sub { + # Get the build URL + my $build_id = $build->id; + + # First, check that the build has products + my @products = $build->buildproducts; + ok(scalar @products >= 1, "Build should have at least 1 product"); + + # Find the doc product (created by build-product-simple.sh) + my ($doc_product) = grep { $_->type eq "doc" } @products; + ok($doc_product, "Should have a doc product"); + + if ($doc_product) { + # Test downloading via the download endpoint + # This tests the serveFile function which was broken in #1520 + my $download_url = "/build/$build_id/download/" . $doc_product->productnr . "/text.txt"; + my $response = request(GET $download_url); + + # The key test: should not return 500 error with "Can't use string ("1") as a HASH ref" + isnt($response->code, 500, "Download should not return 500 error (regression test for #1520)"); + is($response->code, 200, "Download should succeed with 200") + or diag("Response code: " . $response->code . ", Content: " . $response->content); + + like($response->header('Content-Security-Policy') // '', qr/\bsandbox\b/, 'CSP header present with sandbox'); + + # Check that we get actual content + ok(length($response->content) > 0, "Should receive file content"); + is($response->content, "Hello\n", "Should get expected content"); + } +}; + +done_testing(); -- 2.51.0