Add unit tests and fix three bugs in akit core#256
Open
bguthro wants to merge 10 commits intoMechanical-Advantage:mainfrom
Open
Add unit tests and fix three bugs in akit core#256bguthro wants to merge 10 commits intoMechanical-Advantage:mainfrom
bguthro wants to merge 10 commits intoMechanical-Advantage:mainfrom
Conversation
Add comprehensive JUnit 5 tests for LogTable, LogFileUtil, ReceiverThread,
AutoLogOutputManager, and LoggedMechanism2d, targeting >90% meaningful coverage
focused on identifying real defects rather than pure line coverage.
Bugs found and fixed:
- LogFileUtil: findReplayLogAdvantageScope() threw NoSuchElementException on an
empty AdvantageScope temp file; guard with hasNextLine() before nextLine()
- LogTable: all array get() overloads (byte[], boolean[], long[], float[],
double[], String[]) returned internal references, allowing callers to mutate
stored log data; return defensive .clone() copies instead
- ReceiverThread: InterruptedException thrown by a receiver's putTable() was
caught by the outer queue.take() handler, causing the run loop to exit and
starving subsequent receivers for that cycle and all future cycles; wrap each
putTable() call in a per-receiver try/catch and re-interrupt the thread so
clean shutdown still occurs after the current cycle completes
Known issues documented in tests (not yet fixed):
- AutoLogOutputManager uses hashCode() not identity for deduplication, causing
silent data loss when two distinct objects share the same hash code
- LogFileUtil.addPathSuffix("", suffix) incorrectly appends "_2" due to
String.endsWith("") always returning true
…dMechanism2d Add 67 additional tests targeting uncovered branches: - LogTableDataTest: float/double with unit metadata, 2D int/float arrays, null guards for all 1D/2D array types, direct LogValue put, and missing-key default returns for every scalar/array type - AutoLogOutputManagerTest: all 15 primitive+array+enum field type branches in registerField(), void-return method skipping, checked-exception method skipping, custom key annotation, key interpolation, and null unannotated field safety - LoggedMechanism2dEdgeCaseTest: logOutput() serialization verification, root getRoot() idempotency, setBackgroundColor(), all Ligament getter/setter overloads (Rotation2d, Angle, Distance), getColor() round-trip, setPosition(), duplicate child name exception, and three-segment chain pose count
…AutoLogOutputManager - LogTableDataTest: add HAL init and four type-mismatch write tests (boolean/double, string/long, long/boolean, double[]/string) that require DriverStation.reportWarning() - LoggerTest: new ordered test class covering pre-start API (recordMetadata, setReplaySource, processInputs no-op, runEveryN, registerURCL, AdvancedHooks) and a full lifecycle test (start → recordOutput of every scalar/array/lambda type → processInputs → verify via reflection → end); uses HAL so DriverStation APIs work - AutoLogOutputManagerTest: add HAL init; add test objects and tests for supplier types (BooleanSupplier/IntSupplier/LongSupplier/DoubleSupplier), Color and LoggedMechanism2d fields, 2D array fields of all primitive/string/enum types, forceSerializable path, Java record types (Record/Record[]/Record[][]), WPI/Struct fallback paths, superclass field lookup in key interpolation, and float/double with unit annotation; AutoLogOutputManager coverage now 90%
…types, mechanism tests - LogTableDataTest: add struct (explicit Struct<T>), WPISerializable, StructSerializable[], StructSerializable[][] round-trips using Translation2d; add record single/array/2D round-trips; add protobuf round-trip using Translation2d.proto; add LogValue equals/hashCode/getWPILOGType/ getNT4Type tests; add type-mismatch default-return tests for all LogValue array accessors; add LoggableType fromWPILOGType/fromNT4Type round-trip tests including json/unknown cases; add enum 2D array tests; add toString tests covering all scalar and array types; LogTable coverage: 51% → 91%, LogTable.LogValue: 72% → 96%, LoggableType: 77% → 100% - LoggerTest: add missing recordOutput overloads to both no-op (pre-start) and lifecycle tests: float[][], String[][], Color, Enum/Enum[]/Enum[][], Record/Record[]/Record[][], WPISerializable, Struct/Struct[]/Struct[][], Measure, float-with-unit, double-with-unit; Logger coverage: 43% → 55% - LoggedMechanism2dEdgeCaseTest: add Distance-based Mechanism2d/Ligament2d constructors; add close() tests; add LoggedMechanismRoot2d(String, Distance, Distance) direct constructor test; add getName() test; add getRoot duplicate-name returns-existing test; add append duplicate-name throws test; add setBackgroundColor no-pub branch test; LoggedMechanismRoot2d coverage: 67% → 71% - LoggerPreStartTest: new test class covering pre-start Logger API without HAL - LoggedNetworkInputTest: new test class for LoggedNetworkInput.removeSlash() utility
- LogTableDataTest: add AllTypesRecord (bool/short/int/long/float/double/enum) and NestedStructRecord (double + Translation2d) round-trip tests to exercise all RecordStruct field-type lambdas; RecordStruct coverage: 32% → 87%, LogDataReceiver: 50% → 100% - LogTableDataTest: add LogValue.equals() tests for all primitive types, type-mismatch, and non-LogValue objects; add hashCode consistency test; add getWPILOGType/getNT4Type for primitive and custom-type (struct) values; add type-mismatch default-return tests for all array accessors; add LoggableType fromWPILOGType/fromNT4Type round-trip including json/unknown; LogValue: 72% → 96%, LoggableType: 77% → 100% - ReceiverThreadTest: add DefaultLifecycleReceiver (uses default start/end) and a test that exercises the no-op default interface methods; LogDataReceiver default end() now covered
- LoggerTest: add setConsoleSource test (Logger.AdvancedHooks: 29% → 47%); add addDataReceiver pre-start test; add getTimestamp pre-start test (covers !running branch); add recordOutput(float/double, String) overloads to both no-op and lifecycle tests; add LoggedMechanism2d output to both no-op and lifecycle tests; Logger: 55% → 58%, Logger.AdvancedHooks: 29% → 47% - ReceiverThreadTest: add DefaultLifecycleReceiver that uses interface default start/end; add test to exercise default no-op implementations; LogDataReceiver: 50% → 100%
…h and findReplayLogUser() - Add findReplayLog() test via AdvantageScope path using @tempdir (skips if env var is set) - Add findReplayLogUser() test using System.setIn to inject mock stdin - LogFileUtil coverage: 60% → 78%
- Add test for InterruptedException thrown during drain loop (inner catch in shutdown path) - Add DefaultLifecycleReceiver using default interface start/end methods - ReceiverThread: 77% → 100%
- LogFileUtil.addPathSuffix: String.endsWith("") is always true, so
passing an empty suffix incorrectly returned "basename_2.ext".
Guard with an early return when suffix is empty.
- LogTable.LogValue.equals(): customTypeStr and unitStr were compared
with == (reference equality). struct.getTypeString() returns a freshly
concatenated String on each call, so two semantically equal LogValues
with non-interned strings compared as unequal. Replace with
Objects.equals(), which also removes the now-redundant null guards.
Tests updated/added to assert correct behaviour.
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
1 participant
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.
Add comprehensive JUnit 5 tests for LogTable, LogFileUtil, ReceiverThread, AutoLogOutputManager, and LoggedMechanism2d, targeting >90% meaningful coverage focused on identifying real defects rather than pure line coverage.
Bugs found and fixed:
Known issues documented in tests (not yet fixed):