refactor: table-driven DIMM format parsing with tests#654
Merged
harp-intel merged 5 commits intomainfrom Mar 3, 2026
Merged
Conversation
Replace the cascading if-else in getDIMMParseInfo and switch in getDIMMSocketSlot with a unified dimmFormats table. Each entry holds pre-compiled regexes, match predicates, and extraction functions. - Fix regex bugs: [\d+] matched single char, not multi-digit numbers - Add descriptive dimmType names (dimmTypeInspurICX, dimmTypeQuantaGNR, etc.) - Add 66 characterization tests covering all 17 formats, ordering edge cases, and multi-digit regression cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the DIMM format detection and parsing logic in internal/extract/dimm.go to use a table-driven approach. It also fixes regex bugs where [\d+] was incorrectly used (matching a single character from the set \d, + rather than one-or-more digits) in some patterns, adds descriptive names for the previously opaque dimmType0–dimmType16 constants, and adds 66 new characterization tests.
Changes:
- Replace cascading if-else/switch in
getDIMMParseInfoandgetDIMMSocketSlotwith a unifieddimmFormatstable containing pre-compiled regexes, match predicates, and extraction functions - Add descriptive
dimmTypeconstant names with backward-compatible aliases for the olddimmType0–dimmType16names - Add a comprehensive test file (
dimm_test.go) with 66 test cases covering all 17 DIMM formats, ordering edge cases, and multi-digit regression cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
internal/extract/dimm.go |
Main refactor: replaces cascading if-else with dimmFormats table, adds new descriptive constants and aliases, introduces dimmFormat struct |
internal/extract/dimm_test.go |
New file: 66 test cases across TestGetDIMMParseInfo, TestGetDIMMSocketSlot, and TestDeriveDIMMInfoOther |
Update all test references from dimmType0-16 to descriptive names (dimmTypeInspurICX, dimmTypeQuantaGNR, etc.) and remove the backward compatibility aliases from dimm.go. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Exercise the return nil, nil path (line 597) where the first DIMM identifies a format but a subsequent DIMM fails to match it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
For formats requiring both bankLocPat and locPat to match (matchBoth), getDIMMSocketSlot now requires both matches to be non-nil before calling extractFunc. Previously, a partial match (one nil, one non-nil) would pass a nil slice to extractFunc, causing an index-out-of-range panic. This is realistic in deriveDIMMInfoOther where the format is identified from dimms[0] but applied to all subsequent DIMM rows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
getDIMMParseInfoand switch ingetDIMMSocketSlotwith a unifieddimmFormatstable where each entry holds pre-compiled regexes, match predicates, and extraction functions[\d+]matched a single character instead of multi-digit numbers (affected Quanta GNR, Birchstream, and Forest City formats)dimmTypenames (dimmTypeInspurICX,dimmTypeQuantaGNR, etc.) replacing opaquedimmType0–dimmType16Test plan
go test ./internal/extract/ -run "TestGetDIMM|TestDeriveDIMMInfoOther")make test) with zero failures🤖 Generated with Claude Code