Repository navigation
360 design review - #1048
Open
jeffjensen wants to merge 175 commits into
Open
360 design review#1048jeffjensen wants to merge 175 commits into
jeffjensen wants to merge 175 commits into
Conversation
…t untouched * Install the executor's operation listener only on a tester that overrides IDatabaseTester.getOperationListener(), so the listener it carries can be wrapped and put back after the test. * Leave the listener of a tester that does not override it exactly as configured, instead of replacing it with a closing one that stays installed for good and discards, for example, the no-op listener marking its connections as managed elsewhere. * Capture the row count check baseline eagerly for such a tester, as there is no executor listener to piggyback on. * Document in the IOperationListener and annotations pages, the latter's properties section included, that the @DbUnitProperty values then reach only the connection the extension resolves itself, not one the tester's own onSetup() retrieves. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q55xijb8HCBhpVuuV644ms
* Discard the baseline in RowCountChecker.verify(), whether the verification passes or fails, so a later lifecycle on the same instance that never captures a baseline of its own, such as the legacy setUp() and tearDown() hooks, does not verify against the counts of the test before it. * Discard the baseline in DefaultPrepAndExpectedTestCase when its database tester is replaced, as it was captured from the replaced tester's database. Refs: 939 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
* Clear the involved data sources in runTest() before tearing them down after failing test steps, as postTest() does, so a postTest(false) call in the caller's finally block does not tear each delegate down a second time. * Document that the run is over once postTest() has run, or runTest() has torn it down after failing test steps. Refs: 968 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…d not be read * Keep the connection for the rest of the test when it had no JDBC connection to capture at acquisition, as a DatabaseDataSourceConnection does when its pool times out, instead of replacing it on every getConnection() call and leaving each replaced connection unclosed. * Replace a kept connection only when its JDBC connection reports closed, or fails to report; release() closes the one it holds. * Drop the sentence of the release() Javadoc saying it leaves the memo in place and relies on a revalidation flag that does not exist: release() forgets a connection it closed and keeps it only when ownership leaves it open for its real owner. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q55xijb8HCBhpVuuV644ms
…ter the test * Report from the tester resolution whether it gave an injected @DbUnitTestCase instance the tester because the instance carried none. * Give such an instance no tester again once the whole test is over, in the new AnnotatedTestExecutor.unbindTesterFromInjectedTestCase(). The JUnit Jupiter extension calls it from its afterEach callback, which runs after the test's @AfterEach methods, and the Spring listener after the after-test steps; both call it also when the step before it fails. A test case shared across tests, such as a static field, is then given each test's own tester instead of keeping the first test's, and an @AfterEach method can still reach the tester through the injected instance. * Hold the instance to the closeConnectionAfterTest = false a test declared while the tester is taken off it, and put the instance's own value back after, so the instance does not close a connection that belongs to its real owner, such as a CachingConnectionProvider. When the test declares nothing, the instance's own value decides. * Build the executor from the resolution in the JUnit Jupiter and Spring bindings. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q55xijb8HCBhpVuuV644ms
…ring case * Index a catalog's definitions by the table name ignoring case, as dbUnit finds a table in a dataset, so verifyTables = "account" selects a catalog's ACCOUNT definition, which is returned under its own spelling. * Reject a table defined twice in catalog classes, named twice in verifyTables, or declared twice in verify, when the names differ only in case. * List the available catalog table names as the catalog spells them. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ig property * Resolve every @DbUnitProperty name, and every name a DatabaseConfigPropertiesProvider supplies, to the long name DatabaseConfig knows the property by, and reject a property that is declared twice under its long and its short form instead of leaving the value that wins to the iteration order of a Properties. * Replace an injected test case's own property with a declared one for the same DatabaseConfig property, whichever form of the name each uses. * Resolve a long or short name with the new DatabaseConfig.findByNameOrShortName(), which setPropertiesByString() and DatabaseConfigOverrides use too, so every place reads a property name the same way. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lared verification * Add PrepAndExpectedTestCase.configureTestVerifyingEveryExpectedTable(), a default method reporting that it configured nothing, which DefaultPrepAndExpectedTestCase implements by taking one default VerifyTableDefinition per table of the expected dataset it loads, instead of the annotation runtime parsing every expected file a first time just to list its tables. * Use it from the prep/expected lifecycle when @DbUnitExpected declares no tables to verify; a subclass that overrides configureTest(VerifyTableDefinition[], String[], String[]), which may expect the definitions it is given, and any other implementation keep deriving the definitions by reading the files. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ssionValueComparer * Hold the compiled form of each expression the comparer is given in a small, bounded cache that drops the expression used least recently, so a column holding the same expression in every row does not compile it for every cell compared. * Make the cache safe to share between threads, as the comparer is when it is the shared ValueComparers constant or an instance an annotation names. Refs: 973 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…rs in one place * Add ConnectionPreservingOperationListener.unwrap(), returning the innermost delegate. * Use it in unwrapsToNoOp() and DefaultPrepAndExpectedTestCase, which each walked down the layers their own way, a recursion and a loop. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…n check and both bindings * Add TesterFields, holding which fields count as tester fields, marked or unmarked, and the walk through a class hierarchy for them. * Use it in AnnotatedTestOptIn, TestInstanceTesterResolver and SpringTesterResolver, which each carried their own copy of the marker predicates and the hierarchy scan, so a change to a field rule reaches the opt-in check and both resolutions together. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
* Add CommaSeparatedValues.split(), the split on commas and trim of each value that DatabaseConfig applied to every String[] property. * Use it in DatabaseConfig and in RowCountCheckConfiguration for the exclude tables system property, which repeated that parsing, so the system property form and the DatabaseConfig form of the same list cannot drift apart. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…t only forward * Call columnsNamedIn() directly where the expected table's columns are made from the actual table's column definitions, and drop the two private overloads that did nothing but pass their arguments on to it. * Keep what the overloads documented on columnsNamedIn(). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
* Remove the blank line after the class's opening brace and the one that detached the logger Javadoc from its field, and end that Javadoc with a period. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ted outcome * Rename the tests that had only a method and an outcome, such as testConnectionRetrieved_runsDelegateThenPropertiesThenHook, to test<MethodName>_<StartingStateConditions>_<AssertedOutcome>. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… 5.13.3 API * Take the classes enclosing a @nested test class from the parent extension contexts in the new EnclosingTestClasses, and search each of them with the two argument AnnotationSupport.findAnnotation(), closest class first, instead of calling ExtensionContext.getEnclosingTestClasses() and the findAnnotation() overload that takes the enclosing classes. Both exist only since JUnit Jupiter 5.13.3 and ran for every test, so the extension failed with a NoSuchMethodError on an older Jupiter even for a test that uses none of the annotations. * The newest Jupiter API the extension still calls is ExtensionContext.getTestInstances(), added in 5.7, so state in DbUnitExtension.adoc and changes.xml that the extension needs Jupiter 5.7 or later. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… @DbUnitExpected path * Apply the setup operation on the prep/expected path only when @DbUnitPrep files or @DbUnitSetup are declared, and the teardown operation only when @DbUnitTearDown is, as the setup/teardown path already does, instead of always setting CLEAN_INSERT and NONE. A setup or teardown operation set on the tester in a @beforeeach method, including one DbUnitOperation cannot express such as a CompositeOperation, now stays in force. Before, the extension replaced it after @beforeeach ran, so a DELETE_ALL teardown became NONE and the prep rows stayed in the database. * Share the setup rule of both paths through AnnotatedTestConfiguration.replacesSetUpOperation(). * Document in annotations.adoc, the Javadoc and changes.xml that both paths replace an operation only when an annotation declares it. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ction rolls back first * Correct the TestScopedConnection.release() Javadoc: JDBC leaves the outcome of closing a connection with an open transaction to the driver, Derby and DB2 refuse to close it, and Oracle's driver commits it. Rolling back before the close stays, so on Oracle the never-committed prep and teardown writes of an autocommit-off connection are now discarded where 3.5.x committed them. * State that change in the changes.xml action, in the Connections page, in the row count check caveats and in the @DbUnitConfig closeConnectionAfterTest description. * Comment the existing rollback-before-close test with the reason for the order. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ut under -Ddbunit.rowCountCheck=true * Resolve the enabled flag a test declares so that -Ddbunit.rowCountCheck=false still disables the check, while a true or bare -Ddbunit.rowCountCheck no longer enables a test that declares enabled = false, such as one that legitimately leaves rows behind. A CI run forcing the check on used to fail exactly the methods the annotation exists to exempt. Without a declared value the system property still wins over the DatabaseConfig feature in either direction. * Add the patterns of the dbunit.rowCountCheckExcludeTables system property to the ones the annotation or DatabaseConfig supply instead of replacing them, so a suite-wide list does not re-enable the check for the tables a test excluded. * Document the new precedence in the DbUnitRowCountCheck and RowCountCheckConfiguration Javadoc, the row count check, annotations and properties pages, and changes.xml. Refs: 945 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…'s case sensitivity on every route * Finish combining a multi-file @DbUnitPrep dataset when a tester whose onSetup() retrieves its own connection reports it to the executor's listener, with the table name case sensitivity of that connection, instead of always combining case-insensitively on that route. The tester reads its dataset only after it has notified the listener, and the dataset combined case-insensitively beforehand stays in place for a tester that never does. Before, whether two tables differing only in case merged depended on how the row count check baseline got its connection, so adding a Connection parameter to a @beforeeach method changed the data a test ran on. The tester still resolves exactly one connection. * Document in annotations.adoc and changes.xml how several @DbUnitPrep files combine. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…hared for the whole run * Say in the DbUnitColumnComparer, DbUnitVerifyTable, VerifyTableDefinitionsProvider and VerifyTableDefinitionCatalog Javadoc, in annotations.adoc and in changes.xml that a comparer class an annotation names is instantiated once and a catalog once per combination of catalog classes, both shared by every test naming them. A comparer must be stateless, and a test must never change the VerifyTableDefinition objects of a catalog, their setters included, or the result of a later test depends on the order the tests ran in. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e connection onSetup() opens * Wrap the listener of a tester that reports it on the 3.5.0-style path too, for the test only, so the row count baseline can be taken from the connection onSetup() retrieves instead of one resolved before it. The wrapper forwards every callback to the tester's own listener, closes included, shields no connection and makes no warning, and the listener is put back after the test. Before, such a test opened a connection of its own only to read whether the check is on, which is off by default, so a plain JdbcDatabaseTester used three connections per test instead of the two of the 3.5.0 lifecycle, and a bounded pool sized for that lifecycle could run out. * Take over the connection onSetup() retrieves only for the baseline and leave it to the tester's listener afterward, as the 3.5.0 lifecycle did, so a listener that keeps it open, such as a custom non-closing listener on a DefaultDatabaseTester over one fixed connection, keeps it for the next test. Close after the test only a connection the extension resolved itself, under the closeConnectionAfterTest and NO_OP rules, through the new TestScopedConnection.releaseUnlessAdopted(). * Decide whether the baseline can come from that connection by whether the listener the executor installed is still on the tester when the setup is about to run, directly or beneath ConnectionPreservingOperationListener layers. A @beforeeach method given the tester as a parameter may set its own listener, such as the no-op one; the baseline is then taken before onSetup(), from a connection the executor resolves itself. The connection is also resolved first for a NONE setup operation, a tester that overrides onSetup() and one that cannot report its listener. * Cover it with a pool of one connection in DbUnitExtensionBoundedPoolIT and with DbUnitExtensionClassicPathIT, and update the DbUnitExtension, IOperationListener and changes.xml descriptions of the path. Refs: 939 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q55xijb8HCBhpVuuV644ms
… a connection to its listener * Assert that onSetup() and onTearDown() retrieve the connection, tell the listener, read the dataset, run the operation and report the operation finished, in that order, and that an operation of NONE retrieves no connection and notifies nobody. The annotation runtime takes the row count check baseline from the connection the listener is told about, and finishes combining several @DbUnitPrep files there, relying on a contract that only the executeOperation() Javadoc stated before. * Say in that Javadoc that the dataset is read after the listener is told, and name the test that pins the order. Refs: 939 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
* Summarize the 3.6.0-SNAPSHOT release in one sentence naming its themes - annotation-driven test configuration for JUnit and Spring, a row count check for incomplete teardown, multi-data-source testing, and the DefaultPrepAndExpectedTestCase fixes - instead of listing nearly every entry, as the description of a release is meant to be a short summary and each entry carries its own detail. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… with an Error * Catch Throwable instead of Exception in DefaultPrepAndExpectedTestCase.cleanupData(), so an Error thrown by the tear down operation or the row count check, such as an AssertionError from the tester's listener in connectionRetrieved() or a LinkageError from a data type, releases the shared connection and puts back the databaseConfigProperties values the way an Exception does, and propagates unchanged. Before, the connection stayed memoized: with closeConnectionAfterTest set to true it was never closed, and a connection that is kept retained the @DbUnitProperty values of the test that failed. * Test it with a tester whose listener throws an AssertionError from connectionRetrieved(). * Add the changes.xml entry. Refs: 1015 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…aset and close its file * Produce an empty dataset from an empty or comment-only YAML document, which SnakeYAML reads as null, instead of failing with a NullPointerException. Throw a DataSetException saying the root must be a mapping of table names to rows when it is a list or a scalar, instead of a ClassCastException. * Throw a DataSetException naming the table and row for a table that is not a list of rows, a row that is not a mapping of column names to values, and a table or column name YAML reads as a number or a boolean, instead of a ClassCastException, and read a list entry with no value as a row of nulls, as JsonProducer reads a null row, instead of failing with a NullPointerException. * Throw a DataSetException for a document SnakeYAML cannot parse, such as one with a syntax error or a second document, so the data file loaders name the file, and report a column named twice in a row with its line and column instead of as an ambiguous table name. * Call endDataSet() on the consumer at the end of produce(), as JsonProducer does, so a consumer that writes in endDataSet(), such as FlatXmlWriter, is no longer left unfinished. * Close the stream the File constructor opens once the dataset is produced, also when producing fails, and leave a stream the caller passes in open. The package-private constructor YamlProducer(InputStream, boolean) carries that choice. * Test it in the new YamlProducerTest, document which documents are accepted in yaml.adoc and add the changes.xml entry. Reported by the code review of main 085c74d..feb1dab as finding F13. Refs: 1016 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rvingOperationListener * Take a tester listener with a blanket ConnectionPreservingOperationListener layer, which keeps every connection from a close, as the sign that the tester's connections are managed elsewhere, as NO_OP_OPERATION_LISTENER already is. DefaultPrepAndExpectedTestCase and the annotation runtime used to close their shared connection after the test under such a listener, although the listener's own documentation says the caller owns every connection. * Add ConnectionPreservingOperationListener.leavesEveryConnectionOpen() and decide ConnectionOwnership, and so the left-open connection warning, with it. * Document the rule in the IOperationListener and annotations pages and changes.xml. Reported by the code review of 360-finalization 085c74d..00775a6; no GitHub issue was filed for it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…le name parts * Make DatabaseConfig.findByShortName() match the whole last part of a property's name, or several whole last parts such as mssql/identityColumnFilter, instead of any ending of it. A name that is only the end of a short name, such as Size, resolved to the first property ending with it, batchSize, so setPropertiesByString(), @DbUnitProperty and DefaultPrepAndExpectedTestCase.setDatabaseConfigProperties() applied it there, and the @DbUnitProperty check that rejects an unknown name let it through. * Document the short name rule in annotations.adoc and add the changes.xml entry. Reported by the code review of 360-finalization 085c74d..00775a6; no GitHub issue was filed for it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…p acquired in one place * Run configureTest(), configureTestVerifyingEveryExpectedTable() and the row count check baseline capture of DefaultPrepAndExpectedTestCase through one helper that releases the shared connection when the step fails and is what acquired it, instead of a copy of that logic in each and in lookupFeatureValue(), whose callers already release it. * The baseline capture now releases the connection for an Error too, such as an AssertionError from a custom RowCounter, as configureTest() already did. Reported by the code review of 360-finalization 085c74d..00775a6; no GitHub issue was filed for it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…DatabaseTester AbstractDatabaseTester.getOperationListener() returned null until the first operation created the default listener, which is also what the interface default returns for a tester that cannot report its listener. Telling the two apart took a reflective check of whether the method is overridden. * A tester starts with a DefaultOperationListener, and setOperationListener(null) sets a new one, so getOperationListener() never returns null. * AnnotatedTestExecutor wraps a tester's listener when the tester reports one, instead of checking by reflection whether the method is overridden. * DefaultPrepAndExpectedTestCase treats a plain DefaultOperationListener as no listener of the tester's own, as it did when the tester reported none. Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNHLHaY2MVKWrdKZeRDWE1
…r threads * Fill the cache of DecimalFormat instances of XlsTable with computeIfAbsent on a ConcurrentHashMap, and format under the lock of the shared instance, as DecimalFormat is not thread-safe. Refs: 1037 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJVLaRzH3ZHSyCvt8v9
* Read the text true or false, in any case, as 1 or 0 in NumberDataType, as the integer types do, so the NUMBER(1) boolean column of Oracle takes it from every dataset format. * Say so in the Dataset Values page. Refs: 1038 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJVLaRzH3ZHSyCvt8v9
…nal to dbUnit * Add one sentence to the class Javadoc of the public types that only the JUnit Jupiter and Spring bindings call across packages, saying they are internal and may change in a minor release. * Add the same statement to the Annotations page. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJVLaRzH3ZHSyCvt8v9
…ted connection only * State on the DbUnitExtension page that the @DbUnitProperty values and the other @DbUnitConfig settings are in force on an injected IDatabaseConnection, and that a connection taken from an injected IDatabaseTester has none of them and, on a tester that opens a connection per call, is the test's to close. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJVLaRzH3ZHSyCvt8v9
…annotations * Add a section to the Annotations page and a note to the DbUnitExtension and DbUnitTestExecutionListener pages: JUnit Jupiter runs the lifecycle callbacks once for a @testfactory method, so dbUnit sets it up before its first dynamic test and verifies it after its last, the dynamic tests share one dataset, a failed dynamic test does not skip the verification, and a @ParameterizedTest or @RepeatedTest gives a setup per case. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJVLaRzH3ZHSyCvt8v9
…with the schema * Build each table name of the row counter sketch with QualifiedTableName, the connection's schema and its escape pattern, as QueryPerTableRowCounter does, so the sketch counts a table in the schema it was listed in as the paragraph above it says. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJVLaRzH3ZHSyCvt8v9
* Start the message of the unsupported extension error of FileExtensionDataFileLoader with the file, as the full XML rejection does, so a test with several dataset files shows which one it cannot load. * Assert in the unsupported extension and no extension tests that the message names the file. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJVLaRzH3ZHSyCvt8v9
…f @DbUnitVerifyTable * Describe on the Annotations page what sortOnFilteredColumnsOnly sorts by, when to set it, its default, and link to Sort Mode, as the hint of a failed verification names this attribute. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJVLaRzH3ZHSyCvt8v9
… both datasets * State on the annotations page and in the @DbUnitTearDownOperation Javadoc that the teardown operation runs over the rows of both datasets. * Say that an inserting operation leaves those rows in the table, and that CLEAN_INSERT and INSERT fail on a duplicate key when the datasets share a row. * Recommend DELETE_ALL or DELETE to clean up. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJW6ba8Bh8ijVyxu7Wr
* Add RequiredLibrary.POI, so reading or writing an Excel dataset, or loading an .xls or .xlsx file through a data file loader, on a classpath without Apache POI fails with a DataSetException that names org.apache.poi:poi and org.apache.poi:poi-ooxml instead of a bare NoClassDefFoundError. * Move the POI code of XlsDataSet into a nested class so that loading XlsDataSet itself no longer needs POI and its constructors and write() can say what is missing. * Word the message so it reads correctly for a library with two artifacts. * Show poi-ooxml and the current POI version on the Excel page and say what a missing library looks like. Refs: 999 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJW6ba8Bh8ijVyxu7Wr
… JdbcDatabaseTester * Build the JdbcDatabaseTester in a constructor that declares ClassNotFoundException in the Complete Example and the other IDatabaseTester example, in the migration page example, and in the JdbcBasedDBTestCase composition sketch, as a field initializer cannot throw a checked exception. * The examples now match the constructor form the annotation, extension and PrepAndExpectedTestCase pages already use. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJW6ba8Bh8ijVyxu7Wr
* Say on the row count check page that the failure is an UnexpectedRowCountException, which a build tool reports as a test error and not a test failure, because it points at the test's setup or teardown and not at a wrong result. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJW6ba8Bh8ijVyxu7Wr
* Describe CLEAN_INSERT as delete all + insert, which is what it runs, instead of truncate + insert, on the home page, in the five-minute tutorial and in CLAUDE.md. * TRUNCATE_TABLE remains its own operation. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJW6ba8Bh8ijVyxu7Wr
* Lift SnakeYAML's default limit of 3,145,728 code points on a document in YamlProducer, as a dataset is a fixture of the test's own project and a document that YamlDataSet.write() wrote must read back. * Leave the limit alone on a SnakeYAML older than 1.32, which has none. * Say on the YAML page that a document is read whatever its size and that SnakeYAML's limit of 50 aliases stays. Refs: 1041 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJW6ba8Bh8ijVyxu7Wr
* Write a cell value of a type other than a string, number, boolean, array, collection or map, such as a UUID, as its toString() text in YamlWriter, instead of the global tag that the reader refuses or the empty mapping that loses the value. * Keep an array, a collection and a map as a sequence or mapping, which the reader rejects naming the table, the row and the column. * Say on the YAML page how a value of another type is written. Refs: 1042 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJW6ba8Bh8ijVyxu7Wr
…a YAML dataset YamlWriter used the non-printable style that SnakeYAML applies by default, which writes a string that holds a control character as a !!binary value. YamlProducer reads such a value as a byte array, so a text column got the Base64 text of the value in place of the value, without an error. * Set the non-printable style of the dumper to ESCAPE, so that text with a control character, such as a form feed, a NUL, an escape character, DEL or a C1 control, is written as a double-quoted value with YAML escapes and reads back as the same text, in the block and in the flow style. * Keep writing a byte array as a !!binary value, as it is not text. * State on the YAML page that a file written before 3.6.0 holds such text as a !!binary value and needs to be written again. Refs: 1043 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJUQnAcLYKL4kJpY1Bh
… page The page opened with the sentence that dbUnit does not use system properties since version 2.0, while the same page documents the dbunit.rowCountCheck and dbunit.rowCountCheckExcludeTables system properties, and PropertiesBasedJdbcDatabaseTester reads five dbunit.* system properties for its connection settings. A reader looking for a way to switch the row count check from the build command line was told that there is none. * Say that dbUnit reads its configuration from DatabaseConfig, with the three exceptions, and name the system properties of each. * Correct "DbUnit use" to "DbUnit uses" in the next sentence. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJUQnAcLYKL4kJpY1Bh
JsonProducer and IsActualEqualToExpectedJsonValueComparer built their ObjectMapper with the defaults of Jackson 2.15 and later, which reject a string value longer than 20,000,000 characters. JsonWriter writes a byte array as Base64, so a BLOB of about 15 MB made a document that JsonDataSet.write() wrote but JsonDataSet could not read. * Build the mapper of JsonProducer and of the comparer from a JsonFactory whose read constraints allow a string of any length, as YamlProducer does for the size of a YAML document. The limits on the depth of nesting and the length of a number stay. * Keep the calls that name the Jackson 2.15 classes in a nested class and fall back to the default mapper when they are missing, as a Jackson older than 2.15 has no limit to lift. * Say on the JSON and value comparer pages, and in the Javadoc of the producer, the dataset and the comparer, that a string value is read whatever its length. Refs: 1044 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJUQnAcLYKL4kJpY1Bh
JsonWriter handed a cell value of a type other than a string, a number, a boolean or a byte array to Jackson, which writes a value it has no serializer for as a JSON object of its getter properties. Such a value reads back as the JSON text of that object, not as the value, and a value without properties failed the write. A query table keeps a column of a type that the data type factory does not recognize, such as an H2 INTERVAL, and its value is the driver's own object. * Write a value of any other type than a string, a number, a boolean, an array, a collection or a map as its toString() text, as YamlWriter does. An array, a collection and a map stay a JSON array and object, which a json or jsonb column takes. * Describe the rule on the JSON page and in the Javadoc of the writer and the dataset, and say that a document written before 3.6.0 needs writing again. Refs: 1045 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJUQnAcLYKL4kJpY1Bh
* Say that an XML <none/> element, or a value missing at the end of a row, gives the column no value: an INSERT, CLEAN_INSERT included, leaves the column out, so it gets the column's default, while an UPDATE sets it to null, and an expected <none/> compares as a null. Only <null/> is a null. * Say that a CSV null is read as a null whether it is quoted or not, as the reader drops the quotes before it compares, so a CSV dataset cannot hold the text null, and name a ReplacementDataSet placeholder as the way to store it. * Say that a blank or absent Excel cell is a null, and a text cell that holds an empty string is the empty string. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJUG5XkkcguFufnRJA5
…t a table * Rethrow the SQLException of a counter that fails with a message that names the exclusion and the escapePattern property, and the original as its cause * Say on the row count page that a table whose stored name needs quoting, such as Flyway's flyway_schema_history on H2, cannot be counted by its unquoted name Refs: 939 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJTYEbkyG9NzTHv5JsE
…Config applies it to each connection * Limit the row count caveat about a fresh, default DatabaseConfig to a JdbcDatabaseTester and a DataSourceDatabaseTester built without a DatabaseConfig * Name the DatabaseConfig the four-argument constructor takes as the place to set the row count feature Refs: 939 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJTYEbkyG9NzTHv5JsE
* Fail a test with an IllegalStateException that names the method and the annotation when a lifecycle method of its class, a superclass, an interface or an enclosing class carries a method-level dbUnit annotation, which no binding reads there * Say on the annotation Javadoc and the annotations, DbUnitExtension and DbUnitTestExecutionListener pages that a method-level annotation counts on the test method only Refs: 753 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJTYEbkyG9NzTHv5JsE
…bers exactly * Fail the comparison with a DatabaseUnitException that names the column and the row when the expected or the actual value has content after its first JSON value, which Jackson ignores by default * Compare a number with a fraction as an exact decimal, as a double drops the digits past about 17 significant ones and made values that differ there equal * Say on the Value Comparers page and in the Javadoc what counts as equal: member order and whitespace do not matter, array order does, 1 and 1.0 differ, a null member differs from a missing one Refs: 1046 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJXVAfxC6BK1iNga1R4
* Fail with an SQLException that names the table and gives the database's message when QueryPerTableRowCounter cannot count a table, with the database's exception as the cause and its SQL state and error code kept, as a driver's own message may not name the table * Say in the RowCounter contract and on the row count page that a failure to count a table must name the table, as the row count check gives the message to the user Refs: 939 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJXVAfxC6BK1iNga1R4
* Describe in the Javadoc and on the Value Comparers page that the actual value is matched as the text dbUnit makes of it * Name the forms that differ from what the database shows: a timestamp's fraction, a decimal's scale and a binary value's Base64 text * Pin the rule with unit tests Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJWqtZkgmLqhKJCqaGD
…e comparer * Read a byte[] actual value as UTF-8 text before parsing, as RFC 8259 makes UTF-8 the encoding of JSON exchanged between systems * Before, the comparer parsed the Base64 text that DataType.asString() makes of a byte[], which H2 returns for a JSON column, so it failed for every document Refs: 1047 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJWqtZkgmLqhKJCqaGD
* Map the JSON type name to the new H2JsonType, which reads the value as the text of the document and writes it as the UTF-8 bytes of the text, as H2 stores a value bound with setString() as a JSON string * Before, H2DataTypeFactory gave the column DataType.UNKNOWN, so the database dataset dropped it with a warning and a dataset could neither load nor verify it * Document the type on the H2 page and keep H2 in the list of the JSON value comparer's databases Refs: 1047 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJWqtZkgmLqhKJCqaGD
… column of the table * Fail the verification of a table when a column name to include matches no column of the database table, instead of comparing only the row count * Fail it too when the column name of a column comparer matches no column, as its comparer would not be used * Name the column, the table and the table's columns in the message * Skip the check when the table's columns cannot be read or a subclass loads the actual table itself * Say so in the Javadoc and on the Annotations, Verify Table Definition and PrepAndExpectedTestCase pages Refs: 1049 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EiskJTJQQHut5U1jXdN7kp
jeffjensen
force-pushed
the
360-design-review
branch
from
November 12, 2026 03:08
d3be007 to
e621b11
Compare
This branch has not been deployed
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.
No description provided.