fix(importers): stop doubling the (scan_type) suffix in dynamic Test Type names#15149
Open
Maffooch wants to merge 1 commit into
Open
fix(importers): stop doubling the (scan_type) suffix in dynamic Test Type names#15149Maffooch wants to merge 1 commit into
Maffooch wants to merge 1 commit into
Conversation
…Type names
Generic Findings Import (and other dynamic-test-type parsers like SARIF)
built the Test Type name by unconditionally appending " Scan ({scan_type})"
to the report's `type` field. When `type` already carried the
" ({scan_type})" suffix, the suffix was doubled, e.g.
"Prisma Cloud (Generic Findings Import) Scan (Generic Findings Import)".
Extract the name resolution into resolve_dynamic_test_type_name(), which is
now idempotent: a `type` already ending in " ({scan_type})" is used verbatim.
The intentional "{type} Scan ({scan_type})" format is unchanged for all other
cases (Tool1 -> "Tool1 Scan (Generic Findings Import)"), preserving SARIF and
existing behavior.
To avoid breaking existing data, the reimport validation accepts either the
new idempotent name or the legacy (pre-patch) name via
legacy_dynamic_test_type_name(), so reimports into tests whose test_type was
created with the old doubled name keep working without a mismatch error.
Also corrects the Generic Findings Import docs, which described a
"{Test Name} (Generic Findings Import)" format the code never produced.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
34b912b to
68f4cbe
Compare
valentijnscholten
left a comment
Member
There was a problem hiding this comment.
Does auto create context need to be aware of old and new formats?
Does deduplication need to be aware of this? Probably not as the get_or_create ensures that if an old format test_type exists it will not create a new one?
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.
Shortcut: sc-13441
Summary
Fixes a bug report: Generic Findings Import created doubled Test Type names such as
Prisma Cloud (Generic Findings Import) Scan (Generic Findings Import).Dynamic-test-type parsers (Generic, SARIF, and ~9 others) build the
Test_Typename by appendingScan ({scan_type})to the report'stypefield. The composition was not idempotent: whentypealready ended with the({scan_type})suffix, it was appended again, producing the doubled name.What changed
resolve_dynamic_test_type_name()(new): single, idempotent name resolver. Atypealready ending in({scan_type})is used verbatim; everything else keeps the intentional{type} Scan ({scan_type})format (soTool1→Tool1 Scan (Generic Findings Import), and SARIF is unchanged).legacy_dynamic_test_type_name()(new): reproduces the pre-patch name. The reimport validation accepts either the new or the legacy name, so reimports into tests created with the old doubled name keep working — no data migration, no newTest type mismatcherrors.generic_findings_import.md,OS__tests.md,PRO__tests.md,file/generic.md, and added the idempotency rule toOS__asset_hierarchy.md. The old docs described a{Test Name} (Generic Findings Import)format the code never produced, which is what set the customer's expectation.Testing
TDD (failing test first, confirmed red reproducing the doubled name, then green):
unittests/scans/generic/generic_test_type_suffix.json+ 3 regression tests: no-doubling on import, idempotent reimport, and pre-patch doubled-name reimport still works.type: "Prisma Cloud (Generic Findings Import)"→Prisma Cloud (Generic Findings Import)(no doubling);type: "Tool1E2E"→Tool1E2E Scan (Generic Findings Import)(unchanged); reimport → HTTP 201, no mismatch.🤖 Generated with Claude Code