From e810127ea8f16d9160a50f6cb67d69ca98f20d1f Mon Sep 17 00:00:00 2001 From: Fabrizio Ferrai Date: Sun, 22 Mar 2026 22:47:41 +0200 Subject: [PATCH 1/2] Fix matrix job cascading for new compiler releases --- .gitignore | 1 + app-e2e/src/Test/E2E/Endpoint/Startup.purs | 24 ++++ app-e2e/src/Test/E2E/Support/Fixtures.purs | 10 +- .../github-packages/console-6.1.0/bower.json | 2 +- app/fixtures/registry-index/ef/fe/effect | 1 + .../registry-storage/effect-4.0.1.tar.gz | Bin 0 -> 5906 bytes app/fixtures/registry/metadata/effect.json | 18 +++ app/src/App/Server/MatrixBuilder.purs | 23 ++-- app/test/App/API.purs | 8 +- app/test/App/Server/MatrixBuilder.purs | 109 ++++++++++++++++++ app/test/Main.purs | 4 + app/test/Test/Assert/Run.purs | 9 ++ lib/src/ManifestIndex.purs | 17 ++- lib/test/Registry/ManifestIndex.purs | 40 +++++++ nix/test/config.nix | 73 ++++++++++++ 15 files changed, 316 insertions(+), 23 deletions(-) create mode 100644 app/fixtures/registry-index/ef/fe/effect create mode 100644 app/fixtures/registry-storage/effect-4.0.1.tar.gz create mode 100644 app/fixtures/registry/metadata/effect.json create mode 100644 app/test/App/Server/MatrixBuilder.purs diff --git a/.gitignore b/.gitignore index 9d091519f..ba8b03a7a 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ result* TODO.md .spec-results +/generated-docs # Keep it secret, keep it safe. .env diff --git a/app-e2e/src/Test/E2E/Endpoint/Startup.purs b/app-e2e/src/Test/E2E/Endpoint/Startup.purs index 5f91ac30b..8dfc6a5f1 100644 --- a/app-e2e/src/Test/E2E/Endpoint/Startup.purs +++ b/app-e2e/src/Test/E2E/Endpoint/Startup.purs @@ -14,6 +14,7 @@ import Registry.Test.Assert as Assert import Registry.Test.Utils as Utils import Test.E2E.Support.Client as Client import Test.E2E.Support.Env (E2ESpec) +import Test.E2E.Support.Env as Env import Test.Spec as Spec spec :: E2ESpec @@ -47,3 +48,26 @@ spec = do <> show (Array.length matrixJobs) <> " matrix jobs for packages: " <> String.joinWith ", " (map PackageName.print matrixPackages) + + Spec.it "cascades matrix jobs to dependant packages after dependency-free jobs complete" do + -- Wait for ALL pending jobs: the dependency-free matrix jobs and any + -- cascade jobs they trigger. + Env.waitForAllPendingJobs + + -- Check that effect got a matrix job with the new compiler (0.15.11). + -- effect depends on prelude, so after prelude's 0.15.11 matrix job + -- succeeds, the cascade should enqueue a job for effect. + allJobs <- Client.getJobsWith Client.IncludeCompleted + let + effectName = Utils.unsafePackageName "effect" + newCompiler = Utils.unsafeVersion "0.15.11" + effectCascadeJobs = Array.filter + ( case _ of + MatrixJob { packageName, compilerVersion } -> + packageName == effectName && compilerVersion == newCompiler + _ -> false + ) + allJobs + + when (Array.null effectCascadeJobs) do + Assert.fail "Expected cascade matrix job for effect with compiler 0.15.11, but found none." diff --git a/app-e2e/src/Test/E2E/Support/Fixtures.purs b/app-e2e/src/Test/E2E/Support/Fixtures.purs index 296e9baa5..bbf1ea020 100644 --- a/app-e2e/src/Test/E2E/Support/Fixtures.purs +++ b/app-e2e/src/Test/E2E/Support/Fixtures.purs @@ -43,9 +43,9 @@ import Registry.Version (Version) type PackageFixture = { name :: PackageName, version :: Version } --- | effect@4.0.0 fixture package +-- | effect@4.0.1 fixture package (4.0.0 is pre-populated in startup fixtures for cascade testing) effect :: PackageFixture -effect = { name: Utils.unsafePackageName "effect", version: Utils.unsafeVersion "4.0.0" } +effect = { name: Utils.unsafePackageName "effect", version: Utils.unsafeVersion "4.0.1" } -- | console@6.1.0 fixture package console :: PackageFixture @@ -59,8 +59,8 @@ prelude = { name: Utils.unsafePackageName "prelude", version: Utils.unsafeVersio slug :: PackageFixture slug = { name: Utils.unsafePackageName "slug", version: Utils.unsafeVersion "3.0.0" } --- | Standard publish data for effect@4.0.0, used by E2E tests. --- | This matches the fixtures in app/fixtures/github-packages/effect-4.0.0 +-- | Standard publish data for effect@4.0.1, used by E2E tests. +-- | This matches the fixtures in app/fixtures/github-packages/effect-4.0.1 effectPublishData :: Operation.PublishData effectPublishData = { name: effect.name @@ -69,7 +69,7 @@ effectPublishData = , repo: "purescript-effect" , subdir: Nothing } - , ref: "v4.0.0" + , ref: "v4.0.1" , compiler: Just $ Utils.unsafeVersion "0.15.10" , resolutions: Nothing , version: effect.version diff --git a/app/fixtures/github-packages/console-6.1.0/bower.json b/app/fixtures/github-packages/console-6.1.0/bower.json index da93c7f6e..6145278d2 100644 --- a/app/fixtures/github-packages/console-6.1.0/bower.json +++ b/app/fixtures/github-packages/console-6.1.0/bower.json @@ -16,7 +16,7 @@ "package.json" ], "dependencies": { - "purescript-effect": "^4.0.0", + "purescript-effect": "^4.0.1", "purescript-prelude": "^6.0.0" } } diff --git a/app/fixtures/registry-index/ef/fe/effect b/app/fixtures/registry-index/ef/fe/effect new file mode 100644 index 000000000..402347381 --- /dev/null +++ b/app/fixtures/registry-index/ef/fe/effect @@ -0,0 +1 @@ +{"name":"effect","version":"4.0.0","license":"BSD-3-Clause","location":{"githubOwner":"purescript","githubRepo":"purescript-effect"},"ref":"v4.0.0","description":"Native side effects","dependencies":{"prelude":">=6.0.0 <7.0.0"}} diff --git a/app/fixtures/registry-storage/effect-4.0.1.tar.gz b/app/fixtures/registry-storage/effect-4.0.1.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..13cffd1cf5a406e5b8dd4394107eef28f1db34fe GIT binary patch literal 5906 zcmV+t7wzaDiwFP!000001MOXHbK5wQp3nLf*wj_6l%W+_uXdcIO0nfcTaN6L|9EiMO=I57?fX`GfQ{w$kKK;u10Q|w)1DJ0bjhe?!kW)h?u5*%d8bKY-_>rA(!Ao8|uBRR1WJT3dtq*(#K-h3#bQ; zNgV3wfDsAmbNdEw`661j#|)~$J?2(Spo9ceUe+Q81lSp-&NkGkS1QdU0AQqOzy|=L zEp`AZ2SWuhkb7bvQm%z<>^9W$biZL<*iOZUI}X|C9(9;*x}kn~qwvQR=pat3PDYgR zr|_<&Zzy8H?;+f^Z7!gKWrvlx+L`AfYC+F-+{#A3BXf!t0{Cr0tp!}AXbb>kV|$bc zt-y0YkljGFCEVa`0gbryZn34M;Z)o$HPCY4A%^zkpv_pqdR$lFpS~YX$mH^T`msMA z5cqyIzWi-?HaH`1KauJC0XeV>0}3buk2DKz-aFO^1VtMuwx)i|e!D=$%G@02y6Q$;I%) za0=C?ml|g(AFG6qk;`-PVK6>@4?p{FhZn=?Cl2I%I2|Fcb70mdSN-vHczS)&ACs%= z@zv#IKtQa@*>G}t(I0*ooYjFla3q7@2BRsNyzgIJq)Lg{BQm}n0LcE^ive=tQl1UR zgVQOB6Mj1d%>lj(jZCfvr$hWQ`1=5a>5o5YlICRapVv?cZj!V9hyJ_4q*C3Z5@?gt z>+#?NVg@}-uHR0k!|C;OK;B(mo^eG_2IJo#@JvoB7nc*Rt?S7^0}j(ZXAC4jRg)9^ z{`Pt@~yMD1Ad^usUjm84c=W0-wj5m1H5~QR6Y(TgBrBeaDo+pB(nY32cFkl zWNafqCB6~`8^{C|q_uL>d0(5pXtNuF&X`nx>(9|*g6rs3_iy`fuu#jC76 zZ|lB%@nQvzdN;2d_U1+S)Qb!PCPX~T<V~{&l1P1io=rYz1))3I9XTeE$4J{khM88~^V>aBEu-TV4OU1tW{! zVIZ?JX>bif)OHs@^Qh}ZsPHc2U)k!$7f_zK2ABTHp)BYYLL{{e22=yN75rO=n!5$p z!hd(6cQ&{fm2Z#i0wVn%*t?J4{rC3&Rp|b>8aac1;~xsW2D`J03a3SsvT0pXwf6!6=I7L3lMK@yOeCO z2I6AInz*|yr)a@guhLdr3MBf6>t;0ZJf}BbTIT>GYiVUeheL+(< z_*@>&zrnkYkCW(t)PlMQk&}`*6cv*VdG(qoN`wod1qlg(l9w-00lH}tz3#V$P7dBc zZ5`TLs!T8ux5Ecj;_@Sf!)xLjMmfg%)08a#e{C5%$6<5z`YP(3_V#OE@Lva&>9d_?wnP!9iO=o$A+`d;#-s;H7Hq z1Yf#ND)6t8!KsuMC!Z`brD&M|OBb^M5t z;~ix*Ud}k||7+u|^II>!jrX)Nj#QHkozm1#+xo{Dt)DaE{WouYwy*j3naO@b!Pkd0 zTS>K+pKe0YHkldy%n-Jd?`#fx(u`Xsn+;t^M8Bif4_eXFn|8H z)#x1_#^-_{-}D<@UJBR?YVOWhBSgN<+_vaTbscXu`wS0?9Y;~{#I#o*sO#!0 zS~cknH8rvkKhewZs)R=Z+oW}sQWEo7+IzyTOCwm>`dL!Ffo9N zT!>l7e1==zd2)rn&`H>4Ll?NVXxK7qS%(BA0 zY~m99Hxy5+0)ReWO3Sky%2pt=ST7Gv02g3oGaS58zDaSgY0|7K#kgO*@FAc83U4v8 zaq9}xa6}_aTQU21+)Z|j^w=ESBssn7B&_x=`0ju6KckWghe5ry4IzDJp?BF?ynko} zQ9!@nd1zx`X>%KMVHh-xy%7ma)NyR*VC#TysH^mZH?%>?4kqP5uYztQ_Z&d47fCea z?Va3TfQ@oz?|1@>Y6;}I%mQL2^X>vKhSwabo6)`=*loheX@Ob*b3i%U)ds$mxuTc& z`y511QuDlg!dOjHko$Hrx4}@Ht%sc$z38HiX;{(2lW9!fxX79jDebqNx zP~4mvpy40ZDTcrVJ4QI)ope}_-zzxR*9wd@7N5Wha8n3Sw0OeYOr3w9p$W(>DbUOo zGeDDauk9XXJ-{VII&@@tASzUv9Mv!&aWh);C9Bfi+4ts%#{mNmYy$Kx z4hkQHJNM4@V7Niv+gl1F8o=WlJ4gusFm!ny*slq4gpR_qZB?H4)S-XH%C3n?awfuc zKp%@6+(WbQJ3*aS)UYq$CTT>48#?<(ekJE%l`eoN{884=YPecj9f1fs4(?S;sd4{R3m(XF<$Y_xUq0p(VQLW%j$Ikd z*P%EvBre0=xM$@5KyQDJo=>{9xU&}>7dlP6Gh>b|qDW17ATPLx4&*x!-$cplV92Ph^2RhAeYUFZK??dqmQ}B)1rFZCv}BadHQ-&6 za`lEoBR=xsFSASzh~(G~-M#;iST`T1+Snf>K4TEmst!o5s ze#H~CU1hiyzR3yiD)+5&IhM?Ez36j}OdmJA9SG=i`m5_x^Q#I1rtZqOVgEXcC!8`L ziQ~O`AX(fCF_F6|U9i!45i9A<6h;cRxDRCY9%Ai#@esI7iB*pp;PSm+#)yqG^k$U<8{kqky1y7iF zJ*qEo4o?@euX2u8-{RRmWO+!In93B!ixK$`RU0F$^6E|OQd6OXvxPQUQ*$d%w%lxS zJsbDS*ei?u4-MAHaKeyo;7c&Ui_iaUqRb^%0I?+? z8ZnS9AEXh3*^)4!W90yC7Xzw<&jL`}KGa1FY&!(Ih{0_KaKQ!S0PYk69ErOHAa_E@ zGzPk(f~GOp9R)U_dBD5HfXAYB0q9*7`Z5N-8wJ0N!S6=kgS*WUpjS+QM1U_KLN6-9 zDkelPF2pJ(MlU7?1dJR(j*1DAG9V}*%28aDbxfF}q%iB4I7bO_AoS%3bX-iJ)X_r$ zk&crhv6xWDX+p7>SjQ=1K`_q?K#hVCC}W&aBq%k~r28C?Q;l>1KgUB?BUMHit3=MZ zvDqRLW98Z&7tdek%ZRRGeJ>tYe@DFHRwk$BK2C{nnZm1yyozvZ?cLZJ%cmuAO~cGa^w6-%iN42b1)JpQk91q{ba@^eeGXNxsr6sf~V;puf2v4@G&zRJ5}E zgtDjqKgt21`}%*Y9Y6oOg~!E~{r?F{eD52!dTisHbOf=ZJI6ad+a>G=ci7>~+D@z# ztGl|Nk8F=Vf5|t;^4;c`k>Yz4oN^Pp#`)Qq&YXD;{xKbwEvP&K+^xqaL({xe=INGt z>7H^+M*n}Dz%R?5{(so+#ruEruv?!0|0E?6|8Z;iAQJKQ)Z+_CHFkV~o^?#ZoLP%v z6ccCsC3qEB{t_J#g-B`zL#{CYXiC}JM1amrcw{B8v<*kJTWVM&`2!7OJL*gs(M`jXdQ}TW+IJM=EU9|qz=$UOGNGn_-f+#oRSbj?R@mw(oe~>2 z#E2^LcB1ZxRoB<~P$^*e7JkJ!VGZEKD<(LJkyd15Nc2r8vK4n;Q-7qN{~w(E@AsK7 z?`{?>|IA3(U_mE#afgF_>+%y3&`rR*qIYZicQrhSMG)f$n20QL{6nO{-(#@QT0tb9 z1jiitlIV)uX(id1o2H)>mxijIxv&*o)Y4NIwTPZryb`9?ls~0ThD$Cg&uiEDU$np} zUp<7O387|kYBxB!fY)*5p*xEpACV_{%9-6+J$Nq*vzK zW27e(u84(T67vBZP^iCG`U zaVdop(;hxw4ZUhU5ip-wm{4khATB2du|o?sH~Doj%|D?kDNovLNm6Ngjsk7W;ph5u zsj=g3(XdUEiqSWO4QscV$Ny{!e^oJUcwr#RStzTDS~NNIg!x_cD4^L=q)lT)7UX1X zVir8?1}C1(OnXME%rZ=H6*sgd@~_tnF4C|5#e`lsGSCWyI22lbROP3lgW6u5;53$_ z-yuA1xCs@A*PE~I$kEM+N6S;|tDvXrGPWhqNp%2JlHlqJ9XAJ8lfiU6Ph0L7_|WB>pF literal 0 HcmV?d00001 diff --git a/app/fixtures/registry/metadata/effect.json b/app/fixtures/registry/metadata/effect.json new file mode 100644 index 000000000..6d25580f7 --- /dev/null +++ b/app/fixtures/registry/metadata/effect.json @@ -0,0 +1,18 @@ +{ + "location": { + "githubOwner": "purescript", + "githubRepo": "purescript-effect" + }, + "published": { + "4.0.0": { + "bytes": 5904, + "compilers": [ + "0.15.10" + ], + "hash": "sha256-6a6UH+Q1C86LCmHWiIq/Xh/2+vHRS69ZeEsQAXrfFQs=", + "publishedTime": "2022-08-18T20:04:00.000Z", + "ref": "v4.0.0" + } + }, + "unpublished": {} +} diff --git a/app/src/App/Server/MatrixBuilder.purs b/app/src/App/Server/MatrixBuilder.purs index d46cecf16..f0431c4aa 100644 --- a/app/src/App/Server/MatrixBuilder.purs +++ b/app/src/App/Server/MatrixBuilder.purs @@ -211,15 +211,22 @@ solveDependantsForCompiler { compilerIndex, name, version, compiler } = do manifestIndex <- Registry.readAllManifests let dependentManifests = ManifestIndex.dependants manifestIndex name version newJobs <- for dependentManifests \(Manifest manifest) -> do - -- we first verify if we have already attempted this package with this compiler, - -- either in the form of having it in the metadata already, or as a failed compilation - -- (i.e. if we find compilers in the metadata for this version we only check this one - -- if it's newer, because all the previous ones have been tried) + -- We skip if this compiler is already in the package's metadata compilers + -- list (meaning it was already successfully tested). Failed compilations + -- are not recorded in metadata, but the DB deduplication in insertMatrixJob + -- prevents re-enqueuing jobs that already exist. shouldAttemptToCompile <- Registry.readMetadata manifest.name >>= case _ of - Nothing -> pure false - Just metadata -> pure $ case Map.lookup version (un Metadata metadata).published of - Nothing -> false - Just { compilers } -> any (_ > compiler) compilers + Nothing -> do + Log.debug $ "Skipping " <> PackageName.print manifest.name <> "@" <> Version.print manifest.version <> ": no metadata found" + pure false + Just metadata -> do + let + result = case Map.lookup manifest.version (un Metadata metadata).published of + Nothing -> false + Just { compilers } -> all (_ /= compiler) compilers + unless result do + Log.debug $ "Skipping " <> PackageName.print manifest.name <> "@" <> Version.print manifest.version <> ": compiler " <> Version.print compiler <> " already tested or version not published" + pure result case shouldAttemptToCompile of false -> pure Nothing true -> do diff --git a/app/test/App/API.purs b/app/test/App/API.purs index 4476f7c53..86ff17172 100644 --- a/app/test/App/API.purs +++ b/app/test/App/API.purs @@ -174,10 +174,12 @@ spec = do copyFixture "registry" copyFixture "registry-storage" copyFixture "github-packages" - -- We remove effect-4.0.0.tar.gz since the unit test publishes it from - -- scratch and will fail if it's already in storage. We have it in - -- storage for the separate integration tests. + -- We remove effect fixtures since the unit test publishes effect from + -- scratch and will fail if it's already registered. We have these in + -- fixtures for the separate integration tests (cascade testing). FS.Extra.remove $ Path.concat [ testFixtures, "registry-storage", "effect-4.0.0.tar.gz" ] + FS.Extra.remove $ Path.concat [ testFixtures, "registry", "metadata", "effect.json" ] + FS.Extra.remove $ Path.concat [ testFixtures, "registry-index", "ef" ] let readFixtures = do diff --git a/app/test/App/Server/MatrixBuilder.purs b/app/test/App/Server/MatrixBuilder.purs new file mode 100644 index 000000000..ecfabb9c0 --- /dev/null +++ b/app/test/App/Server/MatrixBuilder.purs @@ -0,0 +1,109 @@ +module Test.Registry.App.Server.MatrixBuilder (spec) where + +import Registry.App.Prelude + +import Data.Map as Map +import Data.Set as Set +import Effect.Ref as Ref +import Registry.App.Server.MatrixBuilder as MatrixBuilder +import Registry.ManifestIndex as ManifestIndex +import Registry.PackageName as PackageName +import Registry.Solver as Solver +import Registry.Test.Assert.Run (runRegistryMock) +import Registry.Test.Utils as Utils +import Test.Spec as Spec +import Test.Spec.Assertions as Assert + +spec :: Spec.Spec Unit +spec = do + Spec.describe "solveDependantsForCompiler" do + Spec.it "cascades to dependant for a new compiler" do + let { solverData, index, metadata } = setup [ "0.15.10" ] + result <- runSolver solverData index metadata + let names = Set.map _.name result + unless (Set.member effectName names) do + Assert.fail $ "Expected cascade to effect, but got: " + <> show (Set.map PackageName.print names) + + Spec.it "skips dependant already tested with this compiler" do + let { solverData, index, metadata } = setup [ "0.15.10", "0.15.11" ] + result <- runSolver solverData index metadata + unless (Set.isEmpty result) do + Assert.fail "Expected empty result set when compiler already tested" + + where + preludeName = Utils.unsafePackageName "prelude" + effectName = Utils.unsafePackageName "effect" + + compiler_0_15_10 = Utils.unsafeVersion "0.15.10" + compiler_0_15_11 = Utils.unsafeVersion "0.15.11" + allCompilers = Utils.unsafeNonEmptyArray [ compiler_0_15_10, compiler_0_15_11 ] + + preludeVersion = Utils.unsafeVersion "6.0.0" + + preludeManifest = Utils.unsafeManifest "prelude" "6.0.0" [] + effectManifest = Utils.unsafeManifest "effect" "4.0.0" + [ Tuple "prelude" ">=6.0.0 <7.0.0" ] + + dummySha = Utils.unsafeSha256 "sha256-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa=" + dummyTime = Utils.unsafeDateTime "2022-08-18T20:04:00.000Z" + + mkPublishedMeta version compilers = + Map.singleton version + { bytes: 1000.0 + , compilers: Utils.unsafeNonEmptyArray (map Utils.unsafeVersion compilers) + , hash: dummySha + , publishedTime: dummyTime + , ref: Nothing + } + + -- | Set up a test scenario where prelude@6.0.0 (no deps) has completed its + -- | matrix job for 0.15.11, and effect@4.0.0 (depends on prelude) has the + -- | given compilers in its metadata. + setup effectCompilers = do + let + index = Utils.fromRight "Failed to build ManifestIndex" + $ ManifestIndex.insert ManifestIndex.ConsiderRanges preludeManifest ManifestIndex.empty + >>= ManifestIndex.insert ManifestIndex.ConsiderRanges effectManifest + + -- prelude must include 0.15.11 in its compilers because this test + -- simulates the state AFTER prelude's own matrix job has completed. + -- In the real flow: runMatrixJob writes the new compiler into the + -- parent's metadata (MatrixBuilder.purs:82-91), then the executor + -- rebuilds the CompilerIndex from current metadata before calling + -- solveDependantsForCompiler. Without 0.15.11 here, the solver + -- would compute purs >=0.15.10 <0.15.11 for prelude, excluding + -- the target compiler from effect's build plan. + preludeMetadata = Metadata + { location: Git { url: "https://github.com/purescript/purescript-prelude.git", subdir: Nothing } + , owners: Nothing + , published: mkPublishedMeta preludeVersion [ "0.15.10", "0.15.11" ] + , unpublished: Map.empty + } + + effectMetadata = Metadata + { location: Git { url: "https://github.com/purescript/purescript-effect.git", subdir: Nothing } + , owners: Nothing + , published: mkPublishedMeta (Utils.unsafeVersion "4.0.0") effectCompilers + , unpublished: Map.empty + } + + metadata = Map.fromFoldable [ Tuple preludeName preludeMetadata, Tuple effectName effectMetadata ] + + compilerIndex = Solver.buildCompilerIndex allCompilers index metadata + + solverData = + { compilerIndex + , compiler: compiler_0_15_11 + , name: preludeName + , version: preludeVersion + , dependencies: Map.empty + } + + { solverData, index, metadata } + + runSolver solverData index metadata = liftAff do + indexRef <- liftEffect $ Ref.new index + metadataRef <- liftEffect $ Ref.new metadata + runRegistryMock metadataRef indexRef + $ MatrixBuilder.solveDependantsForCompiler solverData diff --git a/app/test/Main.purs b/app/test/Main.purs index cc0deaef9..15cae06e6 100644 --- a/app/test/Main.purs +++ b/app/test/Main.purs @@ -16,6 +16,7 @@ import Test.Registry.App.Legacy.LenientVersion as Test.Legacy.LenientVersion import Test.Registry.App.Legacy.Manifest as Test.Legacy.Manifest import Test.Registry.App.Legacy.PackageSet as Test.Legacy.PackageSet import Test.Registry.App.Manifest.SpagoYaml as Test.Manifest.SpagoYaml +import Test.Registry.App.Server.MatrixBuilder as Test.Server.MatrixBuilder import Test.Spec as Spec import Test.Spec.Reporter.Console (consoleReporter) import Test.Spec.Runner.Node (runSpecAndExitProcess') @@ -49,6 +50,9 @@ main = runSpecAndExitProcess' config [ consoleReporter ] do Spec.describe "Registry.App.Manifest" do Spec.describe "SpagoYaml" Test.Manifest.SpagoYaml.spec + + Spec.describe "Registry.App.Server" do + Spec.describe "MatrixBuilder" Test.Server.MatrixBuilder.spec where config = { defaultConfig: Cfg.defaultConfig { timeout = Just $ Milliseconds 300_000.0 } diff --git a/app/test/Test/Assert/Run.purs b/app/test/Test/Assert/Run.purs index 2925dd72a..43259230e 100644 --- a/app/test/Test/Assert/Run.purs +++ b/app/test/Test/Assert/Run.purs @@ -3,6 +3,7 @@ module Registry.Test.Assert.Run ( TEST_EFFECTS , runBaseEffects + , runRegistryMock , runTestEffects , shouldContain , shouldNotContain @@ -139,6 +140,14 @@ runBaseEffects = do >>> Except.catch (\err -> Run.liftAff (Aff.throwError (Aff.error err))) >>> Run.runBaseAff' +-- | For testing Run functions that only need the REGISTRY effect. +runRegistryMock :: forall a. Ref (Map PackageName Metadata) -> Ref ManifestIndex -> Run (EXCEPT String + LOG + REGISTRY + AFF + EFFECT + ()) a -> Aff a +runRegistryMock metadataRef indexRef = + Registry.interpret (handleRegistryMock { metadataRef, indexRef }) + >>> Log.interpret (\(Log _ _ next) -> pure next) + >>> Except.catch (\err -> Run.liftAff (Aff.throwError (Aff.error err))) + >>> Run.runBaseAff' + runGitHubCacheMemory :: forall r a. CacheRef -> Run (GITHUB_CACHE + LOG + EFFECT + r) a -> Run (LOG + EFFECT + r) a runGitHubCacheMemory = Cache.interpret GitHub._githubCache <<< Cache.handleMemory diff --git a/lib/src/ManifestIndex.purs b/lib/src/ManifestIndex.purs index eb3b08480..2b1a8dba5 100644 --- a/lib/src/ManifestIndex.purs +++ b/lib/src/ManifestIndex.purs @@ -133,10 +133,9 @@ insert consider manifest@(Manifest { name, version, dependencies }) (ManifestInd Left unsatisfied -- | Delete a package version from the manifest index, failing if it produces an --- | invalid manifest index. Since we only verify unsatisfied dependencies wrt --- | package names (and not package versions), it is always acceptable to delete --- | a package version so long as it has at least 2 versions. However, removing --- | a package altogether incurs a full validation check. +-- | invalid manifest index. When considering ranges, we must validate that +-- | removing the version doesn't leave any dependent's ranges unsatisfied, +-- | even if other versions of the same package remain. delete :: IncludeRanges -> PackageName -> Version -> ManifestIndex -> Either (Map PackageName (Map Version (Map PackageName Range))) ManifestIndex delete consider name version (ManifestIndex index) = do case Map.lookup name index of @@ -146,8 +145,14 @@ delete consider name version (ManifestIndex index) = do Tuple _ versions <- Map.toUnfoldableUnordered (Map.delete name index) Tuple _ manifest <- Map.toUnfoldableUnordered versions [ manifest ] - Just _ -> do - pure (ManifestIndex (Map.update (Just <<< Map.delete version) name index)) + Just _ -> case consider of + IgnoreRanges -> + pure (ManifestIndex (Map.update (Just <<< Map.delete version) name index)) + ConsiderRanges -> + fromSet consider $ Set.fromFoldable do + Tuple _ versions <- Map.toUnfoldableUnordered (Map.update (Just <<< Map.delete version) name index) + Tuple _ manifest <- Map.toUnfoldableUnordered versions + [ manifest ] -- | Convert a set of manifests into a `ManifestIndex`. Reports all failures -- | encountered rather than short-circuiting. diff --git a/lib/test/Registry/ManifestIndex.purs b/lib/test/Registry/ManifestIndex.purs index 1fb7e13a6..4cd23b5ba 100644 --- a/lib/test/Registry/ManifestIndex.purs +++ b/lib/test/Registry/ManifestIndex.purs @@ -139,6 +139,40 @@ spec = do testIndex ManifestIndex.ConsiderRanges { satisfied, unsatisfied: [] } + Spec.it "Rejects deletion when remaining versions don't satisfy dependents' ranges" do + let + prelude = unsafeManifest "prelude" "1.0.0" [] + effect_1 = unsafeManifest "effect" "1.0.0" [ Tuple "prelude" ">=1.0.0 <2.0.0" ] + effect_2 = unsafeManifest "effect" "2.0.0" [ Tuple "prelude" ">=1.0.0 <2.0.0" ] + -- console depends on effect >=2.0.0, so removing effect@2.0.0 should fail + console = unsafeManifest "console" "1.0.0" [ Tuple "effect" ">=2.0.0 <3.0.0" ] + index = ManifestIndex.insert ManifestIndex.ConsiderRanges prelude ManifestIndex.empty + >>= ManifestIndex.insert ManifestIndex.ConsiderRanges effect_1 + >>= ManifestIndex.insert ManifestIndex.ConsiderRanges effect_2 + >>= ManifestIndex.insert ManifestIndex.ConsiderRanges console + case index of + Left errors -> Assert.fail $ formatInsertErrors errors + Right idx -> case ManifestIndex.delete ManifestIndex.ConsiderRanges (Utils.unsafePackageName "effect") (Utils.unsafeVersion "2.0.0") idx of + Left _ -> pure unit + Right _ -> Assert.fail "Expected deletion to fail when remaining versions don't satisfy dependent's range" + + Spec.it "Allows deletion when remaining versions satisfy dependents' ranges" do + let + prelude = unsafeManifest "prelude" "1.0.0" [] + effect_1 = unsafeManifest "effect" "1.0.0" [ Tuple "prelude" ">=1.0.0 <2.0.0" ] + effect_2 = unsafeManifest "effect" "2.0.0" [ Tuple "prelude" ">=1.0.0 <2.0.0" ] + -- console depends on effect >=1.0.0, so removing effect@2.0.0 is fine (1.0.0 satisfies) + console = unsafeManifest "console" "1.0.0" [ Tuple "effect" ">=1.0.0 <3.0.0" ] + index = ManifestIndex.insert ManifestIndex.ConsiderRanges prelude ManifestIndex.empty + >>= ManifestIndex.insert ManifestIndex.ConsiderRanges effect_1 + >>= ManifestIndex.insert ManifestIndex.ConsiderRanges effect_2 + >>= ManifestIndex.insert ManifestIndex.ConsiderRanges console + case index of + Left errors -> Assert.fail $ formatInsertErrors errors + Right idx -> case ManifestIndex.delete ManifestIndex.ConsiderRanges (Utils.unsafePackageName "effect") (Utils.unsafeVersion "2.0.0") idx of + Left errors -> Assert.fail $ "Expected deletion to succeed but got errors:\n" <> formatDeleteErrors errors + Right _ -> pure unit + Spec.it "Does not parse unacceptable cyclical index" do let unsatisfied :: Array Manifest @@ -213,6 +247,12 @@ testSorted input = do , JSON.printIndented $ CJ.encode (CJ.array manifestCodec') sorted ] +formatDeleteErrors :: Map PackageName (Map Version.Version (Map PackageName Range)) -> String +formatDeleteErrors errors = String.joinWith "\n" + [ "Failed to delete. Unsatisfied:" + , JSON.printIndented $ CJ.encode (Internal.Codec.packageMap (Internal.Codec.versionMap (Internal.Codec.packageMap Range.codec))) errors + ] + formatInsertErrors :: Map PackageName Range -> String formatInsertErrors errors = String.joinWith "\n" [ "Failed to insert. Failed to satisfy:" diff --git a/nix/test/config.nix b/nix/test/config.nix index 7a98227e6..92fa3535e 100644 --- a/nix/test/config.nix +++ b/nix/test/config.nix @@ -169,6 +169,30 @@ let }; }; + # effect@4.0.1 (same source as 4.0.0, used by publish E2E tests) + effectV401Base64Response = + fileName: + base64Response { + url = "/repos/purescript/purescript-effect/contents/${fileName}?ref=v4.0.1"; + inherit fileName; + filePath = rootPath + "/app/fixtures/github-packages/effect-4.0.0/${fileName}"; + }; + + effectV401404Response = fileName: { + request = { + method = "GET"; + url = "/repos/purescript/purescript-effect/contents/${fileName}?ref=v4.0.1"; + }; + response = { + status = 404; + headers."Content-Type" = "application/json"; + jsonBody = { + message = "Not Found"; + documentation_url = "https://docs.github.com/rest/repos/contents#get-repository-content"; + }; + }; + }; + # Console package helpers (console@6.1.0) consoleBase64Response = fileName: @@ -275,6 +299,13 @@ let (effect404Response "spago.dhall") (effect404Response "purs.json") (effect404Response "package.json") + # effect@4.0.1 (same source, different ref — used by publish E2E tests) + (effectV401Base64Response "bower.json") + (effectV401Base64Response "LICENSE") + (effectV401404Response "spago.yaml") + (effectV401404Response "spago.dhall") + (effectV401404Response "purs.json") + (effectV401404Response "package.json") # Console package (console@6.1.0) (consoleBase64Response "bower.json") (consoleBase64Response "LICENSE") @@ -353,6 +384,45 @@ let jsonBody = [ ]; }; } + # Tags for effect package (v4.0.0 and v4.0.1 are published) + { + request = { + method = "GET"; + url = "/repos/purescript/purescript-effect/tags"; + }; + response = { + status = 200; + headers."Content-Type" = "application/json"; + jsonBody = [ + { + name = "v4.0.0"; + commit = { + sha = "effect-sha-400"; + url = "https://api.github.com/repos/purescript/purescript-effect/commits/effect-sha-400"; + }; + } + { + name = "v4.0.1"; + commit = { + sha = "effect-sha-401"; + url = "https://api.github.com/repos/purescript/purescript-effect/commits/effect-sha-401"; + }; + } + ]; + }; + } + # Commits endpoint for effect - return empty (no recent commits) + { + request = { + method = "GET"; + urlPattern = "/repos/purescript/purescript-effect/commits\\?since=.*"; + }; + response = { + status = 200; + headers."Content-Type" = "application/json"; + jsonBody = [ ]; + }; + } # Commits endpoint for type-equality - return the v4.0.2 commit sha # This makes the DailyImporter detect that v4.0.2 is a recent commit { @@ -1033,6 +1103,9 @@ let gitbot -C "$FIXTURES_DIR/purescript/package-sets" tag -m "psc-0.15.9-20230105" psc-0.15.9-20230105 gitbot -C "$FIXTURES_DIR/purescript/purescript-effect" tag -m "v4.0.0" v4.0.0 + # Create a new commit for v4.0.1 so it's on a different commit than v4.0.0 + gitbot -C "$FIXTURES_DIR/purescript/purescript-effect" commit --allow-empty -m "v4.0.1 release" + gitbot -C "$FIXTURES_DIR/purescript/purescript-effect" tag -m "v4.0.1" v4.0.1 gitbot -C "$FIXTURES_DIR/purescript/purescript-console" tag -m "v6.1.0" v6.1.0 gitbot -C "$FIXTURES_DIR/purescript/purescript-unsafe-coerce" tag -m "v6.0.0" v6.0.0 gitbot -C "$FIXTURES_DIR/purescript/purescript-type-equality" tag -m "v4.0.1" v4.0.1 From 39d5798cae64fa230b191de8449012a15d930754 Mon Sep 17 00:00:00 2001 From: Fabrizio Ferrai Date: Mon, 23 Mar 2026 06:57:08 +0200 Subject: [PATCH 2/2] Apply review feedback --- app/test/App/API.purs | 2 +- app/test/App/Server/MatrixBuilder.purs | 6 +++--- app/test/Test/Assert/Run.purs | 4 +--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/test/App/API.purs b/app/test/App/API.purs index 86ff17172..d63746361 100644 --- a/app/test/App/API.purs +++ b/app/test/App/API.purs @@ -176,7 +176,7 @@ spec = do copyFixture "github-packages" -- We remove effect fixtures since the unit test publishes effect from -- scratch and will fail if it's already registered. We have these in - -- fixtures for the separate integration tests (cascade testing). + -- fixtures for the separate integration tests. FS.Extra.remove $ Path.concat [ testFixtures, "registry-storage", "effect-4.0.0.tar.gz" ] FS.Extra.remove $ Path.concat [ testFixtures, "registry", "metadata", "effect.json" ] FS.Extra.remove $ Path.concat [ testFixtures, "registry-index", "ef" ] diff --git a/app/test/App/Server/MatrixBuilder.purs b/app/test/App/Server/MatrixBuilder.purs index ecfabb9c0..ec481f83b 100644 --- a/app/test/App/Server/MatrixBuilder.purs +++ b/app/test/App/Server/MatrixBuilder.purs @@ -62,9 +62,9 @@ spec = do -- | given compilers in its metadata. setup effectCompilers = do let - index = Utils.fromRight "Failed to build ManifestIndex" - $ ManifestIndex.insert ManifestIndex.ConsiderRanges preludeManifest ManifestIndex.empty - >>= ManifestIndex.insert ManifestIndex.ConsiderRanges effectManifest + index = Utils.fromRight "Failed to build ManifestIndex" do + ManifestIndex.insert ManifestIndex.ConsiderRanges preludeManifest ManifestIndex.empty + >>= ManifestIndex.insert ManifestIndex.ConsiderRanges effectManifest -- prelude must include 0.15.11 in its compilers because this test -- simulates the state AFTER prelude's own matrix job has completed. diff --git a/app/test/Test/Assert/Run.purs b/app/test/Test/Assert/Run.purs index 43259230e..f6572d3e6 100644 --- a/app/test/Test/Assert/Run.purs +++ b/app/test/Test/Assert/Run.purs @@ -144,9 +144,7 @@ runBaseEffects = do runRegistryMock :: forall a. Ref (Map PackageName Metadata) -> Ref ManifestIndex -> Run (EXCEPT String + LOG + REGISTRY + AFF + EFFECT + ()) a -> Aff a runRegistryMock metadataRef indexRef = Registry.interpret (handleRegistryMock { metadataRef, indexRef }) - >>> Log.interpret (\(Log _ _ next) -> pure next) - >>> Except.catch (\err -> Run.liftAff (Aff.throwError (Aff.error err))) - >>> Run.runBaseAff' + >>> runBaseEffects runGitHubCacheMemory :: forall r a. CacheRef -> Run (GITHUB_CACHE + LOG + EFFECT + r) a -> Run (LOG + EFFECT + r) a runGitHubCacheMemory = Cache.interpret GitHub._githubCache <<< Cache.handleMemory