Ignifyr Test Extension

[KeremHmd]

• New E2E tests are added to test new behaviors built around enterprise/community separation and Kafka/REDcap streaming.
• A test-flow suite is built to provide options and configurations on how to run different kinds of tests/demo builds of Ignifyr and related tools. All options are documented to only allow the desired tests to be run during a development or a release build.

[Okanmercan99]

"On JDK 17+ the build adds Spark's required --add-opens automatically" — I don't think this is
in the pushed tree. Grepping *.xml, *.sh and *.yml for add-opens, argLine, jvmArgs and
extraJvmArgs returns nothing, so no module (and no script) passes JVM options to the forked test
JVM.

This looks like it lines up with your note that the Java/Spark pom setting you were using locally
didn't get pushed. Flagging it because the README currently reads as if the build handles it, which
sends anyone on JDK 17+ looking for an environment problem instead of a missing build setting.

Why the build has to do it: Spark 3.3+ carries org.apache.spark.launcher.JavaModuleOptions, and
spark-submit applies those --add-opens flags automatically on JDK 17+. These suites don't go
through spark-submit — they use Spark as an embedded library inside the JVM that
scalatest-maven-plugin forks — so nothing adds them and the responsibility lands on the pom. So the
behaviour the README describes is the right behaviour; it just isn't implemented yet.

Cheapest place to fix it once for every module is the root pom's pluginManagement entry for
scalatest-maven-plugin, which already exists (pom.xml:187):

<configuration>
  <argLine>
    -XX:+IgnoreUnrecognizedVMOptions
    --add-opens=java.base/java.lang=ALL-UNNAMED
    --add-opens=java.base/java.lang.invoke=ALL-UNNAMED
    --add-opens=java.base/java.lang.reflect=ALL-UNNAMED
    --add-opens=java.base/java.io=ALL-UNNAMED
    --add-opens=java.base/java.net=ALL-UNNAMED
    --add-opens=java.base/java.nio=ALL-UNNAMED
    --add-opens=java.base/java.util=ALL-UNNAMED
    --add-opens=java.base/java.util.concurrent=ALL-UNNAMED
    --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED
    --add-opens=java.base/sun.nio.ch=ALL-UNNAMED
    --add-opens=java.base/sun.nio.cs=ALL-UNNAMED
    --add-opens=java.base/sun.security.action=ALL-UNNAMED
    --add-opens=java.base/sun.util.calendar=ALL-UNNAMED
    -Djdk.reflect.useDirectMethodHandle=false
  </argLine>
  ...
</configuration>

-XX:+IgnoreUnrecognizedVMOptions keeps the same argLine valid on the JDK 11 target, so it can be
unconditional rather than profile-gated. Worth double-checking that the modules which override
<configuration> per execution (ignifyr-engine, ignifyr-runtime-streaming, ignifyr-runtime-scheduling,
connector-file, connector-sql) still inherit it — per-execution <configuration> merges with the
managed one, but it's the kind of thing worth confirming with one mvn -X run.

Note that:

• README:15 lists "Java 11 + Maven" as the prerequisite, and there is still no requireJavaVersion
  enforcer rule, so building on JDK 8 fails with a cryptic scalac error ('11' is not a valid choice for '-release') rather than a clear message. Given maven-enforcer is already configured for
  ban-enterprise-deps, a requireJavaVersion rule next to it would pair well with this fix.

[Okanmercan99]

Really glad this suite exists — it runs under plain mvn test (I verified: 4 tests, no Docker, no
SparkSession needed), so unlike the shell scripts it actually gates in CI. One structural gap though:
every assertion here is a denylist ("these specific enterprise things are absent"), and nothing
asserts "only these are present". An enterprise module nobody thought to enumerate passes.

I verified this rather than guessing. I temporarily added ignifyr-format-json (enterprise) to
ignifyr-cli/pom.xml and ran both guards:

mvn -pl ignifyr-cli validate  ->  Rule 0: BannedDependencies passed  ->  BUILD SUCCESS
mvn -pl ignifyr-cli test      ->  Tests: succeeded 4, failed 0       ->  BUILD SUCCESS

while the registry visibly changed:

before: Discovered 3 file source format handler(s): csv, parquet, tsv
after:  Discovered 5 file source format handler(s): csv, json, ndjson, parquet, tsv

So enterprise JSON/NDJSON source support shipped into the community distribution with both guards
green. (Pom restored afterwards.)

Why each guard misses it:

• the enforcer, because format-json's only compile dependency is the community
  ignifyr-connector-file — no banned coordinate to trip on;
• this spec, because format-json registers no IgnifyrExtension at all, only a FileSourceFormat
  sub-SPI entry. Note Loaded 5 Ignifyr extension(s) is unchanged above. The sub-registries are
  never inspected here.

