Replace asserts with exceptions and refactor test structure#32
Merged
Harry-Chen merged 3 commits intomasterfrom Mar 13, 2026
Merged
Replace asserts with exceptions and refactor test structure#32Harry-Chen merged 3 commits intomasterfrom
Harry-Chen merged 3 commits intomasterfrom
Conversation
assert() is stripped in release builds, leaving invalid states undetected. Replace all remaining assert() calls in lib/ with appropriate exception types: - RangeError.range for numeric bound checks - FormatException for malformed data during decoding - ArgumentError.value for invalid argument values Also fix misuse of ArgumentError.notNull for a non-null validation, and correct "adress" → "address" typo in error message. Made-with: Cursor
Extract common helpers (testParse, testGenerate, testRoundTrip) into
test/helpers.dart. Split ndef_test.dart into focused test files
mirroring the lib/records/ directory structure:
test/
helpers.dart
record_test.dart
utilities_test.dart
records/
well_known/
uri_test.dart
text_test.dart
signature_test.dart
device_info_test.dart
smart_poster_test.dart
handover_test.dart
media/
bluetooth_test.dart
wifi_test.dart
Also replace assert() with expect() across all test files for
better failure diagnostics, and reorganize wifi_test.dart with
group() for logical structure.
Made-with: Cursor
Signed-off-by: Shengqi Chen <harry-chen@outlook.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes runtime validation by replacing assert-based checks with explicit exceptions in several NDEF record implementations, and refactors the Dart test suite into smaller, record-focused files with shared helper utilities.
Changes:
- Replace
assertstatements with thrown exceptions (FormatException,RangeError,ArgumentError) in Bluetooth, Handover, and Text record parsing/validation paths. - Introduce
test/helpers.dartfor shared encode/decode/round-trip test helpers and migrate existing message tests into focused files. - Reorganize tests by record type (e.g., well-known vs media) and add/adjust tests for WiFi/Bluetooth/utility behavior.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/wifi_test.dart | Removes the old monolithic WiFi test file (superseded by record-scoped tests). |
| test/utilities_test.dart | Adds focused tests for ByteUtils and hex conversion helpers. |
| test/records/well_known/uri_test.dart | Adds URI record encode/decode tests using shared helpers. |
| test/records/well_known/text_test.dart | Adds Text record encode/decode tests (UTF-8/UTF-16/long payload). |
| test/records/well_known/smart_poster_test.dart | Adds Smart Poster record encode/decode coverage via helpers. |
| test/records/well_known/signature_test.dart | Adds Signature record encode/decode tests. |
| test/records/well_known/handover_test.dart | Adds Handover record parse + round-trip coverage using helpers. |
| test/records/well_known/device_info_test.dart | Adds Device Information record encode/decode coverage using helpers. |
| test/records/media/wifi_test.dart | Reintroduces WiFi tests in a media-record scoped structure with expect(...). |
| test/records/media/bluetooth_test.dart | Adds Bluetooth Easy Pairing record encode/decode test using helpers. |
| test/record_test.dart | Adds construction/default-value tests and a basic exception test. |
| test/ndef_test.dart | Removes the previous all-in-one test file (split across new files + helpers). |
| test/helpers.dart | New shared test helpers for parse/generate/round-trip and hex conversion. |
| lib/records/well_known/text.dart | Replaces a decoding assert with a FormatException for invalid language length. |
| lib/records/well_known/handover.dart | Replaces multiple asserts with RangeError/FormatException for invalid inputs/trailing bytes. |
| lib/records/media/bluetooth.dart | Replaces several asserts with explicit exceptions for invalid Bluetooth address/data shapes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+84
to
+88
| if (carrierPowerStateIndex < 0 || | ||
| carrierPowerStateIndex >= CarrierPowerState.values.length) { | ||
| throw RangeError.range( | ||
| carrierPowerStateIndex, | ||
| 0, |
Comment on lines
+137
to
+141
| if (!stream.isEnd()) { | ||
| throw FormatException( | ||
| "Payload has ${stream.unreadLength} unexpected trailing bytes", | ||
| ); | ||
| } |
Comment on lines
+21
to
+24
| throw ArgumentError.value( | ||
| bytes.length, | ||
| "bytes.length", | ||
| "Bluetooth address must be 6 bytes", |
Comment on lines
+9
to
+13
| void testParse(List<String> hexStrings, List<List<NDEFRecord>> messages) { | ||
| for (int i = 0; i < hexStrings.length; i++) { | ||
| var decoded = decodeRawNdefMessage(hexStrings[i].toBytes()); | ||
| expect(decoded.length, messages[i].length, | ||
| reason: 'Message $i record count mismatch'); |
Comment on lines
+109
to
+111
| if (languagePayloadLength == 0) { | ||
| throw FormatException("Language code length must not be zero"); | ||
| } |
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.
This pull request improves error handling and test structure throughout the codebase. It replaces
assertstatements with explicit exceptions for better error reporting, introduces new helper functions for test code reuse, and splits tests into more focused files. The most important changes are summarized below:Error Handling Improvements
assertstatements with explicit exception throwing (ArgumentError,FormatException,RangeError) for invalid input values and formats in Bluetooth and NFC record classes, ensuring clearer runtime error messages and safer input validation. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Test Code Refactoring
testParse,testGenerate,testRoundTrip, etc.) into a newtest/helpers.dartfile for easier reuse and maintenance.test/ndef_test.dartand updated it to use the new helpers, streamlining and clarifying the test code.Test Suite Organization
test/record_test.dartfile, using thetestpackage'sexpectfunctions for clearer, more robust assertions.test/records/media/bluetooth_test.dartto specifically test Bluetooth Easy Pairing record encoding and decoding, improving test coverage and organization.