Run the download tests against a local HTTP server

CHANGES

@@ -1,5 +1,13 @@
 This file describes changes in the PackageManager package.
 
+unreleased
+  - Report the underlying error when a download fails (at InfoPackageManager
+    level 2)
+  - Test the GetPackageURLs failure paths against a local HTTP server instead
+    of a live website, which was a frequent source of spurious test failures
+  - Test updating a package installed in a directory without a version number
+    against the same local server, instead of downloading transgrp twice
+
 1.6.3 (2025-05-14)
   - Alter tests for compatibility with AutoDoc 2025.05.09
 

PackageInfo.g

@@ -87,6 +87,7 @@ Dependencies := rec(
   SuggestedOtherPackages := [ [ "GAPDoc", ">= 1.6.1" ],
                               [ "AutoDoc", ">= 2025.05.09" ],
                               [ "curlInterface", ">= 2.1.0" ] ],
+  TestPackages := [ [ "io", ">= 4.7.0" ] ],
   ExternalConditions := [ ],
 ),
 

gap/distro.gi

@@ -111,6 +111,8 @@ function()
   urls := rec(success := false);
   if not get.success then
     Info(InfoPackageManager, 1, "Could not contact server");
+    Info(InfoPackageManager, 2, "Tried to download ", PKGMAN_PackageInfoURLList);
+    PKGMAN_InfoDownloadError(get);
     return urls;
   fi;
   for line in SplitString(get.result, "\n") do

gap/download.gd

@@ -1,3 +1,4 @@
+DeclareGlobalFunction("PKGMAN_InfoDownloadError");
 DeclareGlobalFunction("PKGMAN_DownloadUrlToTempFile");
 DeclareGlobalFunction("PKGMAN_DownloadURL");
 DeclareGlobalFunction("PKGMAN_DownloadPackageInfo");

gap/download.gi

