CLARIN-DSpace v9/Port #1413 (de-flake scheduled ITs) to the v9 base - #1435
Conversation
…ted handle collisions, leaked bitstream formats, stale Solr searcher) (#1413) Source: 23f3786 (dtq-dev PR #1413) Test-only. All six flake sources the source commit removed exist unchanged on this branch, so the port is a straight carry-over: - ItemHandleCheckerIT / RequiredMetadataIT planted a random numeric handle (`123456789/` + 1000..1999) that `handle_seq` reaches late in the module run; the sequence mint path does no existence check, so whichever test was minting at that moment died on the handle unique index. Replaced by deterministic non-numeric suffixes (`handle-checker-test`, `required-metadata-test`), and the now-unused `randomString()` helper and `java.util.Random` import go away. - XmlWorkflowFactoryTest used the numeric handle `123456789/999` for the non-mapped collection; same collision class, now `123456789/workflow-test-2`. - PreviewContentServiceImplIT deleted its custom bitstream format through the shared test context after earlier builder deletes had detached it from the thread-bound session, so the delete was silently lost -- one leaked `bitstreamformatregistry` row per test, which broke BitstreamFormatRestRepositoryIT's counts whenever class order put Preview first. The id is now captured before `super.destroy()` and the row deleted afterwards through `BitstreamFormatBuilder.deleteBitstreamFormat`, which uses its own Context. - BitstreamFormatRestRepositoryIT no longer asserts against the hardcoded `DEFAULT_AMOUNT_FORMATS = 95`; each affected test reads the live baseline via `currentTotalFormats()`. Short descriptions are unique in the format registry, so `createMockBitstreamRest()` derives them from the JUnit `@Rule TestName` instead of a random number, and `createAdminAccess` captures the created id in the `andDo` attached to the status check so cleanup still runs if a later assertion fails. - StatisticsRestRepositoryIT#totalVisitsReport_Item_Visited queried the usage report before the posted view event became visible to a new Solr searcher; it now commits, exactly as topCountriesReport_Community_Visited already did. One adaptation vs the source commit, in BitstreamFormatRestRepositoryIT only: this branch already carries an unrelated refactor that replaced the six per-test `ObjectMapper mapper = new ObjectMapper();` locals with a single `@Autowired private ObjectMapper mapper` field. That field is kept and the source hunks were applied on top of it, so no `new ObjectMapper()` returns. Every added and removed line of the source patch for this file is otherwise byte-identical (44 added / 25 removed, verified by diffing the two patches). Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 23f3786) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
253021d to
ccef3de
Compare
Local gate outputs (per sync rule 2.5)Base: B1 AC gate — the two ITs in reverse-alphabetical class order (Preview first, i.e. the order that
(Three further green runs were done at the pre-rebase base, so six in total. Negative control — the port is not vacuous. The identical command on the un-ported base: Exactly the four tests that assert against Hunk-level parity. Every added and removed line of |
1 participant
What
Ports
dataquest-dev/DSpacePR #1413 ("de-flake scheduled ITs", commit23f37863a4) todtq-dev-9-base. Test-only — no production code, no config, no migration.Sync card: BE-03 (
_sync3/cards/BE-03.md, tranche T4, disposition TESTS-ONLY, size S, risk low).Why
All six flake sources the source PR removed are live on this branch. Verified at blob level
before porting — four of the six files are byte-identical with the source commit's pre-image:
23f37863a4^blobdspace-api/.../curate/ItemHandleCheckerIT.javad006c9ebccd006c9ebccdspace-api/.../curate/RequiredMetadataIT.javaca9be23295ca9be23295dspace-api/.../xmlworkflow/XmlWorkflowFactoryTest.java03a6a0e94903a6a0e949dspace-server-webapp/.../PreviewContentServiceImplIT.java47f7564b8e47f7564b8edspace-server-webapp/.../BitstreamFormatRestRepositoryIT.javaf9b2bb3023af963696f0(unrelated@Autowired ObjectMapperrefactor)dspace-server-webapp/.../StatisticsRestRepositoryIT.java3baeeb9b7a716ccf8b8d(v9 drift)The failure modes, unchanged from the source PR:
RequiredMetadataIT/ItemHandleCheckerITplanted a random numeric handle(
123456789/+ 1000..1999) thathandle_seqreaches late in the module run. The sequencemint path does no existence check, so the flush of whichever test was minting at that moment
died on the handle unique index.
XmlWorkflowFactoryTesthad the same problem with the fixednumeric handle
123456789/999. All three now use non-numeric suffixes the sequence can nevermint.
PreviewContentServiceImplITdeleted its custom bitstream format through the shared testcontext after earlier builder deletes had already detached it from the thread-bound session,
so the delete was silently lost — one leaked
bitstreamformatregistryrow per test.BitstreamFormatRestRepositoryITasserted against a hardcodedDEFAULT_AMOUNT_FORMATS = 95,so any leaked row (see above) turned it red whenever class order put Preview first.
StatisticsRestRepositoryIT#totalVisitsReport_Item_Visitedqueried the usage report beforeits posted view event was visible to a new Solr searcher — the one place in the class that
did not commit (
grep -c 'getSolrLoggerService().commit()'→ 1 here, 2 ondtq-dev).Changes
git cherry-pick -x 23f37863a4. Four files applied clean;StatisticsRestRepositoryITauto-merged (the single hunk is far from the v9 drift);
BitstreamFormatRestRepositoryIThad six conflicts, all the same one.
The single adaptation: this branch already carries an unrelated refactor that replaced the
six per-test
ObjectMapper mapper = new ObjectMapper();locals with one@Autowired private ObjectMapper mapperfield. That refactor is kept; the source hunks wereapplied on top of it. Nothing else differs — every added and removed line of the source patch
is byte-identical here:
Per-file comparison of the two patches'
+/-payload lines:Testing
Static checks (card AC-BE-03-1 / -3):
Build:
mvn clean install -P-assembly -DskipTests-> BUILD SUCCESS, 0 checkstyle violations.Local IT gate —
PreviewContentServiceImplIT+BitstreamFormatRestRepositoryITwith-Dfailsafe.runOrder=reversealphabetical(Preview first, i.e. the order that used to leak),three separate runs, all green, each number read from the per-class report file with its
mtime:
Tests run: 35, Failures: 0, Errors: 0, Skipped: 2Tests run: 35, Failures: 0, Errors: 0, Skipped: 2Tests run: 35, Failures: 0, Errors: 0, Skipped: 2(
Skipped: 2= the two pre-existing@Ignored tests, untouched.)Negative control — the port is not vacuous. The same command on the un-ported base
origin/dtq-dev-9-base(1cda0dcd6c):Exactly the four tests that assert against
DEFAULT_AMOUNT_FORMATSfail, and the drift is108 - 95 = 13— one leakedbitstreamformatregistryrow perPreviewContentServiceImplITtest, which is precisely the mechanism the source PR describes. Note that Preview itself is
green on the base: the leak is invisible from inside the class that causes it, which is why
the gate has to fix class order.
🤖 Generated with Claude Code