This is an automated email from the ASF dual-hosted git repository. Cole-Greer pushed a commit to branch docs-3.7 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 0d8d9426456e558bd9cc492a9ce09567cefa2172 Author: Cole Greer <[email protected]> AuthorDate: Wed Jun 3 10:28:26 2026 -0700 Fail docs build on Gremlin execution errors instead of silent skip Rework the docs build so a genuine Gremlin error fails the build rather than rendering as a silently-empty block. Investigation confirmed no executed gremlin-groovy block is expected to error: error output goes to the console's stderr (the "Display stack trace?" prompt), which neither the old nor new build captured, and all error examples in the docs are hand-authored [source,text] blocks. GremlinConsole now records the stderr error prompt and surfaces it from execute() as a GremlinExecutionException; the treeprocessor propagates it (fatal) and the silent dry-run fallback is removed. The 90s timeout remains a failsafe and single restart-and-retry recovery is preserved. Enabling this surfaced several previously-masked setup issues, also fixed: - buildStatements tracks bracket depth so multi-line Groovy closures (e.g. "(1..10).each { ... }; []") stay grouped instead of hanging at a continuation prompt. - initGraphIfNeeded closes the prior graph and clears /tmp/neo4j and /tmp/tinkergraph.kryo before each block, so a stale Neo4j store lock no longer hangs Neo4jGraph.open (mirrors the old preprocessor). - SugarLoader runs on a freshly restarted console so it takes effect on a pristine Groovy metaclass. - process-docs.sh resolves the Neo4j DB impl onto the console classpath, strips only the conflicting io.netty 4.1.24 (keeping netty-3.9.x that Neo4j needs), and registers TinkerGraph and Credential plugins. REQUIREMENTS.md FR-4 updated. A remaining Neo4j/Spark Scala classpath conflict requiring per-book plugin isolation is tracked separately. (tinkerpop-6jq.14) Assisted-by: Kiro:claude-opus-4.8 [Kiro CLI] --- bin/process-docs.sh | 33 ++++- tools/tinkerpop-docs/REQUIREMENTS.md | 14 +- .../tinkerpop/gremlin/docs/GremlinConsole.java | 62 +++++---- .../gremlin/docs/GremlinTreeprocessor.java | 141 ++++++++++++--------- .../tinkerpop/gremlin/docs/GremlinConsoleTest.java | 62 ++++++++- .../gremlin/docs/GremlinTreeprocessorTest.java | 67 +++++++++- 6 files changed, 281 insertions(+), 98 deletions(-) diff --git a/bin/process-docs.sh b/bin/process-docs.sh index ea5345900d..81ac12c7db 100755 --- a/bin/process-docs.sh +++ b/bin/process-docs.sh @@ -130,9 +130,40 @@ for plugin in ${PLUGINS}; do fi done +# 3b. Resolve the Neo4j database implementation onto the console classpath. +# neo4j-gremlin declares neo4j-tinkerpop-api-impl as a Gremlin-Plugin-Dependencies manifest +# entry (test-scoped / gated behind the includeNeo4j profile), so it is not bundled by the +# local jar copy above. The old console ':install' flow fetched it via DependencyGrabber; +# here we resolve it (and its transitive deps) from Maven so Neo4jGraph blocks can execute. +NEO4J_IMPL_VERSION="0.9-3.4.0" +NEO4J_PLUGIN_LIB="${CONSOLE_HOME}/ext/neo4j-gremlin/lib" +if [ -d "${NEO4J_PLUGIN_LIB}" ] && ! ls "${NEO4J_PLUGIN_LIB}"/neo4j-tinkerpop-api-impl-*.jar >/dev/null 2>&1; then + echo " * resolving Neo4j implementation (neo4j-tinkerpop-api-impl:${NEO4J_IMPL_VERSION})" + NEO4J_POM="${TP_HOME}/target/neo4j-impl-pom.xml" + cat > "${NEO4J_POM}" <<POM +<project xmlns="http://maven.apache.org/POM/4.0.0";><modelVersion>4.0.0</modelVersion> +<groupId>org.apache.tinkerpop.docs</groupId><artifactId>neo4j-impl-resolver</artifactId><version>1</version> +<dependencies><dependency><groupId>org.neo4j</groupId><artifactId>neo4j-tinkerpop-api-impl</artifactId><version>${NEO4J_IMPL_VERSION}</version></dependency></dependencies></project> +POM + mvn -q -f "${NEO4J_POM}" dependency:copy-dependencies -DoutputDirectory="${NEO4J_PLUGIN_LIB}" + # Drop ONLY the conflicting io.netty 4.x jar that Neo4j pulls in (netty-all-4.1.24): it + # contains an older io.netty.handler.codec.http.websocketx.WebSocketClientHandshaker13 that + # shadows the console driver's 4.1.125 class and breaks ':remote' server connections with a + # NoSuchMethodError. Keep netty-3.9.x (org.jboss.netty package) -- it does NOT conflict and + # is required by Neo4j 3.4's IO layer. + rm -f "${NEO4J_PLUGIN_LIB}"/netty-all-4.*.jar + cp "${NEO4J_PLUGIN_LIB}/"*.jar "${CONSOLE_HOME}/lib/" 2>/dev/null +fi + # 4. Register plugins in console echo "Registering plugins..." -PLUGIN_CLASSES="org.apache.tinkerpop.gremlin.hadoop.jsr223.HadoopGremlinPlugin +# TinkerGraphGremlinPlugin must be (re)registered explicitly: the console rewrites plugins.txt +# to the set of successfully-activated plugins on startup, and a transient activation hiccup +# while bringing up the newly-added plugins can otherwise drop tinkergraph -- leaving +# TinkerFactory/TinkerGraph unavailable and failing the first doc block. +PLUGIN_CLASSES="org.apache.tinkerpop.gremlin.tinkergraph.jsr223.TinkerGraphGremlinPlugin +org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.CredentialGraphGremlinPlugin +org.apache.tinkerpop.gremlin.hadoop.jsr223.HadoopGremlinPlugin org.apache.tinkerpop.gremlin.spark.jsr223.SparkGremlinPlugin org.apache.tinkerpop.gremlin.neo4j.jsr223.Neo4jGremlinPlugin org.apache.tinkerpop.gremlin.sparql.jsr223.SparqlGremlinPlugin" diff --git a/tools/tinkerpop-docs/REQUIREMENTS.md b/tools/tinkerpop-docs/REQUIREMENTS.md index 0c7d444a04..2df1d336c2 100644 --- a/tools/tinkerpop-docs/REQUIREMENTS.md +++ b/tools/tinkerpop-docs/REQUIREMENTS.md @@ -77,9 +77,17 @@ Supported graph names: `modern`, `classic`, `theCrew`, `kitchenSink`, `gratefulD ### FR-4: Error Handling in Code Blocks -- When a Gremlin statement produces a runtime error, the extension SHALL capture the error message and include it in the console output — this IS the expected output for documentation purposes. -- Errors in code blocks SHALL NOT fail the docs build. -- The "Display stack trace? [yN]" prompt SHALL be dismissed automatically without user intervention. +- When a Gremlin statement produces a runtime error (signalled by the console's + `Display stack trace? [yN]` prompt), the extension SHALL fail the docs build, reporting the + failing statement. Executed `gremlin-groovy` blocks are not expected to error; documentation + examples that intentionally show errors are authored as manual `[source,text]`/`[source,<lang>]` + blocks (Gremlin error output goes to the console's stderr and is not captured into rendered + output by design). +- The "Display stack trace? [yN]" prompt SHALL be answered automatically (without user + intervention) solely to unblock the console process and detect the error; the build then fails. +- A statement exceeding the execution timeout, or a console that dies and does not recover after + a single restart-and-retry, SHALL fail the build. The timeout is a failsafe against indefinite + hanging, not an expected path. ### FR-5: Manual Language Variant Tabs diff --git a/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinConsole.java b/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinConsole.java index 530a26a79a..2ae49ee26a 100644 --- a/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinConsole.java +++ b/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinConsole.java @@ -53,6 +53,10 @@ public class GremlinConsole implements Closeable { private final long timeoutMs; private volatile boolean running; private boolean inEscape; + private volatile boolean errorPromptSeen; + private volatile String lastErrorText = ""; + private final StringBuilder errorCapture = new StringBuilder(); + private static final int MAX_ERROR_CAPTURE = 8192; /** * Creates a new GremlinConsole by starting the {@code bin/gremlin.sh} subprocess. @@ -93,14 +97,23 @@ public class GremlinConsole implements Closeable { * * @param statement the Gremlin statement to execute * @return the console output for this statement - * @throws IOException if an I/O error occurs - * @throws ConsoleTimeoutException if the prompt is not received within the timeout period + * @throws IOException if an I/O error occurs + * @throws ConsoleTimeoutException if the prompt is not received within the timeout period + * @throws GremlinExecutionException if the statement produced a Gremlin error */ public String execute(final String statement) throws IOException, ConsoleTimeoutException { - dismissPendingErrorPrompt(); + synchronized (stderrStdinLock) { + errorPromptSeen = false; + lastErrorText = ""; + errorCapture.setLength(0); + } stdin.write(statement + "\n"); stdin.flush(); - return readUntilPrompt(); + final String output = readUntilPrompt(); + if (errorPromptSeen) { + throw new GremlinExecutionException(statement, lastErrorText); + } + return output; } @Override @@ -190,27 +203,12 @@ public class GremlinConsole implements Closeable { } } - /** - * Dismisses any pending error prompt before sending a new statement. - */ - private void dismissPendingErrorPrompt() throws IOException { - synchronized (stderrStdinLock) { - if (stderr.ready()) { - final StringBuilder errBuf = new StringBuilder(); - while (stderr.ready()) { - errBuf.append((char) stderr.read()); - } - if (errBuf.toString().contains(ERROR_PROMPT)) { - stdin.write("\n"); - stdin.flush(); - } - } - } - } - /** * Background thread that monitors stderr and automatically dismisses error prompts. - * Uses a sliding window of the last N characters where N = ERROR_PROMPT.length(). + * The {@code Display stack trace? [yN]} prompt is the signal that the current statement + * raised a Gremlin error; the prompt is answered to unblock the process and the error is + * recorded so {@link #execute(String)} can surface it as a {@link GremlinExecutionException}. + * Error report text precedes the prompt on stderr, so it is captured for the message. */ private void dismissErrorPrompts() { final int windowSize = ERROR_PROMPT.length(); @@ -222,11 +220,17 @@ public class GremlinConsole implements Closeable { final int ch = stderr.read(); if (ch == -1) return; errBuffer.append((char) ch); + if (errorCapture.length() < MAX_ERROR_CAPTURE) { + errorCapture.append((char) ch); + } if (errBuffer.toString().contains(ERROR_PROMPT)) { stdin.write("\n"); stdin.flush(); + lastErrorText = errorCapture.toString().trim(); + errorPromptSeen = true; errBuffer.setLength(0); + errorCapture.setLength(0); } else if (errBuffer.length() > windowSize) { errBuffer.delete(0, errBuffer.length() - windowSize); } @@ -267,4 +271,16 @@ public class GremlinConsole implements Closeable { super(message); } } + + /** + * Thrown when an executed statement produced a Gremlin error (signalled by the console's + * {@code Display stack trace? [yN]} prompt). Such errors must fail the docs build rather + * than render as silently-empty output. + */ + public static class GremlinExecutionException extends RuntimeException { + public GremlinExecutionException(final String statement, final String errorText) { + super("Gremlin statement failed: " + statement + + (errorText == null || errorText.isEmpty() ? "" : "\n" + errorText)); + } + } } diff --git a/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessor.java b/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessor.java index 41e5c238b0..5185cdccde 100644 --- a/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessor.java +++ b/tools/tinkerpop-docs/src/main/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessor.java @@ -429,37 +429,12 @@ public class GremlinTreeprocessor extends Treeprocessor { try { return doBuildConsoleOutput(block, dryRun); } catch (final ConsoleRestartedException e) { - // Console was restarted — retry the entire block on the fresh console + // Console was restarted — retry the entire block once on the fresh console LOG.info("Retrying block after console restart"); - try { - return doBuildConsoleOutput(block, dryRun); - } catch (final ConsoleRestartedException e2) { - // Block is genuinely broken — skip it - LOG.warning("Block failed after retry, skipping: " + e2.getMessage()); - return buildDryRunOutput(block); - } + return doBuildConsoleOutput(block, dryRun); } } - private String buildDryRunOutput(final Block block) { - final String source = block.getSource(); - if (source == null || source.isEmpty()) return ""; - final StringBuilder output = new StringBuilder(); - final String[] lines = source.split("\\r?\\n"); - final List<String> statements = buildDisplayStatements(lines); - for (final String statement : statements) { - final String[] stmtLines = statement.split("\\r?\\n"); - for (int l = 0; l < stmtLines.length; l++) { - if (l == 0) { - output.append(PROMPT).append(stmtLines[l]).append("\n"); - } else { - output.append(" ").append(stmtLines[l]).append("\n"); - } - } - } - return output.toString().stripTrailing(); - } - private String doBuildConsoleOutput(final Block block, final boolean dryRun) { if (!dryRun) { ensureConsoleStarted(); @@ -474,17 +449,17 @@ public class GremlinTreeprocessor extends Treeprocessor { final List<String> displayStatements = buildDisplayStatements(lines); final List<String> execStatements = buildStatements(lines); - // Sugar syntax permanently mutates the Groovy metaclass, so run sugar - // blocks on a fresh console and mark it dirty to force a restart after. + // Sugar syntax permanently mutates the Groovy metaclass and only takes effect when + // SugarLoader.load() runs on a pristine metaclass -- i.e. BEFORE any traversal has been + // used in the JVM. Since the console is long-lived and has executed prior blocks, always + // restart to get a fresh console, then load sugar before initializing the graph/source. final boolean needsSugar = !dryRun && execStatements.stream().anyMatch(GremlinTreeprocessor::usesSugarSyntax); if (needsSugar && getActiveExecutor() != null) { - if (sugarLoaded) { - restartConsole(); - } - final String graphName = extractGraphName(block); - initGraphIfNeeded(graphName); + restartConsole(); executeSafely("org.apache.tinkerpop.gremlin.groovy.loaders.SugarLoader.load()"); sugarLoaded = true; + final String graphName = extractGraphName(block); + initGraphIfNeeded(graphName); } else if (!dryRun && getActiveExecutor() != null) { if (sugarLoaded) { // Previous block loaded sugar; restart to get a clean metaclass. @@ -526,28 +501,7 @@ public class GremlinTreeprocessor extends Treeprocessor { * Groups source lines into complete statements for execution. Strips callouts. */ static List<String> buildStatements(final String[] lines) { - final List<String> statements = new ArrayList<>(); - final StringBuilder current = new StringBuilder(); - boolean inTripleQuote = false; - for (final String line : lines) { - final String cleaned = stripCallouts(line); - // Track triple-quote state - final int tqCount = countOccurrences(cleaned, "\"\"\""); - if (current.length() == 0) { - current.append(cleaned); - } else if (inTripleQuote || (cleaned.length() > 0 && Character.isWhitespace(cleaned.charAt(0)))) { - current.append("\n").append(cleaned); - } else { - statements.add(current.toString()); - current.setLength(0); - current.append(cleaned); - } - if (tqCount % 2 != 0) inTripleQuote = !inTripleQuote; - } - if (current.length() > 0) { - statements.add(current.toString()); - } - return statements; + return groupStatements(lines, true); } private static int countOccurrences(final String str, final String sub) { @@ -561,21 +515,42 @@ public class GremlinTreeprocessor extends Treeprocessor { * Groups source lines into complete statements for display. Preserves callouts. */ static List<String> buildDisplayStatements(final String[] lines) { + return groupStatements(lines, false); + } + + /** + * Groups source lines into complete statements. A new statement starts only at a + * non-indented line when the accumulated statement has balanced brackets and is not + * inside a triple-quoted string. Tracking bracket depth keeps multi-line Groovy + * constructs (e.g. {@code (1..10).each \{ ... \}}) together even though their closing + * line is not indented, which would otherwise send an incomplete statement to the + * console and hang at a continuation prompt. + * + * @param strip when {@code true}, callouts are stripped from the emitted text (for + * execution); when {@code false}, the original line is emitted (for display) + */ + private static List<String> groupStatements(final String[] lines, final boolean strip) { final List<String> statements = new ArrayList<>(); final StringBuilder current = new StringBuilder(); boolean inTripleQuote = false; + int depth = 0; for (final String line : lines) { - final String stripped = stripCallouts(line); - final int tqCount = countOccurrences(stripped, "\"\"\""); + final String detect = stripCallouts(line); + final String content = strip ? detect : line; + final int tqCount = countOccurrences(detect, "\"\"\""); + final boolean touchesTriple = inTripleQuote || tqCount > 0; if (current.length() == 0) { - current.append(line); - } else if (inTripleQuote || (stripped.length() > 0 && Character.isWhitespace(stripped.charAt(0)))) { - current.append("\n").append(line); + current.append(content); + } else if (inTripleQuote || depth > 0 + || (detect.length() > 0 && Character.isWhitespace(detect.charAt(0)))) { + current.append("\n").append(content); } else { statements.add(current.toString()); current.setLength(0); - current.append(line); + depth = 0; + current.append(content); } + if (!touchesTriple) depth += bracketDelta(detect); if (tqCount % 2 != 0) inTripleQuote = !inTripleQuote; } if (current.length() > 0) { @@ -584,6 +559,36 @@ public class GremlinTreeprocessor extends Treeprocessor { return statements; } + /** + * Net change in bracket nesting ({@code (} {@code [} {@code \{}) contributed by a line, + * ignoring brackets inside single- or double-quoted string literals (so GString + * interpolation like {@code ${it}} does not affect the count). + */ + private static int bracketDelta(final String s) { + int d = 0; + boolean sq = false; + boolean dq = false; + for (int i = 0; i < s.length(); i++) { + final char c = s.charAt(i); + if (sq) { + if (c == '\\') i++; + else if (c == '\'') sq = false; + } else if (dq) { + if (c == '\\') i++; + else if (c == '"') dq = false; + } else if (c == '\'') { + sq = true; + } else if (c == '"') { + dq = true; + } else if (c == '{' || c == '(' || c == '[') { + d++; + } else if (c == '}' || c == ')' || c == ']') { + d--; + } + } + return d; + } + /** * Strips AsciiDoc callout markers (e.g. {@code <1>}, {@code <2>}) from the end of a line. */ @@ -628,6 +633,15 @@ public class GremlinTreeprocessor extends Treeprocessor { return; } + // Clear file-backed graph stores reused across blocks. Neo4j holds an exclusive + // store lock, so a stale '/tmp/neo4j' left by a prior block (or prior build run) + // makes the next Neo4jGraph.open('/tmp/neo4j') hang acquiring the lock. Close any + // prior graph to release its lock, then delete the dirs -- mirroring the old + // preprocessor which cleared these before every graph-init block. + executeSafely("try { if (binding.hasVariable('graph') && graph != null) graph.close() } catch (e) {}"); + executeSafely("['/tmp/neo4j', '/tmp/tinkergraph.kryo'].each { p -> " + + "def f = new File(p); if (f.exists()) f.deleteDir() }"); + final String initStatement; if (graphName == null) { initStatement = "graph = TinkerGraph.open()"; @@ -647,6 +661,9 @@ public class GremlinTreeprocessor extends Treeprocessor { private String executeSafely(final String statement) { try { return getActiveExecutor().execute(statement); + } catch (final GremlinConsole.GremlinExecutionException e) { + // A genuine Gremlin error must fail the build, not render as empty output. + throw e; } catch (final GremlinConsole.ConsoleTimeoutException | IOException e) { // Console may have died — restart it and propagate to retry at block level LOG.warning("Console appears dead, restarting: " + e.getMessage()); diff --git a/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinConsoleTest.java b/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinConsoleTest.java index 15ce98f154..d1a7fe5c86 100644 --- a/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinConsoleTest.java +++ b/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinConsoleTest.java @@ -29,9 +29,12 @@ import java.io.PipedInputStream; import java.io.PipedOutputStream; import java.nio.charset.StandardCharsets; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; /** @@ -136,7 +139,10 @@ public class GremlinConsoleTest { @Test public void shouldDismissErrorPromptOnStderr() throws Exception { - final String fullStdout = "gremlin>" + "gremlin>"; + // The async dismisser must answer the "Display stack trace? [yN]" prompt on stderr + // (sending a newline to stdin) so the console process is never left blocked. Surfacing + // the error to the caller is covered by shouldThrowExecutionExceptionWhenErrorPromptSeen. + final String fullStdout = "gremlin>"; final String stderrContent = "Display stack trace? [yN]"; final ByteArrayOutputStream capturedStdin = new ByteArrayOutputStream(); final GremlinConsole console = createConsoleWithStdin(fullStdout, stderrContent, capturedStdin); @@ -147,8 +153,6 @@ public class GremlinConsoleTest { while (capturedStdin.size() == 0 && System.currentTimeMillis() < deadline) { Thread.sleep(10); } - final String result = console.execute("g.V()"); - assertThat(result, is("")); // Verify that a newline was sent to dismiss the error prompt final String sent = capturedStdin.toString(StandardCharsets.UTF_8.name()); assertThat(sent.contains("\n"), is(true)); @@ -157,6 +161,58 @@ public class GremlinConsoleTest { } } + @Test + public void shouldThrowExecutionExceptionWhenErrorPromptSeen() throws Exception { + // Mirror the real console ordering: after the statement is sent, the error report + + // "Display stack trace?" prompt arrive on stderr and the process blocks on stdin until + // answered, so the next gremlin> prompt on stdout appears only AFTER the dismisser + // replies. execute() must then surface a GremlinExecutionException so the build fails. + final PipedOutputStream stdoutFeeder = new PipedOutputStream(); + final PipedInputStream stdoutStream = new PipedInputStream(stdoutFeeder); + stdoutFeeder.write("gremlin>".getBytes(StandardCharsets.UTF_8)); + stdoutFeeder.flush(); + + final PipedOutputStream stderrFeeder = new PipedOutputStream(); + final PipedInputStream stderrStream = new PipedInputStream(stderrFeeder); + final ByteArrayOutputStream capturedStdin = new ByteArrayOutputStream(); + final GremlinConsole console = new GremlinConsole( + new MockProcess(stdoutStream, stderrStream, capturedStdin), 5_000); + + final AtomicReference<Throwable> thrown = new AtomicReference<>(); + final Thread exec = new Thread(() -> { + try { + console.execute("g.V().fail('boom')"); + } catch (final Throwable t) { + thrown.set(t); + } + }); + exec.start(); + try { + // Emit the error report + prompt on stderr only after execute() has started, so the + // per-statement error flag is reset before the dismisser observes the prompt. + Thread.sleep(200); + stderrFeeder.write("fail() Step Triggered\nDisplay stack trace? [yN]".getBytes(StandardCharsets.UTF_8)); + stderrFeeder.flush(); + // Wait for the dismisser to answer (an extra newline beyond the statement itself, + // which execute() also wrote to stdin), then release the next prompt on stdout. + final int statementBytes = "g.V().fail('boom')\n".getBytes(StandardCharsets.UTF_8).length; + final long deadline = System.currentTimeMillis() + 5000; + while (capturedStdin.size() <= statementBytes && System.currentTimeMillis() < deadline) { + Thread.sleep(10); + } + stdoutFeeder.write("gremlin>".getBytes(StandardCharsets.UTF_8)); + stdoutFeeder.flush(); + exec.join(5000); + } finally { + console.close(); + stdoutFeeder.close(); + stderrFeeder.close(); + } + assertThat(thrown.get(), is(notNullValue())); + assertThat(thrown.get(), instanceOf(GremlinConsole.GremlinExecutionException.class)); + assertThat(thrown.get().getMessage(), containsString("g.V().fail('boom')")); + } + @Test public void shouldThrowIOExceptionWhenProcessDiesMidRead() throws Exception { final PipedOutputStream feeder = new PipedOutputStream(); diff --git a/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessorTest.java b/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessorTest.java index 55c93a7e62..ac15a64679 100644 --- a/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessorTest.java +++ b/tools/tinkerpop-docs/src/test/java/org/apache/tinkerpop/gremlin/docs/GremlinTreeprocessorTest.java @@ -23,7 +23,6 @@ import org.asciidoctor.Attributes; import org.asciidoctor.Options; import org.junit.Test; -import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -164,16 +163,41 @@ public class GremlinTreeprocessorTest { } @Test - public void shouldCaptureErrorsWithoutFailingBuild() { - final GremlinTreeprocessor.StatementExecutor failingExecutor = statement -> { - throw new IOException("Simulated error"); + public void shouldFailBuildWhenStatementErrors() { + // A statement that raises a Gremlin error (signalled by the console via a + // GremlinExecutionException) must fail the docs build rather than render as a + // silently-empty block. + final String erroringStatement = "g.V().fail('we failed!')"; + final GremlinTreeprocessor.StatementExecutor erroringExecutor = statement -> { + if (statement.equals(erroringStatement)) { + throw new GremlinConsole.GremlinExecutionException(statement, "fail() Step Triggered"); + } + return "==>ok"; }; - final GremlinTreeprocessor processor = new GremlinTreeprocessor(failingExecutor); + final GremlinTreeprocessor processor = new GremlinTreeprocessor(erroringExecutor); try (final Asciidoctor asciidoctor = Asciidoctor.Factory.create()) { asciidoctor.unregisterAllExtensions(); asciidoctor.javaExtensionRegistry().treeprocessor(processor); - final String input = "= Test\n\n[gremlin-groovy,modern]\n----\ninvalid()\n----\n"; + final String input = "= Test\n\n[gremlin-groovy,modern]\n----\n" + erroringStatement + "\n----\n"; + asciidoctor.convert(input, Options.builder().build()); + throw new AssertionError("Expected build to fail with GremlinExecutionException"); + } catch (final GremlinConsole.GremlinExecutionException e) { + assertThat(e.getMessage(), containsString(erroringStatement)); + } + } + + @Test + public void shouldNotFailBuildForEmptyResultBlock() { + // A block that legitimately produces no ==> result (e.g. a void iterate() or an empty + // service list) is NOT an error and must build successfully. + final RecordingExecutor executor = new RecordingExecutor(""); + final GremlinTreeprocessor processor = new GremlinTreeprocessor(executor); + + try (final Asciidoctor asciidoctor = Asciidoctor.Factory.create()) { + asciidoctor.unregisterAllExtensions(); + asciidoctor.javaExtensionRegistry().treeprocessor(processor); + final String input = "= Test\n\n[gremlin-groovy,modern]\n----\ng.V().iterate()\n----\n"; final String result = asciidoctor.convert(input, Options.builder().build()); assertThat(result, is(notNullValue())); assertThat(processor.getGremlinBlockCount(), is(1)); @@ -199,6 +223,37 @@ public class GremlinTreeprocessorTest { } } + @Test + public void shouldKeepMultiLineClosureAsSingleStatement() { + // A Groovy closure whose closing line is not indented (e.g. "}; []") must stay grouped + // with its opening line; splitting it would send an incomplete statement to the console + // and hang at a continuation prompt. + final String[] lines = { + "(1..10).each {", + " g.addV(\"product\").property(\"name\",\"product #${it}\").iterate()", + "}; []", + "g.V().count()" + }; + final List<String> statements = GremlinTreeprocessor.buildStatements(lines); + assertThat(statements.size(), is(2)); + assertThat(statements.get(0), containsString("(1..10).each {")); + assertThat(statements.get(0), containsString("}; []")); + assertThat(statements.get(1), is("g.V().count()")); + } + + @Test + public void shouldNotCountBracketsInsideStrings() { + // Brackets inside string literals (including GString interpolation) must not affect + // statement grouping. + final String[] lines = { + "x = '}'", + "y = \"${a}\"", + "z = 1" + }; + final List<String> statements = GremlinTreeprocessor.buildStatements(lines); + assertThat(statements.size(), is(3)); + } + @Test public void shouldHandleCrewGraph() { final RecordingExecutor executor = new RecordingExecutor("==>result");