@@ -1,10 +1,21 @@
+# Report why a download failed.  This is only shown at info level 2, so that
+# the (tested) info level 1 output stays stable, but it makes transient
+# network problems diagnosable after the fact, e.g. in CI logs.
+InstallGlobalFunction(PKGMAN_InfoDownloadError,
+function(get)
+  if IsRecord(get) and IsBound(get.error) then
+    Info(InfoPackageManager, 2, "Download error: ", get.error);
+  fi;
+end);
+
 InstallGlobalFunction(PKGMAN_DownloadUrlToTempFile,
 function(url)
   local get, url_parts, filename, path;
   Info(InfoPackageManager, 3, "Downloading archive from URL ", url, " ...");
   get := PKGMAN_DownloadURL(url);
   if get.success <> true then
     Info(InfoPackageManager, 1, "Could not download from ", url);
+    PKGMAN_InfoDownloadError(get);
     return fail;
   fi;
   url_parts := SplitString(url, "/");
@@ -18,7 +29,7 @@ end);
 
 InstallGlobalFunction(PKGMAN_DownloadURL,
 function(url)
-  local tool, exec;
+  local tool, exec, errors;
 
   # Use curlInterface if available
   if TestPackageAvailability("curlInterface", PKGMAN_CurlIntReqVer) = true then
@@ -27,19 +38,27 @@ function(url)
   fi;
 
   # Try command line tools (wget/curl)
+  errors := [];
   for tool in PKGMAN_DownloadCmds do
     Info(InfoPackageManager, 4, "Using ", tool[1], " to download...");
     exec := CallFuncList(PKGMAN_Exec, Concatenation(["."], [tool[1]], tool[2], [url]));
     if exec = fail then
       Info(InfoPackageManager, 4, tool[1], " unavailable");
+      Add(errors, Concatenation(tool[1], " unavailable"));
     elif exec.code <> 0 then
       Info(InfoPackageManager, 4, "Download failed with ", tool[1]);
+      Add(errors, Concatenation(tool[1], " failed with exit code ",
+                                String(exec.code)));
     else
       return rec(success := true, result := exec.output);
     fi;
   od;
 
-  return rec(success := false, error := "no download method is available");
+  if IsEmpty(errors) then
+    return rec(success := false, error := "no download method is available");
+  fi;
+  return rec(success := false,
+             error := JoinStringsWithSeparator(errors, "; "));
 end);
 
 InstallGlobalFunction(PKGMAN_DownloadPackageInfo,
@@ -50,6 +69,7 @@ function(url)
   get := PKGMAN_DownloadURL(url);
   if not get.success then
     Info(InfoPackageManager, 1, "Unable to download from ", url);
+    PKGMAN_InfoDownloadError(get);
     return fail;
   fi;
   info := PKGMAN_GetPackageInfo(InputTextString(get.result));

tst/archive.tst

@@ -21,23 +21,35 @@ gap> InstallPackage("https://gap-packages.github.io/PackageManager/dummy/badpack
 #I  PackageInfo.g lacks PackageName field
 false
 
-# Updating old package that doesn't have the version number in its directory name
-gap> InstallPackage("https://www.math.colostate.edu/~hulpke/transgrp/transgrp3.6.4.tar.gz");
+# Updating old package that doesn't have the version number in its directory
+# name.  We use a dummy package served by a local HTTP server: the real-world
+# example used to be transgrp, but downloading it twice meant fetching 120MB
+# from a third-party server on every test run.
+gap> LoadPackage("io", false);
 true
-gap> oldinfo := First(PackageInfo("transgrp"), x -> x.Version = "3.6.4");;
+gap> ReadPackage("PackageManager", "tst/http-server.g");
+true
+gap> server := PKGMAN_StartHTTPTestServer(PKGMAN_PrepareTestData());;
+gap> InstallPackage(Concatenation(server.url, "/pmdummy-1.0.tar.gz"));
+true
+gap> oldinfo := First(PackageInfo("pmdummy"), x -> x.Version = "1.0");;
 gap> oldinfo <> fail;
 true
-gap> PositionSublist(oldinfo.InstallationPath, "3.6.4");  # version number not in dir name
+gap> PositionSublist(oldinfo.InstallationPath, "1.0");  # version number not in dir name
 fail
-gap> UpdatePackage("transgrp", false);  # also removes old version
+gap> urllist := PKGMAN_PackageInfoURLList;;
+gap> PKGMAN_PackageInfoURLList := Concatenation(server.url, "/pkglist.csv");;
+gap> UpdatePackage("pmdummy", false);  # also removes old version
 #I  Package already installed at target location
 #I  Appending '.old' to old version directory
 true
-gap> newinfo := PKGMAN_UserPackageInfo("transgrp")[1];;
-gap> CompareVersionNumbers(newinfo.Version, ">=3.6.5");
-true
-gap> RemovePackage("transgrp", false);
+gap> PKGMAN_PackageInfoURLList := urllist;;
+gap> newinfo := PKGMAN_UserPackageInfo("pmdummy")[1];;
+gap> newinfo.Version;
+"2.0"
+gap> RemovePackage("pmdummy", false);
 true
+gap> PKGMAN_StopHTTPTestServer(server);
 
 # Install to existing empty directory
 gap> CreateDir(Filename(Directory(PKGMAN_PackageDir()), "Toric-1.9.5"));

tst/data/badurls.txt

@@ -0,0 +1,2 @@
+# comment lines are ignored
+<!DOCTYPE html> <html lang="en-US"> <head> <meta charset="UTF-8"> <meta http-equiv="X-UA-Compatible">

tst/data/new/pmdummy/PackageInfo.g

@@ -0,0 +1,55 @@
+#
+# A minimal dummy package for the PackageManager tests.  It is served by the
+# test HTTP server (see tst/http-server.g); "@SERVER@" is replaced by the
+# address of that server when the file is served, but not inside the tarball.
+#
+# Note that the tarball of this package unpacks into a directory whose name
+# does not contain the version number, which is what we want to test.
+#
+SetPackageInfo( rec(
+
+PackageName := "pmdummy",
+Subtitle := "A dummy package for the PackageManager tests",
+Version := "2.0",
+Date := "01/01/2020",  # dd/mm/yyyy format
+License := "GPL-2.0-or-later",
+
+Persons := [
+  rec(
+    IsAuthor := true,
+    IsMaintainer := true,
+    FirstNames := "The GAP",
+    LastName := "Team",
+    Email := "support@gap-system.org",
+  ),
+],
+
+PackageWWWHome := "http://@SERVER@/",
+ArchiveURL := "http://@SERVER@/pmdummy-2.0",
+README_URL := "http://@SERVER@/README.md",
+PackageInfoURL := "http://@SERVER@/PackageInfo.g",
+ArchiveFormats := ".tar.gz",
+
+Status := "other",
+
+AbstractHTML := "A dummy package for the PackageManager tests",
+
+PackageDoc := rec(
+  BookName := "pmdummy",
+  ArchiveURLSubset := ["doc"],
+  HTMLStart := "doc/chap0.html",
+  PDFFile := "doc/manual.pdf",
+  SixFile := "doc/manual.six",
+  LongTitle := "A dummy package for the PackageManager tests",
+),
+
+Dependencies := rec(
+  GAP := ">= 4.12",
+  NeededOtherPackages := [],
+  SuggestedOtherPackages := [],
+  ExternalConditions := [],
+),
+
+AvailabilityTest := ReturnTrue,
+
+));

tst/data/new/pmdummy/README.md

@@ -0,0 +1,4 @@
+# pmdummy
+
+A minimal dummy package used by the PackageManager tests.  It is never
+loaded; it only needs to be installable.

tst/data/new/pmdummy/doc/chap0.html

@@ -0,0 +1,3 @@
+<html><head><title>pmdummy</title></head>
+<body>Dummy manual, so that the tests need not build any documentation.</body>
+</html>

tst/data/new/pmdummy/doc/manual.six

@@ -0,0 +1,2 @@
+#SIXFILE
+# Not a real manual.six; a placeholder so that ValidatePackageInfo is happy.

tst/data/new/pmdummy/init.g

@@ -0,0 +1,3 @@
+#
+# pmdummy: a dummy package for the PackageManager tests
+#

tst/data/new/pmdummy/read.g

@@ -0,0 +1,3 @@
+#
+# pmdummy: a dummy package for the PackageManager tests
+#

tst/data/old/pmdummy/PackageInfo.g

@@ -0,0 +1,55 @@
+#
+# A minimal dummy package for the PackageManager tests.  It is served by the
+# test HTTP server (see tst/http-server.g); "@SERVER@" is replaced by the
+# address of that server when the file is served, but not inside the tarball.
+#
+# Note that the tarball of this package unpacks into a directory whose name
+# does not contain the version number, which is what we want to test.
+#
+SetPackageInfo( rec(
+
+PackageName := "pmdummy",
+Subtitle := "A dummy package for the PackageManager tests",
+Version := "1.0",
+Date := "01/01/2020",  # dd/mm/yyyy format
+License := "GPL-2.0-or-later",
+
+Persons := [
+  rec(
+    IsAuthor := true,
+    IsMaintainer := true,
+    FirstNames := "The GAP",
+    LastName := "Team",
+    Email := "support@gap-system.org",
+  ),
+],
+
+PackageWWWHome := "http://@SERVER@/",
+ArchiveURL := "http://@SERVER@/pmdummy-1.0",
+README_URL := "http://@SERVER@/README.md",
+PackageInfoURL := "http://@SERVER@/PackageInfo.g",
+ArchiveFormats := ".tar.gz",
+
+Status := "other",
+
+AbstractHTML := "A dummy package for the PackageManager tests",
+
+PackageDoc := rec(
+  BookName := "pmdummy",
+  ArchiveURLSubset := ["doc"],
+  HTMLStart := "doc/chap0.html",
+  PDFFile := "doc/manual.pdf",
+  SixFile := "doc/manual.six",
+  LongTitle := "A dummy package for the PackageManager tests",
+),
+
+Dependencies := rec(
+  GAP := ">= 4.12",
+  NeededOtherPackages := [],
+  SuggestedOtherPackages := [],
+  ExternalConditions := [],
+),
+
+AvailabilityTest := ReturnTrue,
+
+));

tst/data/old/pmdummy/README.md

@@ -0,0 +1,4 @@
+# pmdummy
+
+A minimal dummy package used by the PackageManager tests.  It is never
+loaded; it only needs to be installable.

tst/data/old/pmdummy/doc/chap0.html

@@ -0,0 +1,3 @@
+<html><head><title>pmdummy</title></head>
+<body>Dummy manual, so that the tests need not build any documentation.</body>
+</html>

tst/data/old/pmdummy/doc/manual.six

@@ -0,0 +1,2 @@
+#SIXFILE
+# Not a real manual.six; a placeholder so that ValidatePackageInfo is happy.

tst/data/old/pmdummy/init.g

@@ -0,0 +1,3 @@
+#
+# pmdummy: a dummy package for the PackageManager tests
+#

tst/data/old/pmdummy/read.g

@@ -0,0 +1,3 @@
+#
+# pmdummy: a dummy package for the PackageManager tests
+#

tst/data/pkglist.csv

@@ -0,0 +1,2 @@
+# A package URLs list, in the format used by the GAP package distribution.
+pmdummy http://@SERVER@/PackageInfo.g

tst/distro.tst

@@ -76,18 +76,30 @@ gap> InstallRequiredPackages();
 false
 gap> GAPInfo.Dependencies := rec(NeededOtherPackages := backup);;
 
-# GetPackageURLs failure
+# GetPackageURLs failures.  These use a local HTTP server rather than any
+# remote one, since otherwise a transient network problem makes the "server
+# unreachable" case indistinguishable from the "bad content" case.
+gap> LoadPackage("io", false);
+true
+gap> ReadPackage("PackageManager", "tst/http-server.g");
+true
+gap> server := PKGMAN_StartHTTPTestServer(PKGMAN_PrepareTestData());;
 gap> default_url := PKGMAN_PackageInfoURLList;;
-gap> PKGMAN_PackageInfoURLList := "http://www.nothing.rubbish/abc.txt";;
+
+# The server cannot be contacted (nothing is listening on that port)
+gap> PKGMAN_PackageInfoURLList := PKGMAN_UnusedURL();;
 gap> GetPackageURLs();
 #I  Could not contact server
 rec( success := false )
-gap> PKGMAN_PackageInfoURLList := "https://www.gap-system.org";;
+
+# The server answers, but with something that is not a package URLs list
+gap> PKGMAN_PackageInfoURLList := Concatenation(server.url, "/badurls.txt");;
 gap> GetPackageURLs();
 #I  Bad line in package URLs list:
 #I  <!DOCTYPE html> <html lang="en-US"> <head> <meta charset="UTF-8"> <meta...
 rec( success := false )
 gap> PKGMAN_PackageInfoURLList := default_url;;
+gap> PKGMAN_StopHTTPTestServer(server);
 
 # InstallPackageFromName failure
 gap> InstallPackage("sillypackage");