Skip to content

Commit 779ff0e

Browse files
vladakvladak
committed
check long descriptions
1 parent 52ef487 commit 779ff0e

File tree

3 files changed

+54
-26
lines changed

3 files changed

+54
-26
lines changed

‎opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,9 @@ public static void main(String[] argv) {
413413
* This method was created so that it would be easier to write unit
414414
* tests against the Indexer option parsing mechanism.
415415
*
416+
* Limit usage lines to {@link org.opengrok.indexer.util.OptionParser.Option#MAX_DESCRIPTION_LINE_LENGTH}
417+
* characters for concise formatting.
418+
*
416419
* @param argv the command line arguments
417420
* @return array of remaining non option arguments
418421
* @throws ParseException if parsing failed
@@ -447,8 +450,6 @@ public static String[] parseOptions(String[] argv) throws ParseException {
447450

448451
searchPaths.clear();
449452

450-
// Limit usage lines to 72 characters for concise formatting.
451-
452453
optParser = OptionParser.execute(parser -> {
453454
parser.setPrologue(String.format("%nUsage: java -jar %s [options] [subDir1 [...]]%n", OPENGROK_JAR));
454455

@@ -1017,7 +1018,8 @@ public void prepareIndexer(RuntimeEnvironment env,
10171018
if (repositories != null && !repositories.isEmpty()) {
10181019
LOGGER.log(Level.INFO, "Generating history cache for repositories: {0}",
10191020
String.join(",", repositories));
1020-
HistoryGuru.getInstance().createCache(repositories);
1021+
HistoryGuru.getInstance().
1022+
createCache(repositories);
10211023
} else {
10221024
LOGGER.log(Level.INFO, "Generating history cache for all repositories ...");
10231025
HistoryGuru.getInstance().createCache();

‎opengrok-indexer/src/main/java/org/opengrok/indexer/util/OptionParser.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,19 @@ void setPattern(String pattern) {
156156
valuePattern = Pattern.compile(pattern);
157157
}
158158

159-
void addDescription(String descrip) {
160-
if (description == null) {
161-
description = new StringBuilder();
159+
public static final int MAX_DESCRIPTION_LINE_LENGTH = 72;
160+
161+
void addDescription(String description) {
162+
if (description.length() > MAX_DESCRIPTION_LINE_LENGTH) {
163+
throw new IllegalArgumentException(String.format("description line longer than %d characters: '%s'",
164+
MAX_DESCRIPTION_LINE_LENGTH, description));
165+
}
166+
167+
if (this.description == null) {
168+
this.description = new StringBuilder();
162169
}
163-
description.append(descrip);
164-
description.append("\n");
170+
this.description.append(description);
171+
this.description.append("\n");
165172
}
166173

167174
/**

‎opengrok-indexer/src/test/java/org/opengrok/indexer/util/OptionParserTest.java

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919

2020
/*
21+
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
2122
* Portions Copyright (c) 2017, Steven Haehn.
2223
*/
2324
package org.opengrok.indexer.util;
@@ -30,16 +31,18 @@
3031
import org.opengrok.indexer.index.Indexer;
3132

3233
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
34+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
3335
import static org.junit.jupiter.api.Assertions.assertEquals;
3436
import static org.junit.jupiter.api.Assertions.assertFalse;
3537
import static org.junit.jupiter.api.Assertions.assertNotNull;
3638
import static org.junit.jupiter.api.Assertions.assertNull;
39+
import static org.junit.jupiter.api.Assertions.assertThrows;
3740
import static org.junit.jupiter.api.Assertions.assertTrue;
3841

3942
/**
4043
* @author shaehn
4144
*/
42-
public class OptionParserTest {
45+
class OptionParserTest {
4346

4447
int actionCounter;
4548

@@ -50,7 +53,7 @@ public void setUp() {
5053

5154
// Scan parser should ignore all options it does not recognize.
5255
@Test
53-
public void scanParserIgnoreUnrecognizableOptions() throws ParseException {
56+
void scanParserIgnoreUnrecognizableOptions() throws ParseException {
5457

5558
String configPath = "/the/config/path";
5659

@@ -69,7 +72,7 @@ public void scanParserIgnoreUnrecognizableOptions() throws ParseException {
6972
// Validate that option can have multiple names
7073
// with both short and long versions.
7174
@Test
72-
public void optionNameAliases() throws ParseException {
75+
void optionNameAliases() throws ParseException {
7376

7477
OptionParser opts = OptionParser.execute(parser -> {
7578

@@ -91,7 +94,7 @@ public void optionNameAliases() throws ParseException {
9194
// Show that parser will throw exception
9295
// when option is not recognized.
9396
@Test
94-
public void unrecognizedOption() {
97+
void unrecognizedOption() {
9598

9699
OptionParser opts = OptionParser.execute(parser -> {
97100
parser.on("-?", "--help").execute(v -> {
@@ -110,7 +113,7 @@ public void unrecognizedOption() {
110113

111114
// Show that parser will throw exception when missing option value
112115
@Test
113-
public void missingOptionValue() {
116+
void missingOptionValue() {
114117

115118
OptionParser opts = OptionParser.execute(parser -> {
116119
parser.on("-a=VALUE").execute(v -> {
@@ -131,7 +134,7 @@ public void missingOptionValue() {
131134
// it is glued next to the option (eg. -xValue), or comes
132135
// as a following argument (eg. -x Value)
133136
@Test
134-
public void shortOptionValue() throws ParseException {
137+
void shortOptionValue() throws ParseException {
135138

136139
OptionParser opts = OptionParser.execute(parser -> {
137140
parser.on("-a=VALUE").execute(v -> {
@@ -152,7 +155,7 @@ public void shortOptionValue() throws ParseException {
152155
// Validate the ability of parser to convert
153156
// string option values into internal data types.
154157
@Test
155-
public void testSupportedDataCoercion() throws ParseException {
158+
void testSupportedDataCoercion() throws ParseException {
156159

157160
OptionParser opts = OptionParser.execute(parser -> {
158161

@@ -227,7 +230,7 @@ public void testSupportedDataCoercion() throws ParseException {
227230
// Make sure that option can take specific addOption of values
228231
// and when an candidate values is seen, an exception is given.
229232
@Test
230-
public void specificOptionValues() {
233+
void specificOptionValues() {
231234

232235
OptionParser opts = OptionParser.execute(parser -> {
233236
String[] onOff = {"on", "off"};
@@ -253,7 +256,7 @@ public void specificOptionValues() {
253256

254257
// See that option value matches a regular expression
255258
@Test
256-
public void optionValuePatternMatch() {
259+
void optionValuePatternMatch() {
257260

258261
OptionParser opts = OptionParser.execute(parser -> {
259262

@@ -286,7 +289,7 @@ public void optionValuePatternMatch() {
286289

287290
// Verify option may have non-required value
288291
@Test
289-
public void missingValueOnOptionAllowed() throws ParseException {
292+
void missingValueOnOptionAllowed() throws ParseException {
290293

291294
OptionParser opts = OptionParser.execute(parser -> {
292295

@@ -347,7 +350,7 @@ public void missingValueOnOptionAllowed() throws ParseException {
347350

348351
// Verify default option summary
349352
@Test
350-
public void defaultOptionSummary() {
353+
void defaultOptionSummary() {
351354
OptionParser opts = OptionParser.execute(parser -> {
352355
parser.on("--help").execute(v -> {
353356
String summary = parser.getUsage();
@@ -369,7 +372,7 @@ public void defaultOptionSummary() {
369372
// Therefore, must be able to catch when option entry matches more
370373
// than one entry.
371374
@Test
372-
public void catchAmbigousOptions() {
375+
void catchAmbigousOptions() {
373376
OptionParser opts = OptionParser.execute(parser -> {
374377
parser.on("--help");
375378
parser.on("--help-me-out");
@@ -386,7 +389,7 @@ public void catchAmbigousOptions() {
386389

387390
// Allow user to enter an initial substring to long option names
388391
@Test
389-
public void allowInitialSubstringOptionNames() throws ParseException {
392+
void allowInitialSubstringOptionNames() throws ParseException {
390393
OptionParser opts = OptionParser.execute(parser -> {
391394
parser.on("--help-me-out").execute(v -> actionCounter++);
392395
});
@@ -398,7 +401,7 @@ public void allowInitialSubstringOptionNames() throws ParseException {
398401

399402
// Specific test to evalutate the internal option candidate method
400403
@Test
401-
public void testInitialSubstringOptionNames() throws ParseException {
404+
void testInitialSubstringOptionNames() throws ParseException {
402405
OptionParser opts = OptionParser.execute(parser -> {
403406
parser.on("--help-me-out");
404407
parser.on("--longOption");
@@ -411,7 +414,7 @@ public void testInitialSubstringOptionNames() throws ParseException {
411414

412415
// Catch duplicate option names in parser construction.
413416
@Test
414-
public void catchDuplicateOptionNames() {
417+
void catchDuplicateOptionNames() {
415418
try {
416419
OptionParser.execute(parser -> {
417420
parser.on("--duplicate");
@@ -425,7 +428,7 @@ public void catchDuplicateOptionNames() {
425428

426429
// Catch single '-' in argument list
427430
@Test
428-
public void catchNamelessOption() {
431+
void catchNamelessOption() {
429432
OptionParser opts = OptionParser.execute(parser -> {
430433
parser.on("--help-me-out");
431434
});
@@ -441,9 +444,9 @@ public void catchNamelessOption() {
441444

442445
// Fail options put into Indexer.java that do not have a description.
443446
@Test
444-
public void catchIndexerOptionsWithoutDescription() throws NoSuchFieldException, IllegalAccessException, ParseException {
447+
void catchIndexerOptionsWithoutDescription() throws NoSuchFieldException, IllegalAccessException, ParseException {
445448
String[] argv = {"---unitTest"};
446-
Indexer.parseOptions(argv);
449+
assertDoesNotThrow(() -> Indexer.parseOptions(argv));
447450

448451
// Use reflection to get the option parser from Indexer.
449452
Field f = Indexer.class.getDeclaredField("optParser");
@@ -465,4 +468,20 @@ public void catchIndexerOptionsWithoutDescription() throws NoSuchFieldException,
465468
assertNull(o.description);
466469
}
467470
}
471+
472+
@Test
473+
void testAddLongDescription() {
474+
String longDescription = "A".repeat(OptionParser.Option.MAX_DESCRIPTION_LINE_LENGTH + 1);
475+
476+
assertThrows(IllegalArgumentException.class, () ->
477+
OptionParser.execute(parser -> {
478+
parser.on("--foo", longDescription);
479+
}));
480+
}
481+
482+
@Test
483+
void testParseOptions() {
484+
String[] argv = {"---unitTest"};
485+
assertDoesNotThrow(() -> Indexer.parseOptions(argv));
486+
}
468487
}

0 commit comments

Comments
 (0)