Require label for Message #322
Conversation
WalkthroughMake Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25–30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/plugins/Label_2P_FM4.test.ts (1)
74-83:⚠️ Potential issue | 🟡 MinorMissing
decodeResult.messageassertion in the invalid test case.Both variant 1 (Line 22) and variant 2 (Line 51) assert
expect(decodeResult.message).toBe(message), but this test omits it. For consistency and to verify the decoder attaches the message reference even on failed decodes, consider adding the same check here.Suggested addition
expect(decodeResult.decoded).toBe(false); expect(decodeResult.decoder.decodeLevel).toBe('none'); + expect(decodeResult.message).toBe(message); expect(decodeResult.formatted.description).toBe('Flight Report'); expect(decodeResult.formatted.items.length).toBe(0);lib/plugins/Label_22_POS.test.ts (1)
23-40:⚠️ Potential issue | 🟡 MinorMissing semicolon on Line 24.
Line 24 is missing a trailing semicolon, unlike the other
message.textassignments in this file (e.g., Line 44).Proposed fix
- message.text = 'N 370824W 760010,-------,194936,30418, , , ,M 42,27335 42, 107,' + message.text = 'N 370824W 760010,-------,194936,30418, , , ,M 42,27335 42, 107,';
🤖 Fix all issues with AI agents
In `@lib/plugins/Label_10_LDR.test.ts`:
- Around line 23-25: Two message.text assignments in the tests are missing
trailing semicolons causing inconsistent style; add a semicolon to the end of
the string assignments where message.text is set (e.g., in the test "decodes
Label 10 Preamble LDR variant 1" and the other message.text assignment later in
the same test file) so both statements mirror the semicolon usage used elsewhere
(the decode call plugin.decode(message) should remain unchanged).
In `@lib/plugins/Label_13Through18_Slash.test.ts`:
- Around line 303-312: The test "does not decode <invalid>" uses message.text =
'/14 Bogus Message' while message.label remains '13', creating an unintended
label mismatch; update the test so the message.label matches the payload (set
message.label to '14') or explicitly assert a mismatched-label scenario—modify
the setup in this test to either set message.label = '14' before calling
plugin.decode(message) or change the test name/assertions to document that a
mismatched label is being tested, referencing the test case and the call to
plugin.decode(message).
In `@lib/plugins/Label_1L_070.test.ts`:
- Around line 25-26: Add the missing semicolon at the end of the message.text
assignment to match project style and avoid potential lint/parse issues; locate
the statement setting message.text (the line before the call to
plugin.decode(message)) and terminate it with a semicolon so the code becomes a
proper statement before invoking plugin.decode.
In `@lib/plugins/Label_30_Slash_EA.test.ts`:
- Line 13: The test name contains a typo: rename the test titled "matches
qualfiers" to "matches qualifiers" by updating the string in the test(...) call
(the test name literal) in the file containing the test function (the test
invocation starting with test('matches qualfiers', ...)); leave the test body
and assertions unchanged so only the human-readable test description is
corrected.
In `@lib/plugins/Label_44_ETA.test.ts`:
- Line 4: The describe block name is wrong: change the string in the top-level
describe call from 'Label 44 IN' to 'Label 44 ETA' so the test suite name
matches the file and imported plugin; locate the describe(...) invocation in
Label_44_ETA.test.ts and update its description accordingly.
In `@lib/plugins/Label_H1_FLR.test.ts`:
- Line 12: The test name string in the unit test 'matches qualifiers' contains
a double space; update the test declaration in Label_H1_FLR.test.ts (the
test(...) call) to use a single space so it reads 'matches qualifiers' and
verify any related references (e.g., test descriptions or snapshots) are updated
to match the corrected string.
In `@lib/plugins/Label_H1_FTX.test.ts`:
- Around line 17-18: Add missing trailing semicolons for consistency: ensure
each statement like the message.text assignments and the following const
decodeResult = plugin.decode(message) have a semicolon at the end (e.g., add
semicolons to the message.text lines shown and to the decodeResult assignment)
so all occurrences of message.text and the plugin.decode(message) call follow
the same style.
In `@lib/plugins/Label_H1_PRG.test.ts`:
- Line 4: The test suite label is incorrect: update the describe call string
from "Label_H1 POS" to "Label_H1 PRG" so the suite name displays correctly;
locate the describe('Label_H1 POS', ...) invocation in the test file
(Label_H1_PRG.test.ts) and change the label text to "Label_H1 PRG".
- Line 124: Remove the stray debug statement console.log(JSON) from the skipped
test in Label_H1_PRG.test.ts; either delete that line entirely or replace it
with the intended debug output (e.g., the specific variable or value you meant
to inspect) so the global JSON constructor is not logged accidentally. Locate
the call in the skipped test block (search for console.log(JSON) inside the
Label_H1_PRG.test.ts test definitions) and update or remove it accordingly.
🧹 Nitpick comments (30)
lib/plugins/Label_H1_PER.test.ts (1)
7-7: Shared mutable fixture risks test isolation ifdecode()mutates the message object.
messageis declared once atdescribescope and never reset between tests. Each test correctly setsmessage.text, but ifplugin.decode()ever adds or modifies other properties on the object (e.g., sets a flag, attaches metadata), that state leaks into subsequent tests. Consider resetting inbeforeEach:Suggested improvement
let plugin: Label_H1; - const message = {label: 'H1', text: ''}; + let message: { label: string; text: string }; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_H1(decoder); + message = { label: 'H1', text: '' }; });Since this pattern is replicated across 45+ test files, this may be an intentional trade-off — just flagging the risk.
lib/plugins/Label_16_Honeywell.test.ts (1)
6-11: Shared mutablemessagerisks test coupling ifdecode()ever mutates the object.
messageis declared once at suite scope and never reset inbeforeEach. Ifplugin.decode()adds or modifies properties on the message beyondtext(now or in the future), that state silently leaks into subsequent tests. Consider resetting it inbeforeEachalongsideplugin:Suggested improvement
describe("Label_16_Honeywell", () => { let plugin: Label_16_Honeywell; - const message = { label: "16", text: "" }; + let message: { label: string; text: string }; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_16_Honeywell(decoder); + message = { label: "16", text: "" }; });lib/plugins/Label_2P_FM4.test.ts (1)
7-7: Shared mutable test fixture creates implicit coupling between tests.Each test mutates
message.texton a suite-level object. While this works given Jest's sequential execution, it means tests are order-dependent and a future test that forgets to setmessage.textwill silently inherit stale state from the previous test. Consider resettingmessageinsidebeforeEachor constructing it per-test.Given this is a project-wide convention across 45+ files, this is a minor observation rather than a blocking concern.
lib/plugins/Label_12_POS.test.ts (1)
7-12: Shared mutablemessageobject is not reset between tests.
messageis declared once at suite scope and mutated in each test, butbeforeEachonly resetsplugin. Since each test setsmessage.textbefore callingdecode, this works today. However, ifdecodeever mutates other properties onmessage(e.g., sets fields on the object), those mutations would leak between tests, causing hard-to-diagnose coupling.Consider resetting
messageinbeforeEachto isolate tests:Suggested change
let plugin: Label_12_POS; - const message = {label: '12', text: ''}; + let message: {label: string, text: string}; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_12_POS(decoder); + message = {label: '12', text: ''}; });This applies to all other test files in this PR that follow the same pattern.
lib/plugins/Label_1J_2J_FTX.test.ts (1)
7-7: Minor:letused instead ofconst— inconsistent with other test files in this PR.All other test files use
const message = .... Since the reference itself isn't reassigned (onlymessage.textis mutated),constwould be more appropriate here. Alternatively, if adopting thebeforeEachreset pattern,letis correct.lib/plugins/Label_H1_INI.test.ts (1)
7-7: Shared mutablemessageobject across tests — consider resetting inbeforeEach.Since
messageis aconstreference shared across all tests and mutated viamessage.text = ..., any properties thedecode()method may attach to the message object will leak into subsequent tests. This applies to all 7 files in this PR. A safer pattern is to reinitialize the message inbeforeEach:♻️ Suggested reset pattern
- const message = {label: 'H1', text: ''}; + let message: {label: string, text: string}; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_H1(decoder); + message = {label: 'H1', text: ''}; });lib/plugins/Label_16_N_Space.test.ts (1)
7-7: Shared mutablemessagerisks test coupling ifdecode()ever mutates its input.The
messageobject is declared once withconstand mutated across all tests. Ifplugin.decode()ever adds/modifies properties on the input (e.g., normalizestext, attaches metadata), later tests will inherit those mutations, causing order-dependent failures. This applies to all 7 test files in this PR.A safer pattern is to reinitialize
messageinbeforeEach, or use a factory function:♻️ Suggested safer pattern
- const message = {label: '16', text: ''}; + let message: {label: string, text: string}; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_16_N_Space(decoder); + message = {label: '16', text: ''}; });lib/plugins/Label_44_ON.test.ts (1)
23-49: Variant 1 and 2 are missing thedecodeResult.messageidentity assertion.Other test files and the invalid-case test in this file (Line 87) assert
expect(decodeResult.message).toBe(message). The two success-path tests don't, which is inconsistent with the PR-wide pattern.Also applies to: 51-78
lib/plugins/Label_44_OFF.test.ts (1)
23-54: Variant 1 is missing thedecodeResult.messageidentity assertion, inconsistent with the invalid-case test at Line 63.Same observation as in
Label_44_ON.test.ts.lib/plugins/Label_44_Slash.test.ts (1)
23-36: Variant 1 and 2 are missing thedecodeResult.messageidentity assertion, inconsistent with the invalid-case test at Line 62.Same observation as the other
Label_44_*test files.Also applies to: 38-54
lib/plugins/Label_44_ETA.test.ts (1)
23-57: Variant 1 is missing thedecodeResult.messageidentity assertion, inconsistent with the invalid-case test at Line 66.lib/plugins/Label_4J_POS.test.ts (1)
58-67: MissingdecodeResult.messageassertion for consistency.Other test files (e.g.,
Label_12_N_Space.test.ts,Label_1L_Slash.test.ts) assertexpect(decodeResult.message).toBe(message)in the invalid-message test case. This test omits it, which is inconsistent with the pattern established across the PR.lib/plugins/Label_1L_3-line.test.ts (1)
83-92: MissingdecodeResult.messageassertion for consistency.Same as flagged in
Label_4J_POS.test.ts— the invalid-message test in other files assertsexpect(decodeResult.message).toBe(message)but this one does not.lib/plugins/Label_H1_StarPOS.test.ts (1)
44-53: MissingdecodeResult.messageassertion for consistency.The valid-decode test (Line 21) asserts
decodeResult.messagereference equality, but this invalid test does not.lib/plugins/Label_H1_FPN.test.ts (1)
14-40: InconsistentdecodeResult.messageassertion coverage within this file.Only the invalid test (Line 262) and the RM test (Line 282) assert
expect(decodeResult.message).toBe(message). The other ~10 decode tests do not. Consider applying the assertion consistently across all tests in this file—or removing it from the two that have it—to keep the test suite uniform.Also applies to: 255-263, 265-283
lib/plugins/Label_1L_660.test.ts (1)
6-6: Shared mutablemessageacross tests risks silent test-order coupling.Since
messageis declared once atdescribescope and mutated by each test, any test that forgets to setmessage.textwill silently inherit the value left by the previous test. Consider resettingmessage.textinsidebeforeEach, or creating a fresh object per test. This pattern repeats across all test files in this PR.♻️ Suggested fix — reset in beforeEach
beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_1L_660(decoder); + message.text = ''; });lib/plugins/Label_4N.test.ts (3)
13-19: Redundant decoder/plugin instantiation in qualifiers test.This test creates its own
decoderanddecoderPlugin(lines 14-15) even thoughpluginis already available frombeforeEach. Consider using the sharedplugininstance for consistency with the rest of the file. This is pre-existing code now wrapped in the describe block.Suggested cleanup
test('matches Label 4N qualifiers', () => { - const decoder = new MessageDecoder(); - const decoderPlugin = new Label_4N(decoder); - - expect(decoderPlugin.decode).toBeDefined(); - expect(decoderPlugin.name).toBe('label-4n'); + expect(plugin.decode).toBeDefined(); + expect(plugin.name).toBe('label-4n'); + expect(plugin.qualifiers).toBeDefined(); + expect(plugin.qualifiers()).toEqual({ + labels: ['4N'], + }); });
13-138: Inconsistent indentation: tests insidedescribeare not indented.All
test(...)blocks from lines 13–138 sit at the top indentation level despite being nested inside thedescribeblock opened at line 4. Other files in this PR (e.g.,Label_22_POS.test.ts,Label_SQ.test.ts) properly indent tests within theirdescribe. This appears to be an artifact of wrapping existing tests without re-indenting.
129-138: Invalid-case test doesn't assertdecodeResult.message.Other invalid tests in this PR (e.g.,
Label_22_POS.test.tsline 49,Label_10_LDR.test.tsline 79) assertexpect(decodeResult.message).toBe(message). This test omits that assertion, which is an inconsistency with the pattern established across the PR.lib/plugins/Label_30_Slash_EA.test.ts (1)
13-41: Tests insidedescribeare not indented.Same indentation inconsistency as in
Label_4N.test.ts— the test blocks should be indented one level within thedescribefor consistency with other files in this PR (e.g.,Label_22_POS.test.ts,Label_SQ.test.ts).lib/plugins/Label_83.test.ts (1)
116-127: Variant 3 tests still create their own decoder/plugin despite sharedpluginbeing available.Lines 117–118 and 142–143 instantiate a local
decoder/decoderPluginwhile the sharedpluginfrombeforeEachgoes unused. These tests do use the sharedmessageobject, creating an inconsistent mix. Consider usingpluginthroughout for uniformity with the other tests in this file.Suggested cleanup (variant 3)
test('decodes Label 83 variant 3', () => { - const decoder = new MessageDecoder(); - const decoderPlugin = new Label_83(decoder); - // https://globe.adsbexchange.com/?icao=AC15A1&showTrace=2024-09-22×tamp=1726977342 message.text = '001PR22035539N4038.6W07427.80292500008'; - const decodeResult = decoderPlugin.decode(message); + const decodeResult = plugin.decode(message);Also applies to: 141-153
lib/plugins/Label_1M_Slash.test.ts (1)
13-58: Tests insidedescribeare not indented.Same indentation issue as
Label_4N.test.tsandLabel_30_Slash_EA.test.ts— test blocks should be indented one level within thedescribefor consistency with files likeLabel_SQ.test.tsandLabel_22_POS.test.tsin this PR.lib/plugins/Label_H1_WRN.test.ts (1)
4-12: Shared mutablemessageobject risks cross-test contamination.
messageis a singleconstobject declared atdescribescope and mutated by each test. While this works because each test reassignsmessage.textbefore callingdecode, the plugin stores a reference (decodeResult.message = message), so any post-decode mutation ofmessagein a later test retroactively changes thedecodeResult.messagefrom a prior test. This is fragile if tests are later reordered or assertions are added that inspect results across tests.A safer pattern is to create a fresh message per test (or reset in
beforeEach):Suggested approach
- const message = {label: 'H1', text: ''}; + let message: {label: string, text: string}; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_H1_WRN(decoder); + message = {label: 'H1', text: ''}; });This applies to all 7 test files in this PR that use the same pattern.
lib/plugins/Label_8E.test.ts (1)
13-23: Redundantdecoder/decoderPlugincreation in qualifiers test.This test creates a local
decoderanddecoderPlugin(lines 14-15) despitepluginalready being initialized inbeforeEach. Other test files in this PR (e.g.,Label_H1_WRN.test.tsline 15,Label_HX.test.tsline 15) use the sharedplugindirectly. This looks like leftover code from before the refactor.Suggested fix
test('matches qualifiers', () => { - const decoder = new MessageDecoder(); - const decoderPlugin = new Label_8E(decoder); - - expect(decoderPlugin.decode).toBeDefined(); - expect(decoderPlugin.name).toBe('label-8e'); - expect(decoderPlugin.qualifiers).toBeDefined(); - expect(decoderPlugin.qualifiers()).toEqual({ + expect(plugin.decode).toBeDefined(); + expect(plugin.name).toBe('label-8e'); + expect(plugin.qualifiers).toBeDefined(); + expect(plugin.qualifiers()).toEqual({ labels: ['8E'], }); });lib/plugins/Label_2P_FM3.test.ts (1)
14-32: InconsistentdecodeResult.messageassertion across variants.Variant 3 (line 65) asserts
expect(decodeResult.message).toBe(message), but variants 1 and 2 do not. Other test files in this PR (e.g.,Label_H1_WRN.test.ts,Label_HX.test.ts) consistently include this assertion in every test. Consider adding it to variants 1, 2, and the invalid test for consistency.lib/plugins/Label_H1_OHMA.test.ts (2)
4-10: Sharedmessageobject is not reset inbeforeEach, risking test pollution.
message.textis mutated per test but never reset inbeforeEach. If a future test omits settingmessage.text, it will silently inherit the previous test's value. Consider resettingmessage.text = ''insidebeforeEachalongside the plugin re-creation, or creating a freshmessageobject per test.This concern applies to all the test files in this PR that follow the same pattern.
Proposed fix
beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_H1_OHMA(decoder); + message.text = ''; });
12-23: Qualifiers test doesn't use the sharedplugin— inconsistent with the refactoring.This test creates a local
decoderanddecoderPlugininstead of using the sharedpluginfrombeforeEach. Other test files in this PR (e.g.,Label_4A.test.ts,Label_4A_DIS.test.ts) use the sharedpluginfor their qualifiers test.Proposed fix
test('matches Label H1 Preamble OHMA qualifiers', () => { - const decoder = new MessageDecoder(); - const decoderPlugin = new Label_H1_OHMA(decoder); - - expect(decoderPlugin.decode).toBeDefined(); - expect(decoderPlugin.name).toBe('label-h1-ohma'); - expect(decoderPlugin.qualifiers).toBeDefined(); - expect(decoderPlugin.qualifiers()).toEqual({ + expect(plugin.decode).toBeDefined(); + expect(plugin.name).toBe('label-h1-ohma'); + expect(plugin.qualifiers).toBeDefined(); + expect(plugin.qualifiers()).toEqual({ labels: ['H1'], preambles: ['OHMA', '/RTNBOCR.OHMA', '#T1B/RTNBOCR.OHMA'], }); });lib/plugins/Label_4A_Slash_01.test.ts (1)
23-38: Decode test creates a redundant local decoder/plugin while mutating the sharedmessage.Lines 24–25 instantiate a local
decoderanddecoderPlugin, but line 28 mutates the sharedmessage.textand line 29 passes the sharedmessagetodecoderPlugin.decode. ThebeforeEachalready creates a sharedplugin— this test should use it for consistency with the rest of the refactored suite.Proposed fix
test('decodes Label 4A_Slash_01', () => { - const decoder = new MessageDecoder(); - const decoderPlugin = new Label_4A_Slash_01(decoder); - // https://app.airframes.io/messages/3460403039 message.text = '/01-C'; - const decodeResult = decoderPlugin.decode(message); + const decodeResult = plugin.decode(message); expect(decodeResult.decoded).toBe(true);lib/plugins/Label_4A_01.test.ts (1)
52-64: Skipped test still creates a redundant local decoder/plugin.The skipped invalid test at lines 53–54 creates a local
decoderanddecoderPlugin, then uses the sharedmessage. Sincepluginis already available frombeforeEach, this should use the shared instance for consistency — especially if the test is ever unskipped.Proposed fix
test.skip('decodes Label 4A_01 <invalid>', () => { - const decoder = new MessageDecoder(); - const decoderPlugin = new Label_4A_01(decoder); - message.text = '4A_01 Bogus message'; - const decodeResult = decoderPlugin.decode(message); + const decodeResult = plugin.decode(message);lib/plugins/Label_4A_DIS.test.ts (1)
12-12: Nit: double space in test name.
'matches Label 4A_DIS qualifiers'has a double space before "qualifiers".Proposed fix
-test('matches Label 4A_DIS qualifiers', () => { +test('matches Label 4A_DIS qualifiers', () => {
lib/plugins/Label_44_ETA.test.ts
Outdated
| @@ -3,6 +3,7 @@ import { Label_44_ETA } from './Label_44_ETA'; | |||
|
|
|||
| describe('Label 44 IN', () => { | |||
There was a problem hiding this comment.
Describe block name 'Label 44 IN' appears to be a copy-paste error — this file tests Label_44_ETA.
Should be 'Label 44 ETA' to match the imported plugin and the rest of the test suite naming convention.
🔧 Proposed fix
-describe('Label 44 IN', () => {
+describe('Label 44 ETA', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('Label 44 IN', () => { | |
| describe('Label 44 ETA', () => { |
🤖 Prompt for AI Agents
In `@lib/plugins/Label_44_ETA.test.ts` at line 4, The describe block name is
wrong: change the string in the top-level describe call from 'Label 44 IN' to
'Label 44 ETA' so the test suite name matches the file and imported plugin;
locate the describe(...) invocation in Label_44_ETA.test.ts and update its
description accordingly.
lib/plugins/Label_H1_PRG.test.ts
Outdated
| @@ -4,6 +4,7 @@ import { Label_H1 } from './Label_H1'; | |||
| describe('Label_H1 POS', () => { | |||
There was a problem hiding this comment.
Incorrect describe label: "POS" should be "PRG".
This appears to be a copy-paste error from another H1 test file. The test suite name will display incorrectly in test output.
Proposed fix
-describe('Label_H1 POS', () => {
+describe('Label_H1 PRG', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('Label_H1 POS', () => { | |
| describe('Label_H1 PRG', () => { |
🤖 Prompt for AI Agents
In `@lib/plugins/Label_H1_PRG.test.ts` at line 4, The test suite label is
incorrect: update the describe call string from "Label_H1 POS" to "Label_H1 PRG"
so the suite name displays correctly; locate the describe('Label_H1 POS', ...)
invocation in the test file (Label_H1_PRG.test.ts) and change the label text to
"Label_H1 PRG".
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/plugins/Label_H1_FLR.test.ts (1)
98-115:⚠️ Potential issue | 🟡 MinorDuplicate test name — this tests ATC, not LQD.
This test is named
'decodes Label H1 Preamble FLR LQD', identical to the test at line 79, but it actually decodes a#CFBFLR/ ATC message. Duplicate names make it impossible to distinguish failures in test runner output.Proposed fix
-test('decodes Label H1 Preamble FLR LQD', () => { +test('decodes Label H1 Preamble FLR ATC', () => {
🧹 Nitpick comments (7)
lib/plugins/Label_H1_PRG.test.ts (1)
7-7: Shared mutablemessagenot reset between tests — minor test-isolation risk.The
messageobject is shared across all tests and onlymessage.textis reassigned before each test. Ifplugin.decode(or a future refactor of it) ever mutates themessageobject (e.g., adds or modifies a property), that mutation will leak into subsequent tests. Consider resetting the object inbeforeEachfor resilience:Suggested improvement
let plugin: Label_H1; - const message = {label: 'H1', text: ''}; + let message: {label: string, text: string}; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_H1(decoder); + message = {label: 'H1', text: ''}; });Currently
Label_H1.decodeonly readsmessage.textand stores a reference (decodeResult.message = message), so this is not a live bug today — just a fragility concern.lib/plugins/Label_13Through18_Slash.test.ts (1)
22-27: Shared mutablemessageobject across tests risks test isolation.Each
describeblock declares a singlemessageobject and mutates.textbefore each test. Ifplugin.decode()ever mutates the input message (e.g., attaches metadata, normalizes fields), those side effects leak into subsequent tests, causing hard-to-debug order-dependent failures.A safer approach is to construct a fresh object per test or use
beforeEachto reset it:♻️ Example using beforeEach
describe('Label 13', () => { - const message = {label: '13', text: ''}; + let message: {label: string, text: string}; + beforeEach(() => { + message = {label: '13', text: ''}; + }); test('decodes 1-line', () => {This applies to all six label
describeblocks (lines 23, 65, 151, 194, 221, 269).lib/plugins/Label_30_Slash_EA.test.ts (2)
4-11: Shared mutablemessageis not reset inbeforeEach.
messageis declared once at describe scope and its.textis mutated per test (line 23), butbeforeEachonly re-creates the plugin. If additional decode tests are added later, they'll inherit.textfrom whatever test ran previously, which breaks test isolation.Consider resetting
message.textinbeforeEach:Proposed fix
beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_30_Slash_EA(decoder); + message.text = ''; });
13-21: Indentation: test blocks insidedescribeare not indented.Both
test(...)blocks (lines 13 and 22) sit at column 0 while the rest of thedescribebody (lines 5–11) is indented by 2 spaces. This is purely cosmetic but inconsistent.lib/plugins/Label_H1_FTX.test.ts (1)
7-12: Shared mutablemessagemay leak state between tests.If
plugin.decodeever mutates themessageobject (e.g., attaches decoded properties), those mutations persist into subsequent tests since onlypluginis reset inbeforeEach. Consider resettingmessageinbeforeEachas well, or constructing a fresh object per test.♻️ Suggested change
let plugin: Label_H1; - const message = {label: 'H1', text: ''}; + let message: {label: string, text: string}; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_H1(decoder); + message = {label: 'H1', text: ''}; });lib/plugins/Label_H1_FLR.test.ts (2)
4-11: Sharedmessageobject is not reset inbeforeEach.
pluginis re-created each run, butmessageretains the.textfrom the previous test. This works because every test re-assignsmessage.textbefore callingdecode, but it would be more robust and consistent to move the message initialization insidebeforeEachalongsideplugin.♻️ Suggested refactor
describe('Label H1 Preamble FLR', () => { let plugin: Label_H1_FLR; - const message = {label: 'H1', text: ''}; + let message: { label: string; text: string }; beforeEach(() => { const decoder = new MessageDecoder(); plugin = new Label_H1_FLR(decoder); + message = { label: 'H1', text: '' }; });
12-20: Inconsistent indentation: tests are not indented inside thedescribeblock.Lines 5–11 are indented within the
describecallback, but alltest(...)blocks starting at line 12 are flush-left. Syntactically they're still insidedescribe(closed at line 128), but the indentation is misleading.
1 participant
require a message label, to choose the correct decoder
Summary by CodeRabbit