Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add --failure-level command line option to force non-zero exit code (fixes #1113) #1114

Merged
merged 3 commits into from
Sep 9, 2022
Merged

Add --failure-level command line option to force non-zero exit code (fixes #1113) #1114

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d876c33
Add --failure-level command line option to force non-zero exit code f…
pasieronen Sep 2, 2022
dac99a5
Improve error messages when parsing --failure-level parameter
pasieronen Sep 9, 2022
beb9f6b
Implement maxSeverity tracking on log handler
pasieronen Sep 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Improvement::

* BREAKING: Fix Macro APIs to take StructuralNodes and return Phrase- or StructuralNodes. (#1084)
* BREAKING: Allow Preprocessor extensions to create new Readers and replace the original Reader. (#1081)
* Add command line option --failure-level to force non-zero exit code from AsciidoctorJ CLI if specified logging level is reached. (#1114)
* Upgrade to asciidoctorj-pdf 2.3.0 (#1109)
* Upgrade to JRuby 9.3.7.0 (#1109)
* Upgrade to tilt 2.0.11 (#1109)
Expand Down
6 changes: 6 additions & 0 deletions asciidoctorj-core/src/main/java/org/asciidoctor/jruby/cli/AsciidoctorCliOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.beust.jcommander.Parameter;
import org.asciidoctor.*;
import org.asciidoctor.log.Severity;

import java.io.File;
import java.util.*;
Expand Down Expand Up @@ -95,6 +96,9 @@ public class AsciidoctorCliOptions {
@Parameter(names = {QUIET, "--quiet"}, description = "suppress warnings (default: false)")
private boolean quiet = false;

@Parameter(names = {"--failure-level"}, converter = SeverityConverter.class, description = "set minimum log level that yields a non-zero exit code: [info, warning, error, fatal] (default: fatal) ")
private Severity failureLevel = Severity.FATAL;

@Parameter(names = {REQUIRE, "--require"}, description = "require the specified library before executing the processor (using require)")
private List<String> require;

Expand All @@ -111,6 +115,8 @@ public boolean isQuiet() {
return quiet;
}

public Severity getFailureLevel() { return failureLevel; }

public boolean isRequire() {
return this.require != null && this.require.size() > 0;
}
Expand Down
14 changes: 11 additions & 3 deletions asciidoctorj-core/src/main/java/org/asciidoctor/jruby/cli/AsciidoctorInvoker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class AsciidoctorInvoker {

private static final String LINE_SEPARATOR = System.getProperty("line.separator");

public void invoke(String... parameters) {
public int invoke(String... parameters) {

AsciidoctorCliOptions asciidoctorCliOptions = new AsciidoctorCliOptions();
JCommander jCommander = new JCommander(asciidoctorCliOptions);
Expand All @@ -43,7 +43,7 @@ public void invoke(String... parameters) {
System.out.println("AsciidoctorJ " + getAsciidoctorJVersion() + " (Asciidoctor " + asciidoctor.asciidoctorVersion() + ") [https://asciidoctor.org]");
Object rubyVersionString = JRubyRuntimeContext.get(asciidoctor).evalScriptlet("\"#{JRUBY_VERSION} (#{RUBY_VERSION})\"");
System.out.println("Runtime Environment: jruby " + rubyVersionString);
return;
return 0;
}

List<File> inputFiles = getInputFiles(asciidoctorCliOptions);
Expand Down Expand Up @@ -72,12 +72,17 @@ public void invoke(String... parameters) {

convertInput(asciidoctor, options, inputFiles);

if (asciidoctorCliOptions.getFailureLevel().compareTo(asciidoctor.getMaxSeverity()) <= 0) {
return 1;
}

if (asciidoctorCliOptions.isTimings()) {
Map<String, Object> optionsMap = options.map();
IRubyObject timings = (IRubyObject) optionsMap.get(TIMINGS_OPTION_NAME);
timings.callMethod(JRubyRuntimeContext.get(asciidoctor).getCurrentContext(), "print_report");
}
}
return 0;
}

private String getAsciidoctorJVersion() {
Expand Down Expand Up @@ -196,7 +201,10 @@ public static void main(String args[]) {
// Process .jrubyrc file
Main.processDotfile();

new AsciidoctorInvoker().invoke(args);
int status = new AsciidoctorInvoker().invoke(args);
if (status != 0) {
System.exit(status);
}
}

}
12 changes: 12 additions & 0 deletions asciidoctorj-core/src/main/java/org/asciidoctor/jruby/cli/SeverityConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.asciidoctor.jruby.cli;

import com.beust.jcommander.IStringConverter;
import org.asciidoctor.SafeMode;
import org.asciidoctor.log.Severity;

public class SeverityConverter implements IStringConverter<Severity> {

@Override
public Severity convert(String argument) { return Severity.valueOf(argument.toUpperCase()); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I pass an invalid option like INFOX I currently get a stacktrace back that does not show the currently supported options:

Exception in thread "main" java.lang.IllegalArgumentException: No enum constant org.asciidoctor.log.Severity.INFOX
	at java.base/java.lang.Enum.valueOf(Enum.java:273)
	at org.asciidoctor.log.Severity.valueOf(Severity.java:3)
	at org.asciidoctor.jruby.cli.SeverityConverter.convert(SeverityConverter.java:10)
...

What do you think about changing it so that you'll get just a message with the invalid value and all supported values like:

Could not convert severity 'INFOX'. Supported values are [DEBUG, INFO, WARN, ERROR, FATAL, UNKNOWN]

Nitpick: Could you please add newlines after the opening brace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree - and it turns out JCommander has EnumConverter class which gets us this nice error message for free.


}
10 changes: 10 additions & 0 deletions asciidoctorj-core/src/main/java/org/asciidoctor/jruby/internal/JRubyAsciidoctor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.asciidoctor.jruby.syntaxhighlighter.internal.SyntaxHighlighterRegistryExecutor;
import org.asciidoctor.log.LogHandler;
import org.asciidoctor.log.LogRecord;
import org.asciidoctor.log.Severity;
import org.asciidoctor.syntaxhighlighter.SyntaxHighlighterRegistry;
import org.jruby.*;
import org.jruby.exceptions.RaiseException;
Expand All @@ -46,6 +47,8 @@ public class JRubyAsciidoctor implements AsciidoctorJRuby, LogHandler {

private List<LogHandler> logHandlers = new ArrayList<>();

private Severity maxSeverity = Severity.DEBUG;

public JRubyAsciidoctor() {
this(createRubyRuntime(Collections.singletonMap(GEM_PATH, null), new ArrayList<>(), null));
processRegistrations(this);
Expand Down Expand Up @@ -507,8 +510,15 @@ private RubyClass getExtensionGroupClass() {

@Override
public void log(LogRecord logRecord) {
if (this.maxSeverity.compareTo(logRecord.getSeverity()) < 0) {
this.maxSeverity = logRecord.getSeverity();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JRubyAsciidoctor can be used in a multithreaded way when embedded into other environments like Spring Boot apps or sth similar.
In such scenarios it would be dangerous to rely on this property.
Could this be moved into a custom org.asciidoctor.log.LogHandler that is only added by the AsciidoctorInvoker?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, moved this to MaxSeverityLogHandler under org.asciidoctor.jruby.cli package.

}
for (LogHandler logHandler : logHandlers) {
logHandler.log(logRecord);
}
}

public Severity getMaxSeverity() {
return this.maxSeverity;
}
}
16 changes: 16 additions & 0 deletions ...doctorj-core/src/test/java/org/asciidoctor/jruby/cli/WhenAsciidoctorIsCalledUsingCli.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,22 @@ public void quiet_option_should_not_write_to_console() {

}

@Test
public void should_exit_with_zero_status_even_if_errors_were_logged() {
File inputFile = classpath.getResource("brokeninclude.asciidoc");
String inputPath = inputFile.getPath().substring(pwd.length() + 1);
int status = new AsciidoctorInvoker().invoke(inputPath);
assertThat(status, is(0));
}

@Test
public void should_exit_with_nonzero_status_if_logged_severity_was_at_least_failure_level() {
File inputFile = classpath.getResource("brokeninclude.asciidoc");
String inputPath = inputFile.getPath().substring(pwd.length() + 1);
int status = new AsciidoctorInvoker().invoke("--failure-level", "warn", inputPath);
assertThat(status, is(1));
}

@Test
public void with_absolute_path_file_should_be_rendered() {

Expand Down