It isn't specific to format-json. Mapping every enterprise module against both guards, two are
invisible to both: format-json and sink-omop (which registers only its id today). format-delta
is caught by the enforcer only, since it contributes just sparkConfContributions. And the sink
assertion at line 38 is contain-only with no absence check, so even a fully implemented enterprise
OMOP sink would pass.

Suggest switching the shape from denylist to allowlist. Expected values below are the ones I actually
observed on the clean branch, and both registries are already on this module's classpath so no new
dependency is needed:

it should "register exactly the community extension set" in {
  ExtensionRegistry.extensions.map(_.id).toSet shouldBe
    Set("core", "connector-sql", "connector-file", "sink-fhir", "sink-file")
}

it should "register exactly the community sinks" in {
  ExtensionRegistry.sinkProviders.keySet shouldBe
    Set(classOf[FhirRepositorySinkSettings], classOf[FileSystemSinkSettings])
}

it should "discover exactly the community file formats (source + sink sub-SPIs)" in {
  FileFormatRegistry.sourceFormats.keySet     shouldBe Set("csv", "tsv", "parquet")
  FileSinkFormatRegistry.sinkFormats.keySet   shouldBe Set("ndjson", "csv", "parquet")
}

Those three together cover the whole class: the id-set catches anything registering an extension
(kafka, fhir-server, scheduling, streaming, redcap, format-delta, sink-omop), and the sub-SPI sets
catch the format modules that plug in below the engine registry. Keeping the existing denylist tests
alongside is still worth it — they produce a much clearer failure message when they're the ones that
trip.

Worth noting this would also close the gap I raised on the base PR, where four enterprise modules
carry no banned library and so are invisible to ban-enterprise-deps. Fixing it here covers them from
the runtime side, in the one check that runs automatically.

[Okanmercan99]

These two are the checks the script itself labels "1", and they're the only ones in the file that
report failure as warn instead of bad. Since warn() (line 26) doesn't touch FAILC and the exit
status is [ "$FAILC" -eq 0 ] (line 101), the script exits 0 even if the community CLI happily
accepts a streaming or Kafka job. The list-plugins check right below uses bad, so the asymmetry
looks deliberate.

The reasoning in the line 26 comment is fair — run --job really does start Spark (Boot's "run" case
constructs IgnifyrEngine, which builds the SparkSession), so on a box without HADOOP_HOME/winutils, or
where Spark can't start at all, the output legitimately won't contain the expected string and a hard
failure would be noise. So I don't think the fix is simply flipping warn to bad — it's
distinguishing "couldn't evaluate" from "evaluated and got the wrong answer". Only the first deserves
a warning.

Something like:

expect_refusal() {  # <job-file> <pattern> <label>
  local out rc
  out="$(timeout 300 java -Dignifyr.mappings.repository.folder-path="$WS/mappings" \
             -Dignifyr.mappings.schemas.repository.folder-path="$WS/schemas" \
             -jar "$CLI_JAR" run --job "$1" 2>&1)"; rc=$?
  if   echo "$out" | grep -q "$2"; then ok "$3"
  elif [ "$rc" -eq 124 ]; then bad "$3 — CLI did not refuse the job; it started running it"
  elif echo "$out" | grep -qiE 'failed to create a child event loop|Unable to establish loopback|winutils|HADOOP_HOME'; then
       warn "$3 — could not evaluate (Spark/environment failed to start)"
  else bad "$3 — CLI ran but never reported the missing capability"
  fi
}

[Okanmercan99]

-DwildcardSuites has no effect here, so --behavior archiving runs all six engine unit suites
instead of the one it names.

The root CLAUDE.md warns about exactly this: -DwildcardSuites and -Dsuites are silently ignored,
because the value in the module POM wins. And ignifyr-engine/pom.xml:66 pins
<wildcardSuites>io.ignifyr.test</wildcardSuites>.

I ran both forms on a clean target:

-DwildcardSuites=...FileStreamInputArchiverTest   ->  6 suites ran
-Dsuffixes='.*FileStreamInputArchiverTest'        ->  1 suite, 9 tests, all passed

So it's a one-line swap:

archiving)  mvn -B -pl ignifyr-engine -am test -Dsuffixes='.*FileStreamInputArchiverTest' ;;

Nothing is being skipped here — the script over-runs rather than under-runs. But --behavior exists
to narrow, and right now it doesn't: you read six suites' mixed output while chasing one behaviour,
and pay the extra time.

The other --behavior branches look fine, since they narrow by module (-pl) rather than by suite.

[Okanmercan99]

Lines 51-54 build a three-way fallback for listing this jar, because Info-ZIP can't handle it:
"Info-ZIP unzip -l truncates/empties its listing there, which would falsely report everything as
missing." But these two lines then extract from the same jar with bare unzip -p and no fallback.

