Skip to content

Commit ddf0c1d

Browse files
authored
Fix matrix job cascading for new compiler releases (#763)
1 parent 5441045 commit ddf0c1d

15 files changed

Lines changed: 314 additions & 23 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ result*
1919

2020
TODO.md
2121
.spec-results
22+
/generated-docs
2223

2324
# Keep it secret, keep it safe.
2425
.env

app-e2e/src/Test/E2E/Endpoint/Startup.purs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import Registry.Test.Assert as Assert
1414
import Registry.Test.Utils as Utils
1515
import Test.E2E.Support.Client as Client
1616
import Test.E2E.Support.Env (E2ESpec)
17+
import Test.E2E.Support.Env as Env
1718
import Test.Spec as Spec
1819

1920
spec :: E2ESpec
@@ -47,3 +48,26 @@ spec = do
4748
<> show (Array.length matrixJobs)
4849
<> " matrix jobs for packages: "
4950
<> String.joinWith ", " (map PackageName.print matrixPackages)
51+
52+
Spec.it "cascades matrix jobs to dependant packages after dependency-free jobs complete" do
53+
-- Wait for ALL pending jobs: the dependency-free matrix jobs and any
54+
-- cascade jobs they trigger.
55+
Env.waitForAllPendingJobs
56+
57+
-- Check that effect got a matrix job with the new compiler (0.15.11).
58+
-- effect depends on prelude, so after prelude's 0.15.11 matrix job
59+
-- succeeds, the cascade should enqueue a job for effect.
60+
allJobs <- Client.getJobsWith Client.IncludeCompleted
61+
let
62+
effectName = Utils.unsafePackageName "effect"
63+
newCompiler = Utils.unsafeVersion "0.15.11"
64+
effectCascadeJobs = Array.filter
65+
( case _ of
66+
MatrixJob { packageName, compilerVersion } ->
67+
packageName == effectName && compilerVersion == newCompiler
68+
_ -> false
69+
)
70+
allJobs
71+
72+
when (Array.null effectCascadeJobs) do
73+
Assert.fail "Expected cascade matrix job for effect with compiler 0.15.11, but found none."

app-e2e/src/Test/E2E/Support/Fixtures.purs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ import Registry.Version (Version)
4343

4444
type PackageFixture = { name :: PackageName, version :: Version }
4545

46-
-- | effect@4.0.0 fixture package
46+
-- | effect@4.0.1 fixture package (4.0.0 is pre-populated in startup fixtures for cascade testing)
4747
effect :: PackageFixture
48-
effect = { name: Utils.unsafePackageName "effect", version: Utils.unsafeVersion "4.0.0" }
48+
effect = { name: Utils.unsafePackageName "effect", version: Utils.unsafeVersion "4.0.1" }
4949

5050
-- | console@6.1.0 fixture package
5151
console :: PackageFixture
@@ -59,8 +59,8 @@ prelude = { name: Utils.unsafePackageName "prelude", version: Utils.unsafeVersio
5959
slug :: PackageFixture
6060
slug = { name: Utils.unsafePackageName "slug", version: Utils.unsafeVersion "3.0.0" }
6161

62-
-- | Standard publish data for effect@4.0.0, used by E2E tests.
63-
-- | This matches the fixtures in app/fixtures/github-packages/effect-4.0.0
62+
-- | Standard publish data for effect@4.0.1, used by E2E tests.
63+
-- | This matches the fixtures in app/fixtures/github-packages/effect-4.0.1
6464
effectPublishData :: Operation.PublishData
6565
effectPublishData =
6666
{ name: effect.name
@@ -69,7 +69,7 @@ effectPublishData =
6969
, repo: "purescript-effect"
7070
, subdir: Nothing
7171
}
72-
, ref: "v4.0.0"
72+
, ref: "v4.0.1"
7373
, compiler: Just $ Utils.unsafeVersion "0.15.10"
7474
, resolutions: Nothing
7575
, version: effect.version

app/fixtures/github-packages/console-6.1.0/bower.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"package.json"
1717
],
1818
"dependencies": {
19-
"purescript-effect": "^4.0.0",
19+
"purescript-effect": "^4.0.1",
2020
"purescript-prelude": "^6.0.0"
2121
}
2222
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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"}}
5.77 KB
Binary file not shown.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"location": {
3+
"githubOwner": "purescript",
4+
"githubRepo": "purescript-effect"
5+
},
6+
"published": {
7+
"4.0.0": {
8+
"bytes": 5904,
9+
"compilers": [
10+
"0.15.10"
11+
],
12+
"hash": "sha256-6a6UH+Q1C86LCmHWiIq/Xh/2+vHRS69ZeEsQAXrfFQs=",
13+
"publishedTime": "2022-08-18T20:04:00.000Z",
14+
"ref": "v4.0.0"
15+
}
16+
},
17+
"unpublished": {}
18+
}

