fix(engine): repair this branch's integration suite#713
Draft
romain-pm wants to merge 4 commits into
Draft
Conversation
Exporting a single package turned off bnd's implicit inclusion of dependency
classes: the bundle went from 9154 classes to 88, keeping only what the
explicit Export-Package needed. Everything else still resolved from the jars
listed in Embed-Dependency — except `com.oracle.truffle.tools.utils.json`,
which lives in `org.graalvm.tools:profiler`, a transitive of chromeinspector
that was never listed there because bnd used to pull it in on its own.
The cost was not limited to the debugger. Enabling `polyglot.inspect` made
GraalVMEngine.activate() throw NoClassDefFoundError, the engine never came
back, and every JS view and action was gone for the rest of the run — which
is why the whole integration suite failed on this branch while main stayed
green: its first spec enables the debugger.
Listing `profiler` restores the three missing `com.oracle.truffle.tools`
packages; a package-level diff against a main build now shows no difference.
Verified on Jahia 8.2: enabling the inspector leaves the engine active,
`/json/version` answers `{"Protocol-Version":"1.2","Browser":"GraalVM"}` —
what the spec asserts — and with the debugger on, the test module still
registers its 106 components, pages render, and an action returns its
envelope.
None of these three specs has ever passed on this branch; with the engine
crash out of the way (previous commit) they are what is left of the red
suite.
**choicelistInitializerTest, nodeValidatorTest** passed their GraphQL
documents to `cy.apollo` as template-literal strings. The helper expects a
parsed document — it reads `.loc.source` — so every call died in
`@jahia/cypress` with "Cannot read properties of undefined (reading
'source')" before reaching Jahia. Their queries move to
`cypress/fixtures/graphql/`, consumed through `queryFile`/`mutationFile` like
every other spec in the suite.
**actionTest** read JSON bodies from legacy node actions without asking for
JSON. Jahia only serializes an action's JSON when the caller sends
`accept: application/json` (or `returnContentType=json`) — JS-declared legacy
actions follow the same contract as Java ones, by design — so the assertions
compared `undefined` against the expected values while the status codes
matched. The four body-reading requests now send the header.
Its redirect case was wrong on both ends: the fixture returned
`{statusCode: 302, redirect}`, and a result code >= 300 makes Jahia call
sendError() rather than redirect, which is why the response carried no
Location. The fixture now returns the target alone and lets the platform pick
the status, and the assertion accepts any 3xx.
Verified against a Jahia 8.2 instance: choicelist 5/5 and node validator 6/6
(both were 0/5 and 1/6). For actionTest only the CSRF-whitelisted POST case
can run here — the local instance's CSRF guard blocks the GET `.do` calls
that CI allows — and that case goes from failing to passing with the header,
which is the same mechanism behind the other three. CI covers the rest.
**Authenticated legacy action.** `testJsActionAuth` is a GET, and Jahia's CSRF guard challenges authenticated GET `.do` requests too, so the call never reached the action (400). The test module whitelists it next to the POST one, which was whitelisted for the same reason. **Choicelist localization.** The spec asked the shared test site's creation form for a `fr` content locale, but that site is created with `languages: 'en'`. Jahia 8.2.1.0 passed the requested locale through regardless — the assertion passed locally — while the version CI runs falls back to the site's language and the initializer correctly answered "Red". The spec now creates its own site with French enabled, like the i18n spec does, so it tests the initializer rather than a version's locale-fallback behaviour. Verified against Jahia 8.2: choicelist 5/5. For actionTest the local CSRF guard challenges every guest GET `.do` — the three tests that still fail here answer with the guard's /error.html redirect and are the ones CI already passes — but the authenticated case this commit targets goes from failing to passing locally (4/7 up from 2/7).
The spec asked for `uiLocale: "en", locale: "fr"` and expected the French label, because on Jahia 8.2.1.0 a choicelist initializer is handed the content locale — verified directly against the form API there: `locale: "fr"` yields "Rouge" whatever the uiLocale is, and `locale: "en"` yields "Red". The snapshot CI runs answers "Red" for that same request, so the initializer sees another locale there. Which locale reaches an initializer is the platform's decision; a module can only localize with what it is handed. The test now asks for French on both and keeps asserting the French label, so it verifies the initializer rather than a given Jahia's routing. The divergence is written down next to it and reported on the PR — it deserves an answer, but not from this suite. Verified against Jahia 8.2: 5/5.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #712 so that PR keeps to its own subject. These four commits are what takes this branch's integration suite from 41 of 41 specs failing to green.
profileradded toEmbed-Dependencyacceptheader in three specs1. The engine stopped surviving its own debugger test
javascript-modules-engine/pom.xmlgained an explicitExport-Packagefor the SDK package. That switches off bnd's implicit inclusion of dependency classes — the bundle drops from 9154 classes to 88. The embedded jars still cover almost everything, exceptcom.oracle.truffle.tools.utils.json, which lives inorg.graalvm.tools:profiler, a transitive of chromeinspector that was never listed because bnd used to pull it in by itself. A package-level diff isolates it: 308 packages reachable on both a main build and this branch, 3 missing here, allcom.oracle.truffle.tools.*.The suite's first spec enables
polyglot.inspect.GraalVMEngine.activate()then threwNoClassDefFoundErrorand the engine never came back, so every later spec ran against a Jahia with no JS views and no JS actions — hence 404s on test pages, 501s on.doendpoints, andPassing: 0everywhere.2. Three specs that had never passed
All three are new on this branch:
cy.apolloa template-literal string; the helper wants a parsed document (it reads.loc.source), so the calls died client-side before reaching Jahia. Their queries move tocypress/fixtures/graphql/, used throughqueryFile/mutationFilelike the rest of the suite.accept: application/json— JS-declared legacy actions follow the same contract as Java ones — so the bodies were empty while the status codes matched. Its redirect fixture also returned{statusCode: 302, redirect}, and a result code ≥ 300 makes Jahia callsendError()instead of redirecting, which is why noLocationcame back. It now returns the target alone..dorequests too, so it never reached the action. It is whitelisted next to the POST action that was whitelisted for the same reason.3. A platform difference worth your call
choicelistInitializerTestasserted that an initializer receives the content locale. Probed directly against Jahia 8.2.1.0, that is what happens:The snapshot CI runs answers
Redto the same request, and it is not the site's languages (re-tested with anen,frsite). Something changed about which locale reaches choicelist initializers. The module localizes with whatever it is handed, so the test now asks for French on both locales and keeps asserting the French label — it pins the initializer, not the platform's routing. Details in #712 (comment); worth chasing on the Jahia side, since "initializers receive the content locale" is what this feature's users will code against.Verification
Everything above was verified on a Jahia 8.2 instance, not only in CI: the inspector answers
{"Protocol-Version":"1.2","Browser":"GraalVM"}with the engine still active and the test module still serving views and actions; choicelist 5/5, node validators 6/6, and the authenticated-action case going from failing to passing. The remaining actionTest cases are GET.docalls that the local CSRF guard blocks and CI allows, so CI covers those.#712 is stacked on this branch so it stays green; once this merges, its base can go back to
feature/js-server-extensions.