Repository: junit-team/junit5 (language: java)

Issue:
@CsvFileSource is unusable for datasets involving very long lines
# Description

If you want to use a CSV file-based dataset, that contains very long values in the columns, you have to increase the `maxCharsPerColumn` property from the default value of `4096`.  If, however, you don't know what is the length of the largest datapoint in your dataset, or cannot be sure to set a hard limit for future expansion, a logical thing to do would be to set the value to the largest possible one, i.e. `Integer.MAX_VALUE`.

This to my surprise crashes the test execution with :
```
Exception in thread "main" java.lang.OutOfMemoryError: Requested array size exceeds VM limit
	at org.junit.jupiter.params.shadow.com.univocity.parsers.common.input.DefaultCharAppender.<init>(DefaultCharAppender.java:40)
	at org.junit.jupiter.params.shadow.com.univocity.parsers.csv.CsvParserSettings.newCharAppender(CsvParserSettings.java:93)
	at org.junit.jupiter.params.shadow.com.univocity.parsers.common.ParserOutput.<init>(ParserOutput.java:111)
	at org.junit.jupiter.params.shadow.com.univocity.parsers.common.AbstractParser.<init>(AbstractParser.java:91)
	at org.junit.jupiter.params.shadow.com.univocity.parsers.csv.CsvParser.<init>(CsvParser.java:70)
	at org.junit.jupiter.params.provider.CsvParserFactory.createParser(CsvParserFactory.java:61)
	at org.junit.jupiter.params.provider.CsvParserFactory.createParserFor(CsvParserFactory.java:40)
	at org.junit.jupiter.params.provider.CsvFileArgumentsProvider.provideArguments(CsvFileArgumentsProvider.java:64)
	at org.junit.jupiter.params.provider.CsvFileArgumentsProvider.provideArguments(CsvFileArgumentsProvider.java:44)
	at org.junit.jupiter.params.provider.AnnotationBasedArgumentsProvider.provideArguments(AnnotationBasedArgumentsProvider.java:52)
	at org.junit.jupiter.params.ParameterizedTestExtension.arguments(ParameterizedTestExtension.java:145)
	at org.junit.jupiter.params.ParameterizedTestExtension.lambda$provideTestTemplateInvocationContexts$2(ParameterizedTestExtension.java:90)
	at org.junit.jupiter.params.ParameterizedTestExtension$$Lambda/0x00007427a8142bb0.apply(Unknown Source)
	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:273)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
```  