app/src/App/Server/MatrixBuilder.purs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,15 +211,22 @@ solveDependantsForCompiler { compilerIndex, name, version, compiler } = do
211211
manifestIndex <- Registry.readAllManifests
212212
let dependentManifests = ManifestIndex.dependants manifestIndex name version
213213
newJobs <- for dependentManifests \(Manifest manifest) -> do
214-
-- we first verify if we have already attempted this package with this compiler,
215-
-- either in the form of having it in the metadata already, or as a failed compilation
216-
-- (i.e. if we find compilers in the metadata for this version we only check this one
217-
-- if it's newer, because all the previous ones have been tried)
214+
-- We skip if this compiler is already in the package's metadata compilers
215+
-- list (meaning it was already successfully tested). Failed compilations
216+
-- are not recorded in metadata, but the DB deduplication in insertMatrixJob
217+
-- prevents re-enqueuing jobs that already exist.
218218
shouldAttemptToCompile <- Registry.readMetadata manifest.name >>= case _ of
219-
Nothing -> pure false
220-
Just metadata -> pure $ case Map.lookup version (un Metadata metadata).published of
221-
Nothing -> false
222-
Just { compilers } -> any (_ > compiler) compilers
219+
Nothing -> do
220+
Log.debug $ "Skipping " <> PackageName.print manifest.name <> "@" <> Version.print manifest.version <> ": no metadata found"
221+
pure false
222+
Just metadata -> do
223+
let
224+
result = case Map.lookup manifest.version (un Metadata metadata).published of
225+
Nothing -> false
226+
Just { compilers } -> all (_ /= compiler) compilers
227+
unless result do
228+
Log.debug $ "Skipping " <> PackageName.print manifest.name <> "@" <> Version.print manifest.version <> ": compiler " <> Version.print compiler <> " already tested or version not published"
229+
pure result
223230
case shouldAttemptToCompile of
224231
false -> pure Nothing
225232
true -> do

app/test/App/API.purs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,12 @@ spec = do
174174
copyFixture "registry"
175175
copyFixture "registry-storage"
176176
copyFixture "github-packages"
177-
-- We remove effect-4.0.0.tar.gz since the unit test publishes it from
178-
-- scratch and will fail if it's already in storage. We have it in
179-
-- storage for the separate integration tests.
177+
-- We remove effect fixtures since the unit test publishes effect from
178+
-- scratch and will fail if it's already registered. We have these in
179+
-- fixtures for the separate integration tests.
180180
FS.Extra.remove $ Path.concat [ testFixtures, "registry-storage", "effect-4.0.0.tar.gz" ]
181+
FS.Extra.remove $ Path.concat [ testFixtures, "registry", "metadata", "effect.json" ]
182+
FS.Extra.remove $ Path.concat [ testFixtures, "registry-index", "ef" ]
181183

182184
let
183185
readFixtures = do
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
module Test.Registry.App.Server.MatrixBuilder (spec) where
2+
3+
import Registry.App.Prelude
4+
5+
import Data.Map as Map
6+
import Data.Set as Set
7+
import Effect.Ref as Ref
8+
import Registry.App.Server.MatrixBuilder as MatrixBuilder
9+
import Registry.ManifestIndex as ManifestIndex
10+
import Registry.PackageName as PackageName
11+
import Registry.Solver as Solver
12+
import Registry.Test.Assert.Run (runRegistryMock)
13+
import Registry.Test.Utils as Utils
14+
import Test.Spec as Spec
15+
import Test.Spec.Assertions as Assert
16+
17+
spec :: Spec.Spec Unit
18+
spec = do
19+
Spec.describe "solveDependantsForCompiler" do
20+
Spec.it "cascades to dependant for a new compiler" do
21+
let { solverData, index, metadata } = setup [ "0.15.10" ]
22+
result <- runSolver solverData index metadata
23+
let names = Set.map _.name result
24+
unless (Set.member effectName names) do
25+
Assert.fail $ "Expected cascade to effect, but got: "
26+
<> show (Set.map PackageName.print names)
27+
28+
Spec.it "skips dependant already tested with this compiler" do
29+
let { solverData, index, metadata } = setup [ "0.15.10", "0.15.11" ]
30+
result <- runSolver solverData index metadata
31+
unless (Set.isEmpty result) do
32+
Assert.fail "Expected empty result set when compiler already tested"
33+
34+
where
35+
preludeName = Utils.unsafePackageName "prelude"
36+
effectName = Utils.unsafePackageName "effect"
37+
38+
compiler_0_15_10 = Utils.unsafeVersion "0.15.10"
39+
compiler_0_15_11 = Utils.unsafeVersion "0.15.11"
40+
allCompilers = Utils.unsafeNonEmptyArray [ compiler_0_15_10, compiler_0_15_11 ]
41+
42+
preludeVersion = Utils.unsafeVersion "6.0.0"
43+
44+
preludeManifest = Utils.unsafeManifest "prelude" "6.0.0" []
45+
effectManifest = Utils.unsafeManifest "effect" "4.0.0"
46+
[ Tuple "prelude" ">=6.0.0 <7.0.0" ]
47+
48+
dummySha = Utils.unsafeSha256 "sha256-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa="
49+
dummyTime = Utils.unsafeDateTime "2022-08-18T20:04:00.000Z"
50+
51+
mkPublishedMeta version compilers =
52+
Map.singleton version
53+
{ bytes: 1000.0
54+
, compilers: Utils.unsafeNonEmptyArray (map Utils.unsafeVersion compilers)
55+
, hash: dummySha
56+
, publishedTime: dummyTime
57+
, ref: Nothing
58+
}
59+
60+
-- | Set up a test scenario where prelude@6.0.0 (no deps) has completed its
61+
-- | matrix job for 0.15.11, and effect@4.0.0 (depends on prelude) has the
62+
-- | given compilers in its metadata.
63+
setup effectCompilers = do
64+
let
65+
index = Utils.fromRight "Failed to build ManifestIndex" do
66+
ManifestIndex.insert ManifestIndex.ConsiderRanges preludeManifest ManifestIndex.empty
67+
>>= ManifestIndex.insert ManifestIndex.ConsiderRanges effectManifest
68+
69+
-- prelude must include 0.15.11 in its compilers because this test
70+
-- simulates the state AFTER prelude's own matrix job has completed.
71+
-- In the real flow: runMatrixJob writes the new compiler into the
72+
-- parent's metadata (MatrixBuilder.purs:82-91), then the executor
73+
-- rebuilds the CompilerIndex from current metadata before calling
74+
-- solveDependantsForCompiler. Without 0.15.11 here, the solver
75+
-- would compute purs >=0.15.10 <0.15.11 for prelude, excluding
76+
-- the target compiler from effect's build plan.
77+
preludeMetadata = Metadata
78+
{ location: Git { url: "https://github.com/purescript/purescript-prelude.git", subdir: Nothing }
79+
, owners: Nothing
80+
, published: mkPublishedMeta preludeVersion [ "0.15.10", "0.15.11" ]
81+
, unpublished: Map.empty
82+
}
83+
84+
effectMetadata = Metadata
85+
{ location: Git { url: "https://github.com/purescript/purescript-effect.git", subdir: Nothing }
86+
, owners: Nothing
87+
, published: mkPublishedMeta (Utils.unsafeVersion "4.0.0") effectCompilers
88+
, unpublished: Map.empty
89+
}
90+
91+
metadata = Map.fromFoldable [ Tuple preludeName preludeMetadata, Tuple effectName effectMetadata ]
92+
93+
compilerIndex = Solver.buildCompilerIndex allCompilers index metadata
94+
95+
solverData =
96+
{ compilerIndex
97+
, compiler: compiler_0_15_11
98+
, name: preludeName
99+
, version: preludeVersion
100+
, dependencies: Map.empty
101+
}
102+
103+
{ solverData, index, metadata }
104+
105+
runSolver solverData index metadata = liftAff do
106+
indexRef <- liftEffect $ Ref.new index
107+
metadataRef <- liftEffect $ Ref.new metadata
108+
runRegistryMock metadataRef indexRef
109+
$ MatrixBuilder.solveDependantsForCompiler solverData

0 commit comments

Comments
 (0)