If unzip -p returns nothing on the server jar, SRV_SPI is empty and all six "server SPI lists X"
checks at line 73 fail — a red script with nothing actually wrong.

Minimal fix, and it matches the "couldn't evaluate vs. wrong answer" split I suggested for the
behaviour checks below: guard the read instead of letting it look like six missing extensions.

[ -n "$SRV_SPI" ] || bad "could not read the SPI manifest from the server jar (unzip -p returned nothing)"

Or extract through the same jar-first path you already trust:

read_entry() {  # <jar> <entry>
  local d; d="$(mktemp -d)"
  ( cd "$d" && jar xf "$1" "$2" 2>/dev/null ) && cat "$d/$2" 2>/dev/null
  rm -rf "$d"
}

[Okanmercan99]

These two E2E tests are a real gain — the streaming path had nothing like them before. One thing to
consider for the "what else can be added" question: both call
fhirMappingJobManager.startMappingJobStream(...) directly, which enters one level below
MappingJobLauncher.

MappingJobLauncher currently has no test at all. Grepping for it returns only
CommandLineInterface.scala:95, ExecutionService.scala:51, and its own definition. That's the seam the
base PR introduced as the single home of batch/streaming/scheduled dispatch shared by the CLI and the
server, so it's the one place where "a streaming job dispatches as streaming" is actually decided.
Going through it in one of these tests would also cover the RunningJobRegistry registration, which
no streaming test touches today.

It would also unlock the archiving assertion. The scaladoc explains why archiving is off here — "the
streaming archiver timer (started only by IgnifyrEngine)" isn't running — so archiveMode = OFF
isn't really a choice, it's a consequence of the entry point. Launching through IgnifyrEngine /
MappingJobLauncher makes archiving testable rather than deferred.

Three additions that would close the remaining gaps, roughly in value order:

1. Route one of the two through MappingJobLauncher.launch(...) — covers the dispatch decision,
   registry tracking, and makes archiving assertable.
2. A variant with one malformed row and saveErroneousRecords = true: asserts the good rows still
   land AND that the bad one is written to the erroneous-records path. That's currently the only way
   to pin StreamingSinkHandler's catch-and-continue. StreamingSinkHandlerTest can't: it has no
   assertion at all (starts a rate stream, awaitTermination, stop), so it passes even if nothing was
   written, and it can't distinguish "swallowed and continued" from "no second chunk arrived".
3. A streaming job with two mapping tasks — the per-job and per-task checkpoint isolation is a
   documented invariant and nothing exercises it.

[Okanmercan99]

Both new E2E tests run a single mapping task, so the per-task checkpoint isolation is still unpinned.

getCheckpointDirectory (FhirMappingJobExecution.scala:90) isolates by
<checkpoint>/<jobId>/<mappingTaskName.hashCode>, and StreamingJobExecutor starts one query per
task, all concurrent. If that isolation ever broke, two queries would share one checkpoint location —
Spark's offset/commit log assumes a single writer, so you'd get either a hard failure or silent
offset corruption where one stream skips data.

The fixture already has enough columns to support a second mapping, and asserting both resource types land plus the two checkpoint directories differ is the whole test.

[Okanmercan99]

withReuse(true) here and kafka.stop() in afterAll (line 75) cancel each other out — reuse exists
to keep the container alive between runs, and stop() destroys it every time.

Nothing is broken today: reuse needs testcontainers.reuse.enable=true in
~/.testcontainers.properties, and without it withReuse(true) is ignored with a warning. So the flag
is dead and stop() is the correct behaviour.

It's more of a trap for whoever tries to speed the suite up later. Enabling reuse gets them no
benefit because of the stop(), and if they then remove the stop() the redcap-patients topic starts
accumulating across runs — with startingOffsets = earliest every run would reprocess all previous
messages. The test would still pass (hashed ids make the writes idempotent), so it would just get
slower and slower for no visible reason.

Suggest picking one. Either drop withReuse(true) and keep stop() — simplest, and matches the
"throwaway container" model — or keep reuse, drop stop(), and give the topic a unique name per run so
it doesn't accumulate. Worth noting the testkit's OnFhirTestContainer already picks the second
approach consistently (reuse, never stopped), so matching it would be defensible too.

[Okanmercan99]

run_cli calls bare java, so it uses whatever is first on PATH. On my machine that's a JDK 8 while
the fat jar is Java 11 bytecode, so every invocation died with UnsupportedClassVersionError. Both
behaviour checks reported WARN and the list-plugins check reported PASS — its stderr goes to
/dev/null, and an empty output contains no enterprise names — so the script printed failed: 0 while
three of its checks had run nothing at all.

Preferring "${JAVA_HOME:-}"/bin/java, or asserting the version once up front, would close that. And
neither the WARN nor PASS branches ever print $out, so when a check misbehaves there's nothing to go
on — echoing a few lines on non-PASS would make this much faster to diagnose.