refactor(tests): share test containers across parallel JVM forks#979
refactor(tests): share test containers across parallel JVM forks#979jnatten wants to merge 4 commits into
Conversation
gunnarvelle
left a comment
gunnarvelle
left a comment
There was a problem hiding this comment.
Det ser ut som om du bruker standardporter 5432 og 9200. Vil det sei at det ikkje brukes randomporter som testene gjør i dag? Altså at det ikkje kan kjøre postgres og es lokal på maskina?
Uhm nei? Hvor da tenker du på? |
Introduce SharedContainer, a file-based coordination mechanism that allows parallel jvm test forks to reuse a single Postgres, Elasticsearch, and Redis container via cross-process refcounting and file locks. - Add SharedContainer with acquire/release lifecycle, health checks, stale container cleanup, and shutdown hooks - Refactor DatabaseIntegrationSuite, ElasticsearchIntegrationSuite, and RedisIntegrationSuite to use SharedContainer instead of spawning individual containers per test suite - Append PID to search index names and database schema names to isolate parallel forks sharing the same container - Make search index properties lazy so they pick up test environment overrides
jnatten
force-pushed
the
refactor/test-container-sharing
branch
2 times, most recently
from
e6530f6 to
c839c5e
Compare
This flag wasn't really in use because it was never false, and even if it was the rest of the suite would throw so this changes that to keep things simple. The original idea was to maybe support running tests without docker environment and keeping the suite green while cancelling the other tests, but since that isn't really a good idea now we don't need this.
jnatten
force-pushed
the
refactor/test-container-sharing
branch
from
c839c5e to
4d7ab83
Compare
jnatten
force-pushed
the
refactor/test-container-sharing
branch
from
3cf954f to
81eaa19
Compare
jnatten
force-pushed
the
refactor/test-container-sharing
branch
from
81eaa19 to
dea2de3
Compare
| val EnablePostgresContainer: Boolean = true | ||
| val PostgresqlVersion: String = "17.5" | ||
| lazy val schemaName: String = "testschema" | ||
| lazy val schemaName: String = s"testschema_${ProcessHandle.current().pid()}" |
There was a problem hiding this comment.
Kunne gjort noe sånt som det her for å slippe å legge inn PID i alle subklasser. Ser f.eks. at ConfigRepositoryTest mangler PID greiene
| lazy val schemaName: String = s"testschema_${ProcessHandle.current().pid()}" | |
| lazy val schemaNamePrefix: String = "testschema" | |
| private lazy val schemaName: String = s"${schemaNamePrefix}_${ProcessHandle.current().pid()}" |
| } | ||
| } | ||
| private def esImageName: DockerImageName = { | ||
| val imgName = env.getOrElse("SEARCH_ENGINE_IMAGE", ElasticsearchImage) |
There was a problem hiding this comment.
Har vi lyst til å fortsette å støtte dette egentlig?
There was a problem hiding this comment.
Tja, koster veldig lite å tåle dette spørru meg men er jo ikke så fryktelig farlig heller.
🤷
| private def stopContainer(containerId: String): Unit = { | ||
| Try { | ||
| val process = new ProcessBuilder("docker", "rm", "-f", containerId).redirectErrorStream(true).start() | ||
| process.waitFor() |
There was a problem hiding this comment.
tja, kanskje?
tanker om hvor lenge haha? et minutt bare sånn for sikkerhetsskyld type?
| Files.writeString(path, lines.mkString("\n"), StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING): Unit | ||
| } | ||
|
|
||
| private def readInfoFile(path: Path): Option[SharedContainerInfo] = { |
There was a problem hiding this comment.
Hadde det ikke vært lettere å bare bruke ObjectInputStream og ObjectOutputStream for å lese/skrive info greiene?
There was a problem hiding this comment.
Etter å ha kløna litt så vurderer jeg sterkt den refaktoreringen jeg styra med på jobb i dag.
La også på litt bedre typing for den infoen og da endrer jeg på disse også.
Men det kan hende jeg er litt blind etter å ha jobbet med det. Hva tenker du?
There was a problem hiding this comment.
Og jeg gjør fortsatt streng skriving da, men føler ikke de var særlig komplekse lenger. I alle fall ikke sammenlignet med denne branchen hehe.
3 participants
Stacker på denne #977
Gir det et nytt forsøk på å kjøre delte containere under testing.
Dette gjør at vi bruker drastisk mye mindre minne under kjøring av tester vha at vi kjører kun en elasticsearch container og en kun en postgres container etc.
Krever bittelitt mer tunga rett i munnen når man skriver tester pga man kan i teorien lage kolliderende tester.
Denne fikser så vi appender pid'en (siden jvmen blir forka) til schema og index navn. Men jeg tror det lett er worth.