I was absolutely shocked to see that the  `CSVParser` implementation used by JUnit is really **pre-allocating** a `char` array to store the CSV values in it. See [1](https://github.com/uniVocity/univocity-parsers/blob/7e7d1b3c0a3dceaed4a8413875eb1500f2a028ec/src/main/java/com/univocity/parsers/common/input/DefaultCharAppender.java#L40).

This is completely unusable for CSV strings of unknown length. One could of course provide a value that would fit within the JVM limits (i.e. `Integer.MAX_VALUE - 8`), at which point this doesn't crash, but that just means your unit test is now allocating absolutely ridiculous amounts of heap memory to run.

Digging further, I found that the shaded univocity parsers library does actually have another implementation of `DefaultCharAppender` called `ExpandingCharAppender`, which seems to grow the `char` buffer at runtime, starting from a modest `8192` buffer length value.

The library is basing it's decision on which `Appender` to use in the `CsvParserSettings`, see [2](https://github.com/uniVocity/univocity-parsers/blob/7e7d1b3c0a3dceaed4a8413875eb1500f2a028ec/src/main/java/com/univocity/parsers/csv/CsvParserSettings.java#L92). Apparently, all that is required to switch to the `ExpandingCharAppender` is to pass a value of `-1` for the `maxCharsPerColumn`.

Unfortunately, the `maxCharsPerColumn` property of the `@CsvFileSource` annotation **requires** the value to be a positive integer:

```
org.junit.platform.commons.PreconditionViolationException: maxCharsPerColumn must be a positive number: -1
	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:273)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	Suppressed: org.junit.platform.commons.PreconditionViolationException: Configuration error: You must configure at least one set of arguments for this @ParameterizedTest
		at java.base/java.util.stream.AbstractPipeline.close(AbstractPipeline.java:323)
		at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:273)
		... 9 more
```

## Steps to reproduce

1. OutOfMemoryError when using large column length limits
```java
@ParameterizedTest
@CsvFileSource(resources = "/file.csv", numLinesToSkip = 1, maxCharsPerColumn = Integer.MAX_VALUE)
void dummy(String columnA, String columnB) {
}
```

2. PreconditionViolationException for trying to use an unbounded `ExpandingCharAppender` with `maxCharsPerColumn = -1`
```java
@ParameterizedTest
@CsvFileSource(resources = "/file.csv", numLinesToSkip = 1, maxCharsPerColumn = -1)
void dummy(String columnA, String columnB) {
}
```

## Context

 - Used versions (Jupiter/Vintage/Platform): JUnit 5.10.3
 - Build Tool/IDE: JDK 21

## TLDR

Please switch to the `ExpandingCharAppender` by default when using `@CsvFileSource`, or at least allow its usage by removing the positive integer validation of `maxCharsPerColumn` property, and document the valid range.

Alternatively, you may consider switching to a better CSV parser implementation altogether. This obscure "Univocity" library has last seen a commit in 2021 and its website [univocity.com](https://univocity.com/) returns an HTTP 404 error page.


Discussion / hints:
> Please switch to the `ExpandingCharAppender` by default when using `@CsvFileSource`, or at least allow its usage by removing the positive integer validation of `maxCharsPerColumn` property, and document the valid range.

We'll do that for now.

> Alternatively, you may consider switching to a better CSV parser implementation altogether. This obscure "Univocity" library has last seen a commit in 2021 and its website [univocity.com](https://univocity.com/) returns an HTTP 404 error page.

I'm hoping the fork mentioned in https://github.com/uniVocity/univocity-parsers/issues/534 will get some traction but we'll keep an eye on it.

Fix patch:
diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc
index f25bb175f4d9..862e81213d53 100644
--- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc
+++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0.adoc
@@ -186,6 +186,8 @@ on GitHub.
 * `@TempDir` now fails fast in case the annotated target is of type `File` and
   `TempDirFactory::createTempDirectory` returns a `Path` that does not belong to the
   default file system.
+* Allow potentially unlimited characters per column in `@CsvSource` and `@CsvFileSource`
+  by specifying `maxCharsPerColumn = -1`.
 
 
 [[release-notes-5.11.0-junit-vintage]]
diff --git a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvFileSource.java b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvFileSource.java
index 1798dfc171b3..77ad9245fc54 100644
--- a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvFileSource.java
+++ b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvFileSource.java
@@ -206,7 +206,8 @@
 	/**
 	 * The maximum number of characters allowed per CSV column.
 	 *
-	 * <p>Must be a positive number.
+	 * <p>Must be a positive number or {@code -1} to allow an unlimited number
+	 * of characters.
 	 *
 	 * <p>Defaults to {@code 4096}.
 	 *
diff --git a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvParserFactory.java b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvParserFactory.java
index 0efb81b3252b..d7ffee880cbc 100644
--- a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvParserFactory.java
+++ b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvParserFactory.java
@@ -77,8 +77,8 @@ private static CsvParserSettings createParserSettings(String delimiter, String l
 		settings.setAutoConfigurationEnabled(false);
 		settings.setIgnoreLeadingWhitespaces(ignoreLeadingAndTrailingWhitespace);
 		settings.setIgnoreTrailingWhitespaces(ignoreLeadingAndTrailingWhitespace);
-		Preconditions.condition(maxCharsPerColumn > 0,
-			() -> "maxCharsPerColumn must be a positive number: " + maxCharsPerColumn);
+		Preconditions.condition(maxCharsPerColumn > 0 || maxCharsPerColumn == -1,
+			() -> "maxCharsPerColumn must be a positive number or -1: " + maxCharsPerColumn);
 		settings.setMaxCharsPerColumn(maxCharsPerColumn);
 		// Do not use the built-in support for skipping rows/lines since it will
 		// throw an IllegalArgumentException if the file does not contain at least
diff --git a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvSource.java b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvSource.java
index ef09eea27ba6..6ee1c92e7c10 100644
--- a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvSource.java
+++ b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/CsvSource.java
@@ -261,7 +261,8 @@
 	/**
 	 * The maximum number of characters allowed per CSV column.
 	 *
-	 * <p>Must be a positive number.
+	 * <p>Must be a positive number or {@code -1} to allow an unlimited number
+	 * of characters.
 	 *
 	 * <p>Defaults to {@code 4096}.
 	 *


Test patch:
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/params/provider/CsvArgumentsProviderTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/params/provider/CsvArgumentsProviderTests.java
index 0ea40f9c70e9..4aec498e9eca 100644
--- a/jupiter-tests/src/test/java/org/junit/jupiter/params/provider/CsvArgumentsProviderTests.java
+++ b/jupiter-tests/src/test/java/org/junit/jupiter/params/provider/CsvArgumentsProviderTests.java
@@ -18,6 +18,7 @@
 import java.util.stream.Stream;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.PreconditionViolationException;
 
@@ -296,20 +297,21 @@ void throwsExceptionWhenSourceExceedsDefaultMaxCharsPerColumnConfig() {
 
 	@Test
 	void providesArgumentsForExceedsSourceWithCustomMaxCharsPerColumnConfig() {
-		var annotation = csvSource().lines("0".repeat(4097)).delimiter(';').maxCharsPerColumn(4097).build();
+		var annotation = csvSource().lines("0".repeat(4097)).maxCharsPerColumn(4097).build();
 
 		var arguments = provideArguments(annotation);
 
 		assertThat(arguments.toArray()).hasSize(1);
 	}
 
-	@Test
-	void throwsExceptionWhenMaxCharsPerColumnIsNotPositiveNumber() {
-		var annotation = csvSource().lines("41").delimiter(';').maxCharsPerColumn(-1).build();
+	@ParameterizedTest
+	@ValueSource(ints = { Integer.MIN_VALUE, -2, 0 })
+	void throwsExceptionWhenMaxCharsPerColumnIsNotPositiveNumberOrMinusOne(int maxCharsPerColumn) {
+		var annotation = csvSource().lines("41").maxCharsPerColumn(maxCharsPerColumn).build();
 
 		assertThatExceptionOfType(PreconditionViolationException.class)//
 				.isThrownBy(() -> provideArguments(annotation).findAny())//
-				.withMessageStartingWith("maxCharsPerColumn must be a positive number: -1");
+				.withMessageStartingWith("maxCharsPerColumn must be a positive number or -1: " + maxCharsPerColumn);
 	}
 
 	@Test
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/params/provider/CsvFileArgumentsProviderTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/params/provider/CsvFileArgumentsProviderTests.java
index 87e2f7e8aee7..f43ea0a2d01f 100644
--- a/jupiter-tests/src/test/java/org/junit/jupiter/params/provider/CsvFileArgumentsProviderTests.java
+++ b/jupiter-tests/src/test/java/org/junit/jupiter/params/provider/CsvFileArgumentsProviderTests.java
@@ -19,7 +19,6 @@
 import static org.mockito.Mockito.when;
 
 import java.io.ByteArrayInputStream;
-import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -30,6 +29,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvFileArgumentsProvider.InputStreamProvider;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.PreconditionViolationException;
@@ -410,7 +410,7 @@ void readsLineFromDefaultMaxCharsFileWithDefaultConfig(@TempDir Path tempDir) th
 	}
 
 	@Test
-	void readsLineFromExceedsMaxCharsFileWithCustomConfig(@TempDir Path tempDir) throws java.io.IOException {
+	void readsLineFromExceedsMaxCharsFileWithCustomExplicitConfig(@TempDir Path tempDir) throws Exception {
 		var csvFile = writeClasspathResourceToFile("exceeds-default-max-chars.csv",
 			tempDir.resolve("exceeds-default-max-chars.csv"));
 		var annotation = csvFileSource()//
@@ -426,24 +426,49 @@ void readsLineFromExceedsMaxCharsFileWithCustomConfig(@TempDir Path tempDir) thr
 	}
 
 	@Test
-	void throwsExceptionWhenMaxCharsPerColumnIsNotPositiveNumber(@TempDir Path tempDir) throws java.io.IOException {
+	void readsLineFromExceedsMaxCharsFileWithCustomUnlimitedConfig(@TempDir Path tempDir) throws Exception {
+		var csvFile = tempDir.resolve("test.csv");
+		try (var out = Files.newBufferedWriter(csvFile)) {
+			var chunks = 10;
+			var chunk = "a".repeat(8192);
+			for (long i = 0; i < chunks; i++) {
+				out.write(chunk);
+			}
+		}
+
+		var annotation = csvFileSource()//
+				.encoding("ISO-8859-1")//
+				.maxCharsPerColumn(-1)//
+				.files(csvFile.toAbsolutePath().toString())//
+				.build();
+
+		var arguments = provideArguments(new CsvFileArgumentsProvider(), annotation);
+
+		assertThat(arguments).hasSize(1);
+	}
+
+	@ParameterizedTest
+	@ValueSource(ints = { Integer.MIN_VALUE, -2, 0 })
+	void throwsExceptionWhenMaxCharsPerColumnIsNotPositiveNumberOrMinusOne(int maxCharsPerColumn, @TempDir Path tempDir)
+			throws Exception {
 		var csvFile = writeClasspathResourceToFile("exceeds-default-max-chars.csv",
 			tempDir.resolve("exceeds-default-max-chars.csv"));
 		var annotation = csvFileSource()//
 				.encoding("ISO-8859-1")//
 				.resources("exceeds-default-max-chars.csv")//
-				.maxCharsPerColumn(-1).files(csvFile.toAbsolutePath().toString())//
+				.maxCharsPerColumn(maxCharsPerColumn)//
+				.files(csvFile.toAbsolutePath().toString())//
 				.build();
 
 		var exception = assertThrows(PreconditionViolationException.class, //
 			() -> provideArguments(new CsvFileArgumentsProvider(), annotation).findAny());
 
 		assertThat(exception)//
-				.hasMessageStartingWith("maxCharsPerColumn must be a positive number: -1");
+				.hasMessageStartingWith("maxCharsPerColumn must be a positive number or -1: " + maxCharsPerColumn);
 	}
 
 	@Test
-	void throwsExceptionForExceedsMaxCharsFileWithDefaultConfig(@TempDir Path tempDir) throws java.io.IOException {
+	void throwsExceptionForExceedsMaxCharsFileWithDefaultConfig(@TempDir Path tempDir) throws Exception {
 		var csvFile = writeClasspathResourceToFile("exceeds-default-max-chars.csv",
 			tempDir.resolve("exceeds-default-max-chars.csv"));
 		var annotation = csvFileSource()//
@@ -461,7 +486,7 @@ void throwsExceptionForExceedsMaxCharsFileWithDefaultConfig(@TempDir Path tempDi
 	}
 
 	@Test
-	void ignoresLeadingAndTrailingSpaces(@TempDir Path tempDir) throws IOException {
+	void ignoresLeadingAndTrailingSpaces(@TempDir Path tempDir) throws Exception {
 		var csvFile = writeClasspathResourceToFile("leading-trailing-spaces.csv",
 			tempDir.resolve("leading-trailing-spaces.csv"));
 		var annotation = csvFileSource()//
@@ -477,7 +502,7 @@ void ignoresLeadingAndTrailingSpaces(@TempDir Path tempDir) throws IOException {
 	}
 
 	@Test
-	void trimsLeadingAndTrailingSpaces(@TempDir Path tempDir) throws IOException {
+	void trimsLeadingAndTrailingSpaces(@TempDir Path tempDir) throws Exception {
 		var csvFile = writeClasspathResourceToFile("leading-trailing-spaces.csv",
 			tempDir.resolve("leading-trailing-spaces.csv"));
 		var annotation = csvFileSource()//
@@ -527,7 +552,7 @@ private static <T> T[] array(T... elements) {
 		return elements;
 	}
 
-	private static Path writeClasspathResourceToFile(String name, Path target) throws IOException {
+	private static Path writeClasspathResourceToFile(String name, Path target) throws Exception {
 		try (var in = CsvFileArgumentsProviderTests.class.getResourceAsStream(name)) {
 			Files.copy(in, target);
 		}