From 6b0303b7a6aa92dc86410049b6b11c21e283e858 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Wed, 22 Jul 2026 15:23:26 +0200 Subject: [PATCH 01/14] Update vscode config --- .vscode/settings.json | 1 - 1 file changed, 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index af5d05721..a7c57fea5 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -6,7 +6,6 @@ "source.generate.finalModifiers": "explicit", "source.fixAll": "explicit" }, - "java.saveActions.organizeImports": true, "java.sources.organizeImports.starThreshold": 3, "java.sources.organizeImports.staticStarThreshold": 3, "java.configuration.updateBuildConfiguration": "automatic", From c6a6a378bd661706f9f47f9efc8ec9ea9f41dda6 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Wed, 22 Jul 2026 15:23:34 +0200 Subject: [PATCH 02/14] Add implementation plan --- ...gherkin-feature-specification-documents.md | 198 ++++++++++++++++++ 1 file changed, 198 insertions(+) create mode 100644 doc/changesets/563-support-gherkin-feature-specification-documents.md diff --git a/doc/changesets/563-support-gherkin-feature-specification-documents.md b/doc/changesets/563-support-gherkin-feature-specification-documents.md new file mode 100644 index 000000000..6894a5f5c --- /dev/null +++ b/doc/changesets/563-support-gherkin-feature-specification-documents.md @@ -0,0 +1,198 @@ +# GH-563 Support Gherkin .feature Files As OpenFastTrace Specification Documents + +## Goal + +Allow OpenFastTrace to import specification items from Gherkin scenarios in `.feature` files while preserving the current behavior for legacy coverage-tag-only imports, based on a shared tag-parsing module that is reusable for GH-562 and GH-563. + +## Scope + +In scope: + +* Define and implement a two-PR approach where shared parser refactoring happens in a dedicated, smaller PR before GH-563 feature work. +* Parse `Scenario` and `Scenario Outline` blocks as OFT specification items in `.feature` files. +* Read one OFT specification ID from a dedicated Gherkin tag (`@id:`). +* Parse `# Covers:` and `# Needs:` metadata inside the defined metadata scope. +* Validate and report malformed or ambiguous Gherkin specification input. +* Keep existing tag importer behavior backward compatible. +* Add and update tests for both new behavior and regression coverage. +* Update user-facing documentation for the new syntax. + +Out of scope: + +* Changing the syntax or behavior of existing long and short coverage tags. +* Importing `Feature`, `Rule`, `Background`, or `Examples` as specification items. +* Adding new external parser dependencies. +* Bundling large parser refactoring and Gherkin feature behavior into one PR. + +## Design References + +* [System Requirements](../spec/system_requirements.md) +* [Design](../spec/design.md) +* [Quality Requirements](../spec/design/quality_requirements.md) +* [User Guide](../user_guide.md) + +## Strategy + +1. Create a separate refactoring PR first that extracts shared parsing logic into a new module under `importer/` (proposed module path: `importer/tag-importer-common`, artifact ID `openfasttrace-importer-tag-importer-common`). +2. Move reusable parsing components from `openfasttrace-importer-tag` into the shared module (ID parsing helpers, metadata token parsing, line/region scanning primitives, validation helpers). +3. Adopt the shared module in `openfasttrace-importer-tag` without behavior changes (pure refactoring and compatibility verification). +4. Implement GH-562 and GH-563 feature-specific parser logic in their own PRs on top of the shared module. +5. For GH-563, implement Gherkin scenario parsing as a deterministic state machine over lines so metadata scope and scenario boundaries are explicit and testable. +6. Do not forward all Gherkin lines into tag-regex parsing; forward only comment lines (and only when the line shape can contain OFT directives). +7. Enforce single-pass streaming parsing: each file is read once line-by-line, without full-file buffering. +8. Keep memory usage bounded to current line plus minimal parser state/context needed for scenario and metadata scope handling. +9. Add strict validation and clear error messages for missing, multiple, duplicate, and malformed OFT IDs and metadata entries. +10. Prove compatibility via regression tests for legacy `.feature` coverage tags and non-feature source files. + +## Concrete Refactoring Proposal + +### New Shared Module + +Create new module `importer/tag-importer-common` with artifact ID `openfasttrace-importer-tag-importer-common`. + +Primary purpose: + +* Provide reusable low-level parsing building blocks for importer implementations that parse OFT tags or OFT-like metadata in text files. +* Keep importer-specific behavior (tag importer vs. Gherkin importer) outside of this module. +* Support streaming importers that process files in one pass with bounded memory. + +### Proposed Packages And Classes + +Package `org.itsallcode.openfasttrace.importer.common.scan` + +* `LineScanner`: reads an `InputFile` line-by-line and emits `(lineNumber, lineContent)` events. +* `LineHandler`: functional interface for line event consumers. +* `CompositeLineHandler`: delegates one line event to multiple handlers in deterministic order. +* `FilteringLineHandler`: delegates only if `LinePredicate` matches (used to avoid unnecessary downstream regex work). +* `LinePredicates`: reusable predicates like `isCommentLine()`, `startsWithAnyPrefix(...)`. +* `StreamingParserContext`: mutable bounded context holder for current parser state (current scenario header, collected tags/metadata for open scenario, line number), explicitly excluding full-file text storage. + +Package `org.itsallcode.openfasttrace.importer.common.regex` + +* `RegexLineHandler`: base class for handlers that detect repeated regex matches in a line and call `processMatch(matcher, lineNumber, lineMatchCount)`. +* Responsibility split from current tag importer code: regex scanning mechanics are shared, concrete match interpretation stays in importer-specific subclasses. + +Package `org.itsallcode.openfasttrace.importer.common.oft` + +* `CoverageListParser`: parses comma-separated specification item IDs and returns validated `List`. +* `NeededTypesParser`: parses comma-separated needed artifact types and returns normalized `List`. +* `GherkinOftTagParser`: parses OFT ID tags in Gherkin syntax (`@id:`), validates exactly one ID token per scenario tag region. +* `MetadataDirectiveParser`: parses OFT directives in comments (`# Covers: ...`, `# Needs: ...`) and returns structured values. + +Package `org.itsallcode.openfasttrace.importer.common.validation` + +* `ImportValidationException`: common exception type for importer parsing/validation errors with location context. +* `ParserErrorMessages`: centralized message templates so tag importer and Gherkin importer report errors consistently. + +### Extraction Mapping From Existing Code + +Refactoring PR should extract or adapt the following existing pieces from `importer/tag` into the shared module: + +* `LineReader` -> `LineScanner` +* `LineReader.LineConsumer` -> `LineHandler` +* `DelegatingLineConsumer` -> `CompositeLineHandler` +* `AbstractRegexLineConsumer` -> `RegexLineHandler` +* `LongTagImportingLineConsumer.parseCoveredIds(...)` logic -> `CoverageListParser` +* `LongTagImportingLineConsumer.parseNeededArtifactTypes(...)` logic -> `NeededTypesParser` + +Keep these in `openfasttrace-importer-tag` (not shared): + +* `LongTagImportingLineConsumer` +* `ShortTagImportingLineConsumer` +* `TagImporter` +* `TagImporterFactory` +* `ChecksumCalculator` (tag importer specific ID-generation detail) + +### Responsibility Boundaries + +Shared module responsibilities: + +* Input scanning and regex match iteration mechanics. +* Generic OFT token and list parsing with validation and normalization. +* Reusable error abstraction and error message consistency. +* Preserve streaming semantics (single pass, bounded context state). + +Importer module responsibilities: + +* File type decisions and importer factory registration. +* Mapping parsed primitives to `ImportEventListener` events. +* Importer-specific semantics and state machines (tag grammar, Gherkin scenario boundaries). +* Routing optimization decisions, e.g. only forwarding comment lines to handlers that parse `Covers`/`Needs` directives. + +### PR-1 Refactoring Acceptance Criteria + +* `openfasttrace-importer-tag` compiles against `openfasttrace-importer-tag-importer-common`. +* Existing tag importer behavior is unchanged (prove through current unit tests). +* No new feature behavior is introduced in PR-1. +* Public API exposure is minimized to package-private where possible. +* Scanner and handler abstractions are usable in single-pass mode only; no API requires buffering complete file contents. + +### PR-2 And PR-3 Follow-Up + +* GH-562 PR: implement ticket-specific parsing behavior using shared module primitives. +* GH-563 PR: implement Gherkin scenario specification import using shared module primitives and dedicated scenario state machine. +* GH-563 PR: use `FilteringLineHandler` (or equivalent) so only comment lines are forwarded to OFT metadata/tag regex handlers. + +## Task List + +- [ ] Create and checkout branch `feature/563_support_gherkin_feature_specification_documents` + +### PR Split Proposal + +- [ ] Create a dedicated refactoring PR (separate from GH-563 and GH-562) that introduces `importer/tag-importer-common` (`openfasttrace-importer-tag-importer-common`) as a reusable parsing module. +- [ ] Limit the refactoring PR to code moves/extractions plus compatibility tests, with no functional behavior changes. +- [ ] Make both follow-up tickets depend on the refactoring PR: + - GH-562 consumes the shared parsing module. + - GH-563 consumes the shared parsing module. +- [ ] Merge the refactoring PR first, then continue with GH-562 and GH-563 feature PRs. + +### Requirements And Design + +- [ ] Update [doc/spec/system_requirements.md](../spec/system_requirements.md) with additive requirements for importing OFT specification items from Gherkin `Scenario` and `Scenario Outline`. +- [ ] Add explicit backward-compatibility requirement: existing `.feature` coverage-tag imports remain unchanged. +- [ ] Add validation requirements for missing ID, invalid ID, duplicate IDs, multiple IDs per scenario, malformed `# Covers:`, and malformed `# Needs:`. +- [ ] Stop and ask user for review of the updated system requirements. +- [ ] Update [doc/spec/design.md](../spec/design.md) with runtime design for the `.feature` parser flow, metadata scope boundaries, and error handling behavior. +- [ ] Add or update `dsn` items that cover the new and changed requirements. +- [ ] Stop and ask user for review of the updated design. + +### Implementation + +- [ ] Add `importer/tag-importer-common` module with reusable parser building blocks and register it in the multi-module Maven build. +- [ ] Add module dependency from `openfasttrace-importer-tag` to `openfasttrace-importer-tag-importer-common`. +- [ ] Add module dependency from the new Gherkin importer to `openfasttrace-importer-tag-importer-common`. +- [ ] Extract scanning infrastructure: move/adapt `LineReader` + nested consumer contract into `LineScanner`/`LineHandler`/`CompositeLineHandler` in the shared module. +- [ ] Extract regex scanning infrastructure: move/adapt `AbstractRegexLineConsumer` into shared `RegexLineHandler`. +- [ ] Extract reusable token parsers: move/adapt covered-ID and needed-types parsing into `CoverageListParser` and `NeededTypesParser`. +- [ ] Add shared parser support for Gherkin OFT tags and metadata directives (`GherkinOftTagParser`, `MetadataDirectiveParser`). +- [ ] In GH-563 implementation PR, add parser logic (using shared module components) to import specification items from Gherkin `Scenario` and `Scenario Outline` blocks. +- [ ] Restrict GH-563 specification-item import logic to `.feature` input while preserving existing tag parsing in all supported extensions. +- [ ] Add selective forwarding in GH-563 parser pipeline: only comment lines are passed to metadata/tag regex handlers. +- [ ] Ensure GH-563 parser pipeline remains single-pass: no second read over input, no buffering of full file content. +- [ ] Keep parser memory bounded to current line and minimal scenario/metadata state required to emit `ImportEventListener` events. +- [ ] Parse scenario title as specification item title and scenario steps as description. +- [ ] Parse metadata region between OFT ID tag and next boundary (next OFT ID tag, next scenario header, next feature header, or end of file). +- [ ] Keep non-OFT Gherkin tags and unrelated comments ignored. +- [ ] Keep old full/short coverage tag support behavior unchanged. + +### Verification + +- [ ] In the refactoring PR, run importer/tag unit tests as regression proof of no behavior changes. +- [ ] Add unit tests in `importer/tag` for valid `.feature` scenarios with `@id:`, `# Covers:`, and `# Needs:`. +- [ ] Add unit tests in `importer/tag` for invalid `.feature` inputs (missing/multiple/invalid IDs, malformed metadata, duplicate IDs). +- [ ] Add regression tests proving legacy coverage tag imports still behave unchanged in `.feature` files and non-`.feature` files. +- [ ] Add parser-pipeline tests proving non-comment lines are not forwarded to metadata/tag regex handlers. +- [ ] Add parser-pipeline tests proving one-pass behavior (line scanner invoked once, no re-read) and no full-file buffering. +- [ ] Add integration test coverage in `product` for mixed input artifacts that include both legacy tags and new Gherkin specification syntax. +- [ ] Run `./oft-self-trace.sh` and ensure trace stays clean. +- [ ] Run `mvn -T 1C verify` and ensure all quality gates pass. + +### Documentation + +- [ ] Extend [doc/user_guide.md](../user_guide.md) with the `.feature` specification syntax and examples. +- [ ] Update [.agents/skills/openfasttrace/SKILL.md](../../.agents/skills/openfasttrace/SKILL.md) to document the new `.feature` syntax (`@id:...`, `# Covers:`, `# Needs:`), boundaries, and backward-compatibility expectations. +- [ ] Add examples that show traceability links between Gherkin scenarios and design/implementation/test artifacts. + +### Version And Changelog + +- [ ] Add a changelog entry in [doc/changes/changes.md](../changes/changes.md) for GH-563. From 033833c1d66de92a1d6e69e3d42b94e05dbd2d42 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Wed, 22 Jul 2026 16:13:23 +0200 Subject: [PATCH 03/14] Update implementation plan --- ...gherkin-feature-specification-documents.md | 282 ++++++++---------- 1 file changed, 120 insertions(+), 162 deletions(-) diff --git a/doc/changesets/563-support-gherkin-feature-specification-documents.md b/doc/changesets/563-support-gherkin-feature-specification-documents.md index 6894a5f5c..1a5661722 100644 --- a/doc/changesets/563-support-gherkin-feature-specification-documents.md +++ b/doc/changesets/563-support-gherkin-feature-specification-documents.md @@ -1,28 +1,32 @@ -# GH-563 Support Gherkin .feature Files As OpenFastTrace Specification Documents +# GH-563 Support Gherkin `.feature` Files As OpenFastTrace Specification Documents ## Goal -Allow OpenFastTrace to import specification items from Gherkin scenarios in `.feature` files while preserving the current behavior for legacy coverage-tag-only imports, based on a shared tag-parsing module that is reusable for GH-562 and GH-563. +Allow OpenFastTrace to import specification items from Gherkin `Scenario` and +`Scenario Outline` blocks in `.feature` files. Preserve legacy coverage tags +when they are written in Gherkin comments, while avoiding coverage-tag regular +expression evaluation for executable Gherkin lines. ## Scope In scope: -* Define and implement a two-PR approach where shared parser refactoring happens in a dedicated, smaller PR before GH-563 feature work. -* Parse `Scenario` and `Scenario Outline` blocks as OFT specification items in `.feature` files. -* Read one OFT specification ID from a dedicated Gherkin tag (`@id:`). -* Parse `# Covers:` and `# Needs:` metadata inside the defined metadata scope. -* Validate and report malformed or ambiguous Gherkin specification input. -* Keep existing tag importer behavior backward compatible. -* Add and update tests for both new behavior and regression coverage. -* Update user-facing documentation for the new syntax. +* Extract shared line scanning and long/short coverage-tag parsing into a + prerequisite module. +* Add a dedicated Gherkin importer that composes the shared tag parser. +* Import OFT scenario items identified by `@id:`. +* Parse scoped `# Covers:` and `# Needs:` metadata strictly. +* Preserve comment-based legacy coverage tags in `.feature` files and all + existing tag importer behavior for non-`.feature` inputs. +* Update traced requirements, design, tests, and user documentation. Out of scope: -* Changing the syntax or behavior of existing long and short coverage tags. -* Importing `Feature`, `Rule`, `Background`, or `Examples` as specification items. -* Adding new external parser dependencies. -* Bundling large parser refactoring and Gherkin feature behavior into one PR. +* Importing `Feature`, `Rule`, `Background`, or `Examples` as specification + items. +* Supporting coverage tags in executable Gherkin lines. +* Adding an external Gherkin parser dependency. +* Moving Gherkin grammar or validation into the shared parser module. ## Design References @@ -33,166 +37,120 @@ Out of scope: ## Strategy -1. Create a separate refactoring PR first that extracts shared parsing logic into a new module under `importer/` (proposed module path: `importer/tag-importer-common`, artifact ID `openfasttrace-importer-tag-importer-common`). -2. Move reusable parsing components from `openfasttrace-importer-tag` into the shared module (ID parsing helpers, metadata token parsing, line/region scanning primitives, validation helpers). -3. Adopt the shared module in `openfasttrace-importer-tag` without behavior changes (pure refactoring and compatibility verification). -4. Implement GH-562 and GH-563 feature-specific parser logic in their own PRs on top of the shared module. -5. For GH-563, implement Gherkin scenario parsing as a deterministic state machine over lines so metadata scope and scenario boundaries are explicit and testable. -6. Do not forward all Gherkin lines into tag-regex parsing; forward only comment lines (and only when the line shape can contain OFT directives). -7. Enforce single-pass streaming parsing: each file is read once line-by-line, without full-file buffering. -8. Keep memory usage bounded to current line plus minimal parser state/context needed for scenario and metadata scope handling. -9. Add strict validation and clear error messages for missing, multiple, duplicate, and malformed OFT IDs and metadata entries. -10. Prove compatibility via regression tests for legacy `.feature` coverage tags and non-feature source files. - -## Concrete Refactoring Proposal - -### New Shared Module - -Create new module `importer/tag-importer-common` with artifact ID `openfasttrace-importer-tag-importer-common`. - -Primary purpose: - -* Provide reusable low-level parsing building blocks for importer implementations that parse OFT tags or OFT-like metadata in text files. -* Keep importer-specific behavior (tag importer vs. Gherkin importer) outside of this module. -* Support streaming importers that process files in one pass with bounded memory. - -### Proposed Packages And Classes - -Package `org.itsallcode.openfasttrace.importer.common.scan` - -* `LineScanner`: reads an `InputFile` line-by-line and emits `(lineNumber, lineContent)` events. -* `LineHandler`: functional interface for line event consumers. -* `CompositeLineHandler`: delegates one line event to multiple handlers in deterministic order. -* `FilteringLineHandler`: delegates only if `LinePredicate` matches (used to avoid unnecessary downstream regex work). -* `LinePredicates`: reusable predicates like `isCommentLine()`, `startsWithAnyPrefix(...)`. -* `StreamingParserContext`: mutable bounded context holder for current parser state (current scenario header, collected tags/metadata for open scenario, line number), explicitly excluding full-file text storage. - -Package `org.itsallcode.openfasttrace.importer.common.regex` - -* `RegexLineHandler`: base class for handlers that detect repeated regex matches in a line and call `processMatch(matcher, lineNumber, lineMatchCount)`. -* Responsibility split from current tag importer code: regex scanning mechanics are shared, concrete match interpretation stays in importer-specific subclasses. - -Package `org.itsallcode.openfasttrace.importer.common.oft` - -* `CoverageListParser`: parses comma-separated specification item IDs and returns validated `List`. -* `NeededTypesParser`: parses comma-separated needed artifact types and returns normalized `List`. -* `GherkinOftTagParser`: parses OFT ID tags in Gherkin syntax (`@id:`), validates exactly one ID token per scenario tag region. -* `MetadataDirectiveParser`: parses OFT directives in comments (`# Covers: ...`, `# Needs: ...`) and returns structured values. - -Package `org.itsallcode.openfasttrace.importer.common.validation` - -* `ImportValidationException`: common exception type for importer parsing/validation errors with location context. -* `ParserErrorMessages`: centralized message templates so tag importer and Gherkin importer report errors consistently. - -### Extraction Mapping From Existing Code - -Refactoring PR should extract or adapt the following existing pieces from `importer/tag` into the shared module: - -* `LineReader` -> `LineScanner` -* `LineReader.LineConsumer` -> `LineHandler` -* `DelegatingLineConsumer` -> `CompositeLineHandler` -* `AbstractRegexLineConsumer` -> `RegexLineHandler` -* `LongTagImportingLineConsumer.parseCoveredIds(...)` logic -> `CoverageListParser` -* `LongTagImportingLineConsumer.parseNeededArtifactTypes(...)` logic -> `NeededTypesParser` - -Keep these in `openfasttrace-importer-tag` (not shared): - -* `LongTagImportingLineConsumer` -* `ShortTagImportingLineConsumer` -* `TagImporter` -* `TagImporterFactory` -* `ChecksumCalculator` (tag importer specific ID-generation detail) - -### Responsibility Boundaries - -Shared module responsibilities: - -* Input scanning and regex match iteration mechanics. -* Generic OFT token and list parsing with validation and normalization. -* Reusable error abstraction and error message consistency. -* Preserve streaming semantics (single pass, bounded context state). - -Importer module responsibilities: - -* File type decisions and importer factory registration. -* Mapping parsed primitives to `ImportEventListener` events. -* Importer-specific semantics and state machines (tag grammar, Gherkin scenario boundaries). -* Routing optimization decisions, e.g. only forwarding comment lines to handlers that parse `Covers`/`Needs` directives. - -### PR-1 Refactoring Acceptance Criteria - -* `openfasttrace-importer-tag` compiles against `openfasttrace-importer-tag-importer-common`. -* Existing tag importer behavior is unchanged (prove through current unit tests). -* No new feature behavior is introduced in PR-1. -* Public API exposure is minimized to package-private where possible. -* Scanner and handler abstractions are usable in single-pass mode only; no API requires buffering complete file contents. - -### PR-2 And PR-3 Follow-Up - -* GH-562 PR: implement ticket-specific parsing behavior using shared module primitives. -* GH-563 PR: implement Gherkin scenario specification import using shared module primitives and dedicated scenario state machine. -* GH-563 PR: use `FilteringLineHandler` (or equivalent) so only comment lines are forwarded to OFT metadata/tag regex handlers. +1. Merge a behavior-preserving refactoring PR that introduces a shared module + for line scanning and long/short coverage-tag parsing. It retains `.feature` + support in the tag importer. +2. Atomically move `.feature` ownership from the tag importer to a new Gherkin + importer with higher precedence when the Gherkin importer is registered. +3. Implement Gherkin parsing as a single-pass state machine. It receives every + line but forwards only comment lines to the shared coverage-tag parser. +4. Keep all Gherkin syntax, state, validation, and `ImportEventListener` + mapping in the Gherkin importer. + +## Gherkin Syntax And Behavior + +* A Gherkin scenario is an OFT item only when its immediately preceding, + contiguous Gherkin tag region contains exactly one + `@id:` tag. Other Gherkin tags are ignored. +* Directives are recognized only in `#` comment lines after that ID tag region + and before the associated `Scenario:` or `Scenario Outline:` header. + Comments elsewhere are ignored by Gherkin metadata parsing. +* `# Covers:` and `# Needs:` are case-sensitive. Each is optional but may occur + at most once. When present, it must contain a non-empty comma-separated list; + duplicate values and malformed IDs or artifact types are errors. +* A repeated or invalid `@id` tag, or a scoped directive without exactly one + valid ID, fails the import with an `ImporterException` containing the file, + line, and reason. Scenarios without OFT metadata remain ignored. +* The scenario header line is the item location. Text after `Scenario:` or + `Scenario Outline:` is the title. The importer streams the scenario-step + block into the description, excluding comments and `Examples`; it ends the + item at the next `Scenario`, `Scenario Outline`, `Feature`, `Rule`, + `Background`, `Examples`, or end of file. +* The importer retains only active metadata and previously imported Gherkin IDs + for duplicate detection. It does not buffer a complete file or description. ## Task List -- [ ] Create and checkout branch `feature/563_support_gherkin_feature_specification_documents` - -### PR Split Proposal - -- [ ] Create a dedicated refactoring PR (separate from GH-563 and GH-562) that introduces `importer/tag-importer-common` (`openfasttrace-importer-tag-importer-common`) as a reusable parsing module. -- [ ] Limit the refactoring PR to code moves/extractions plus compatibility tests, with no functional behavior changes. -- [ ] Make both follow-up tickets depend on the refactoring PR: - - GH-562 consumes the shared parsing module. - - GH-563 consumes the shared parsing module. -- [ ] Merge the refactoring PR first, then continue with GH-562 and GH-563 feature PRs. +- [ ] Create and checkout branch + `feature/563_support_gherkin_feature_specification_documents`. + +### PR 1: Shared Coverage-Tag Parser Refactoring + +- [ ] Add `importer/tag-importer-common` with artifact ID + `openfasttrace-importer-tag-importer-common` to the Maven reactor. +- [ ] Add JPMS module `org.itsallcode.openfasttrace.importer.tag.common` and + export only `LineReader` (including its line-consumer contract) and + `CoverageTagParser` from + `org.itsallcode.openfasttrace.importer.tag.common`. +- [ ] Move line scanning, line-handler composition, regex matching, long/short + coverage-tag parsing, and CRC32 ID generation from `importer/tag` into + the shared module without changing parsing semantics, generated IDs, + listener events, logging, or exception wrapping. +- [ ] Define `CoverageTagParser.create(PathConfig, InputFile, + ImportEventListener)` to compose the long-tag parser and, when a path + configuration is present, the short-tag parser into one line consumer. + Keep parser implementation classes encapsulated. +- [ ] Refactor `openfasttrace-importer-tag` into a thin adapter that creates + the shared parser and scans its input once with the shared `LineReader`. + Keep all supported extensions, including `.feature`, unchanged in this + refactoring PR. +- [ ] Move scanner tests to the shared module and add focused shared-parser + tests for representative long and configured short tags, asserting + listener events, locations, generated IDs, coverage links, and needed + artifact types. +- [ ] Keep the existing tag-importer parsing and factory/configuration tests as + regression tests, including `.feature` support, to prove that the + refactoring is behavior-preserving. ### Requirements And Design -- [ ] Update [doc/spec/system_requirements.md](../spec/system_requirements.md) with additive requirements for importing OFT specification items from Gherkin `Scenario` and `Scenario Outline`. -- [ ] Add explicit backward-compatibility requirement: existing `.feature` coverage-tag imports remain unchanged. -- [ ] Add validation requirements for missing ID, invalid ID, duplicate IDs, multiple IDs per scenario, malformed `# Covers:`, and malformed `# Needs:`. +- [ ] Add requirements for importing Gherkin scenarios and outlines, strict + scoped metadata validation, Gherkin importer selection, and comment-only + legacy coverage-tag compatibility. - [ ] Stop and ask user for review of the updated system requirements. -- [ ] Update [doc/spec/design.md](../spec/design.md) with runtime design for the `.feature` parser flow, metadata scope boundaries, and error handling behavior. -- [ ] Add or update `dsn` items that cover the new and changed requirements. +- [ ] Add design items for factory precedence, the streaming Gherkin state + machine, metadata scope, event mapping, and shared-parser delegation. - [ ] Stop and ask user for review of the updated design. -### Implementation - -- [ ] Add `importer/tag-importer-common` module with reusable parser building blocks and register it in the multi-module Maven build. -- [ ] Add module dependency from `openfasttrace-importer-tag` to `openfasttrace-importer-tag-importer-common`. -- [ ] Add module dependency from the new Gherkin importer to `openfasttrace-importer-tag-importer-common`. -- [ ] Extract scanning infrastructure: move/adapt `LineReader` + nested consumer contract into `LineScanner`/`LineHandler`/`CompositeLineHandler` in the shared module. -- [ ] Extract regex scanning infrastructure: move/adapt `AbstractRegexLineConsumer` into shared `RegexLineHandler`. -- [ ] Extract reusable token parsers: move/adapt covered-ID and needed-types parsing into `CoverageListParser` and `NeededTypesParser`. -- [ ] Add shared parser support for Gherkin OFT tags and metadata directives (`GherkinOftTagParser`, `MetadataDirectiveParser`). -- [ ] In GH-563 implementation PR, add parser logic (using shared module components) to import specification items from Gherkin `Scenario` and `Scenario Outline` blocks. -- [ ] Restrict GH-563 specification-item import logic to `.feature` input while preserving existing tag parsing in all supported extensions. -- [ ] Add selective forwarding in GH-563 parser pipeline: only comment lines are passed to metadata/tag regex handlers. -- [ ] Ensure GH-563 parser pipeline remains single-pass: no second read over input, no buffering of full file content. -- [ ] Keep parser memory bounded to current line and minimal scenario/metadata state required to emit `ImportEventListener` events. -- [ ] Parse scenario title as specification item title and scenario steps as description. -- [ ] Parse metadata region between OFT ID tag and next boundary (next OFT ID tag, next scenario header, next feature header, or end of file). -- [ ] Keep non-OFT Gherkin tags and unrelated comments ignored. -- [ ] Keep old full/short coverage tag support behavior unchanged. +### PR 2: Gherkin Importer + +- [ ] Add `importer/gherkin` with artifact ID + `openfasttrace-importer-gherkin`; register it in the Maven reactor and + product dependencies. +- [ ] Provide a Gherkin importer factory for `.feature` files with priority + `9000`, ahead of the tag importer's priority `10000`. +- [ ] Implement the defined single-pass Gherkin state machine and map imported + scenario fragments to `ImportEventListener` events. +- [ ] Inject the shared coverage-tag parser and forward only lines whose + trimmed form starts with `#` to it. +- [ ] Implement the specified `@id:`, `Covers`, `Needs`, title, description, + boundary, duplicate-ID, and error behavior. +- [ ] Preserve the shared parser's existing `ImporterException` behavior for + legacy coverage tags; do not introduce a new shared validation exception. ### Verification -- [ ] In the refactoring PR, run importer/tag unit tests as regression proof of no behavior changes. -- [ ] Add unit tests in `importer/tag` for valid `.feature` scenarios with `@id:`, `# Covers:`, and `# Needs:`. -- [ ] Add unit tests in `importer/tag` for invalid `.feature` inputs (missing/multiple/invalid IDs, malformed metadata, duplicate IDs). -- [ ] Add regression tests proving legacy coverage tag imports still behave unchanged in `.feature` files and non-`.feature` files. -- [ ] Add parser-pipeline tests proving non-comment lines are not forwarded to metadata/tag regex handlers. -- [ ] Add parser-pipeline tests proving one-pass behavior (line scanner invoked once, no re-read) and no full-file buffering. -- [ ] Add integration test coverage in `product` for mixed input artifacts that include both legacy tags and new Gherkin specification syntax. -- [ ] Run `./oft-self-trace.sh` and ensure trace stays clean. +- [ ] Add Gherkin importer unit tests for valid scenarios and outlines, + location/title/description extraction, coverage metadata, non-OFT tags, + and ignored ordinary scenarios. +- [ ] Add validation tests for invalid or multiple IDs, orphan directives, + repeated or empty directives, malformed list entries, duplicate metadata + values, and duplicate Gherkin IDs. Assert exception type and relevant + message content. +- [ ] Add regression tests proving comment coverage tags import in `.feature` + files, non-comment coverage-tag text is ignored in `.feature` files, and + tag importer behavior for non-`.feature` inputs is unchanged. +- [ ] Add pipeline tests proving each Gherkin file is scanned once and only + comment lines reach the shared coverage-tag parser. +- [ ] Add product-level tests for Gherkin importer precedence and mixed + scenario specifications with comment-based legacy coverage tags. +- [ ] Run `./oft-self-trace.sh` and ensure the trace stays clean. - [ ] Run `mvn -T 1C verify` and ensure all quality gates pass. -### Documentation - -- [ ] Extend [doc/user_guide.md](../user_guide.md) with the `.feature` specification syntax and examples. -- [ ] Update [.agents/skills/openfasttrace/SKILL.md](../../.agents/skills/openfasttrace/SKILL.md) to document the new `.feature` syntax (`@id:...`, `# Covers:`, `# Needs:`), boundaries, and backward-compatibility expectations. -- [ ] Add examples that show traceability links between Gherkin scenarios and design/implementation/test artifacts. - -### Version And Changelog +### Documentation And Changelog -- [ ] Add a changelog entry in [doc/changes/changes.md](../changes/changes.md) for GH-563. +- [ ] Extend [doc/user_guide.md](../user_guide.md) with the `.feature` syntax, + placement rules, validation behavior, and examples. +- [ ] Update [.agents/skills/openfasttrace/SKILL.md](../../.agents/skills/openfasttrace/SKILL.md) + with the Gherkin syntax and comment-only compatibility rule. +- [ ] Add the GH-563 entry to [doc/changes/changes_4.6.0.md](../changes/changes_4.6.0.md). From 1f110538d8fd33419fdde8b2ee3d7cbb33c65abf Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 07:53:20 +0200 Subject: [PATCH 04/14] #563: Refactoring: extract tag parser into separate module --- importer/tag-importer-common/pom.xml | 27 +++++ .../src/main/java/module-info.java | 9 ++ .../tag/common/AbstractRegexLineConsumer.java | 30 ++++++ .../tag/common}/ChecksumCalculator.java | 2 +- .../tag/common/CoverageTagParser.java | 39 ++++++++ .../tag/common}/DelegatingLineConsumer.java | 5 +- .../importer/tag/common/LineReader.java | 77 +++++++++++++++ .../common}/LongTagImportingLineConsumer.java | 99 +++++++------------ .../ShortTagImportingLineConsumer.java | 66 +++++-------- .../tag/common}/TestChecksumCalculator.java | 3 +- .../tag/common/TestCoverageTagParser.java | 68 +++++++++++++ .../importer/tag/common}/TestLineReader.java | 58 ++++------- importer/tag/pom.xml | 4 + importer/tag/src/main/java/module-info.java | 4 +- .../tag/AbstractRegexLineConsumer.java | 46 --------- .../importer/tag/LineReader.java | 70 ------------- .../importer/tag/TagImporter.java | 22 +---- oft-self-trace.sh | 1 + parent/pom.xml | 6 ++ pom.xml | 1 + 20 files changed, 350 insertions(+), 287 deletions(-) create mode 100644 importer/tag-importer-common/pom.xml create mode 100644 importer/tag-importer-common/src/main/java/module-info.java create mode 100644 importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/AbstractRegexLineConsumer.java rename importer/{tag/src/main/java/org/itsallcode/openfasttrace/importer/tag => tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common}/ChecksumCalculator.java (87%) create mode 100644 importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/CoverageTagParser.java rename importer/{tag/src/main/java/org/itsallcode/openfasttrace/importer/tag => tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common}/DelegatingLineConsumer.java (77%) create mode 100644 importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LineReader.java rename importer/{tag/src/main/java/org/itsallcode/openfasttrace/importer/tag => tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common}/LongTagImportingLineConsumer.java (77%) rename importer/{tag/src/main/java/org/itsallcode/openfasttrace/importer/tag => tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common}/ShortTagImportingLineConsumer.java (70%) rename importer/{tag/src/test/java/org/itsallcode/openfasttrace/importer/tag => tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common}/TestChecksumCalculator.java (91%) create mode 100644 importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestCoverageTagParser.java rename importer/{tag/src/test/java/org/itsallcode/openfasttrace/importer/tag => tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common}/TestLineReader.java (69%) delete mode 100644 importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/AbstractRegexLineConsumer.java delete mode 100644 importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LineReader.java diff --git a/importer/tag-importer-common/pom.xml b/importer/tag-importer-common/pom.xml new file mode 100644 index 000000000..e2ec81236 --- /dev/null +++ b/importer/tag-importer-common/pom.xml @@ -0,0 +1,27 @@ + + 4.0.0 + openfasttrace-importer-tag-importer-common + OpenFastTrace Tag Importer Common + + ../../openfasttrace-mc-deployable-parent/pom.xml + org.itsallcode.openfasttrace + openfasttrace-mc-deployable-parent + ${revision} + + + ${reproducible.build.timestamp} + + + + org.itsallcode.openfasttrace + openfasttrace-api + + + org.itsallcode.openfasttrace + openfasttrace-testutil + test + + + diff --git a/importer/tag-importer-common/src/main/java/module-info.java b/importer/tag-importer-common/src/main/java/module-info.java new file mode 100644 index 000000000..edaf5fa73 --- /dev/null +++ b/importer/tag-importer-common/src/main/java/module-info.java @@ -0,0 +1,9 @@ +/** + * Shared line scanning and coverage-tag parsing support for importers. + */ +module org.itsallcode.openfasttrace.importer.tag.common { + requires java.logging; + requires transitive org.itsallcode.openfasttrace.api; + + exports org.itsallcode.openfasttrace.importer.tag.common; +} diff --git a/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/AbstractRegexLineConsumer.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/AbstractRegexLineConsumer.java new file mode 100644 index 000000000..da70364b8 --- /dev/null +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/AbstractRegexLineConsumer.java @@ -0,0 +1,30 @@ +package org.itsallcode.openfasttrace.importer.tag.common; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.itsallcode.openfasttrace.importer.tag.common.LineReader.LineConsumer; + +abstract class AbstractRegexLineConsumer implements LineConsumer { + private final Pattern pattern; + + AbstractRegexLineConsumer(final String patternRegex) { + this(Pattern.compile(patternRegex)); + } + + private AbstractRegexLineConsumer(final Pattern pattern) { + this.pattern = pattern; + } + + @Override + public void readLine(final int lineNumber, final String line) { + final Matcher matcher = this.pattern.matcher(line); + int counter = 0; + while (matcher.find()) { + processMatch(matcher, lineNumber, counter); + counter++; + } + } + + abstract void processMatch(Matcher matcher, int lineNumber, int lineMatchCount); +} diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/ChecksumCalculator.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/ChecksumCalculator.java similarity index 87% rename from importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/ChecksumCalculator.java rename to importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/ChecksumCalculator.java index b9619019c..ee99f3c2f 100644 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/ChecksumCalculator.java +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/ChecksumCalculator.java @@ -1,4 +1,4 @@ -package org.itsallcode.openfasttrace.importer.tag; +package org.itsallcode.openfasttrace.importer.tag.common; import java.nio.charset.StandardCharsets; import java.util.zip.CRC32; diff --git a/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/CoverageTagParser.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/CoverageTagParser.java new file mode 100644 index 000000000..34951b722 --- /dev/null +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/CoverageTagParser.java @@ -0,0 +1,39 @@ +package org.itsallcode.openfasttrace.importer.tag.common; + +import java.util.ArrayList; +import java.util.List; + +import org.itsallcode.openfasttrace.api.importer.ImportEventListener; +import org.itsallcode.openfasttrace.api.importer.input.InputFile; +import org.itsallcode.openfasttrace.api.importer.tag.config.PathConfig; +import org.itsallcode.openfasttrace.importer.tag.common.LineReader.LineConsumer; + +/** + * Creates line consumers that import full and configured short coverage tags. + */ +public final class CoverageTagParser { + private CoverageTagParser() { + // Prevent instantiation. + } + + /** + * Create a line consumer for coverage tags in an input file. + * + * @param config + * optional configuration for short coverage tags + * @param file + * input file that contains the tags + * @param listener + * listener receiving imported specification items + * @return line consumer that imports full and configured short coverage tags + */ + public static LineConsumer create(final PathConfig config, final InputFile file, + final ImportEventListener listener) { + final List parsers = new ArrayList<>(); + parsers.add(new LongTagImportingLineConsumer(file, listener)); + if (config != null) { + parsers.add(new ShortTagImportingLineConsumer(config, file, listener)); + } + return new DelegatingLineConsumer(parsers); + } +} diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/DelegatingLineConsumer.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/DelegatingLineConsumer.java similarity index 77% rename from importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/DelegatingLineConsumer.java rename to importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/DelegatingLineConsumer.java index da7a2ed1b..0def4d7ce 100644 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/DelegatingLineConsumer.java +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/DelegatingLineConsumer.java @@ -1,8 +1,9 @@ -package org.itsallcode.openfasttrace.importer.tag; +package org.itsallcode.openfasttrace.importer.tag.common; + import java.util.Collections; import java.util.List; -import org.itsallcode.openfasttrace.importer.tag.LineReader.LineConsumer; +import org.itsallcode.openfasttrace.importer.tag.common.LineReader.LineConsumer; class DelegatingLineConsumer implements LineConsumer { diff --git a/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LineReader.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LineReader.java new file mode 100644 index 000000000..cfb1dec4a --- /dev/null +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LineReader.java @@ -0,0 +1,77 @@ +package org.itsallcode.openfasttrace.importer.tag.common; + +import java.io.IOException; +import java.io.LineNumberReader; + +import org.itsallcode.openfasttrace.api.importer.ImporterException; +import org.itsallcode.openfasttrace.api.importer.input.InputFile; + +/** + * Reads an input file line by line and forwards every line to a consumer. + */ +public class LineReader { + private final InputFile file; + + private LineReader(final InputFile file) { + this.file = file; + } + + /** + * Create a reader for an input file. + * + * @param file + * input file to read + * @return line reader for the input file + */ + public static LineReader create(final InputFile file) { + return new LineReader(file); + } + + /** + * Read the input file and forward each line to the consumer. + * + * @param consumer + * consumer receiving line numbers and line content + * @throws ImporterException + * if the input cannot be read or a line cannot be + * processed + */ + public void readLines(final LineConsumer consumer) { + int currentLineNumber = 0; + try (final LineNumberReader reader = new LineNumberReader(this.file.createReader())) { + String line; + while ((line = reader.readLine()) != null) { + currentLineNumber = reader.getLineNumber(); + processLine(consumer, currentLineNumber, line); + } + } catch (final IOException exception) { + throw new ImporterException("Error reading \"" + this.file + "\" at line " + currentLineNumber, exception); + } + } + + private void processLine(final LineConsumer consumer, final int currentLineNumber, + final String line) { + try { + consumer.readLine(currentLineNumber, line); + } catch (final RuntimeException exception) { + throw new ImporterException("Error processing line " + this.file.getPath() + ":" + currentLineNumber + " '" + + line + "': " + exception, exception); + } + } + + /** + * Receives a line read from an input file. + */ + @FunctionalInterface + public interface LineConsumer { + /** + * Process a single input line. + * + * @param lineNumber + * line number, starting at {@code 1} + * @param line + * line content without its line separator + */ + void readLine(int lineNumber, String line); + } +} diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LongTagImportingLineConsumer.java similarity index 77% rename from importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java rename to importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LongTagImportingLineConsumer.java index 16c7d7939..b4aaebd2a 100644 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LongTagImportingLineConsumer.java +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LongTagImportingLineConsumer.java @@ -1,10 +1,8 @@ -package org.itsallcode.openfasttrace.importer.tag; +package org.itsallcode.openfasttrace.importer.tag.common; import static java.util.Collections.emptyList; -import java.util.Arrays; -import java.util.List; -import java.util.Optional; +import java.util.*; import java.util.function.Predicate; import java.util.logging.Logger; import java.util.regex.Matcher; @@ -16,8 +14,7 @@ // [impl->dsn~import.full-coverage-tag~1] // [impl->dsn~import.full-coverage-tag-with-needed-coverage~1] // [impl->dsn~import.full-coverage-tag-multiple-needed-coverage~1] -class LongTagImportingLineConsumer extends AbstractRegexLineConsumer -{ +class LongTagImportingLineConsumer extends AbstractRegexLineConsumer { private static final Logger LOG = Logger .getLogger(LongTagImportingLineConsumer.class.getName()); @@ -32,54 +29,46 @@ class LongTagImportingLineConsumer extends AbstractRegexLineConsumer private static final String NEEDS_COVERAGE = ">>" + OPTIONAL_WHITESPACE + "(?\\p{Alpha}+(?:" + OPTIONAL_WHITESPACE + "," + OPTIONAL_WHITESPACE + "\\p{Alpha}+)*)"; - private static final String TAG_REGEX = TAG_PREFIX + OPTIONAL_WHITESPACE// + private static final String TAG_REGEX = TAG_PREFIX + OPTIONAL_WHITESPACE + "(?" + COVERING_ARTIFACT_TYPE_PATTERN + ")" + "(?:" + SpecificationItemId.ARTIFACT_TYPE_SEPARATOR // [impl->dsn~import.full-coverage-tag-with-name-and-revision~1] + "(?" + SpecificationItemId.ITEM_NAME_PATTERN + ")?" + SpecificationItemId.REVISION_SEPARATOR - + "(?" + SpecificationItemId.ITEM_REVISION_PATTERN + "))?" // - + OPTIONAL_WHITESPACE + "->" + OPTIONAL_WHITESPACE // - + "(?" + COVERED_IDS + ")" // - + OPTIONAL_WHITESPACE + "(?:" + NEEDS_COVERAGE + OPTIONAL_WHITESPACE + ")?" // + + "(?" + SpecificationItemId.ITEM_REVISION_PATTERN + "))?" + + OPTIONAL_WHITESPACE + "->" + OPTIONAL_WHITESPACE + + "(?" + COVERED_IDS + ")" + + OPTIONAL_WHITESPACE + "(?:" + NEEDS_COVERAGE + OPTIONAL_WHITESPACE + ")?" + TAG_SUFFIX; private final InputFile file; private final ImportEventListener listener; - LongTagImportingLineConsumer(final InputFile file, final ImportEventListener listener) - { + LongTagImportingLineConsumer(final InputFile file, final ImportEventListener listener) { super(TAG_REGEX); this.file = file; this.listener = listener; } @Override - public void processMatch(final Matcher matcher, final int lineNumber, final int lineMatchCount) - { + public void processMatch(final Matcher matcher, final int lineNumber, final int lineMatchCount) { final List coveredIds = parseCoveredIds(matcher.group("coveredIds")); final List neededArtifactTypes = parseNeededArtifactTypes(matcher.group("neededArtifactTypes")); - final List generatedIds = createItemIds(matcher, lineNumber, lineMatchCount, coveredIds, neededArtifactTypes); - if (generatedIds.size() > 1) - { + if (generatedIds.size() > 1) { assert generatedIds.size() == coveredIds.size(); - for (int i = 0; i < generatedIds.size(); i++) - { + for (int i = 0; i < generatedIds.size(); i++) { addSpecificationItem(lineNumber, generatedIds.get(i), List.of(coveredIds.get(i)), neededArtifactTypes); } - } - else - { + } else { addSpecificationItem(lineNumber, generatedIds.get(0), coveredIds, neededArtifactTypes); } } private void addSpecificationItem(final int lineNumber, final SpecificationItemId generatedId, - final List coveredIds, final List neededArtifactTypes) - { + final List coveredIds, final List neededArtifactTypes) { this.listener.beginSpecificationItem(); this.listener.setLocation(this.file.getPath(), lineNumber); this.listener.setId(generatedId); @@ -89,10 +78,8 @@ private void addSpecificationItem(final int lineNumber, final SpecificationItemI logItem(lineNumber, coveredIds, neededArtifactTypes, generatedId); } - private static List parseCoveredIds(final String input) - { - if (input == null) - { + private static List parseCoveredIds(final String input) { + if (input == null) { return emptyList(); } return Arrays.stream(input.split(",")) @@ -102,13 +89,10 @@ private static List parseCoveredIds(final String input) .toList(); } - private static List parseNeededArtifactTypes(final String input) - { - if (input == null) - { + private static List parseNeededArtifactTypes(final String input) { + if (input == null) { return emptyList(); } - return Arrays.stream(input.split(",")) .map(String::trim) .filter(Predicate.not(String::isEmpty)) @@ -116,20 +100,17 @@ private static List parseNeededArtifactTypes(final String input) } private List createItemIds(final Matcher matcher, final int lineNumber, - final int lineMatchCount, - final List coveredIds, final List neededArtifactTypes) - { + final int lineMatchCount, final List coveredIds, + final List neededArtifactTypes) { final String artifactType = matcher.group("artifactType"); final String customName = matcher.group("customName"); final String revision = matcher.group("revision"); - if (customName != null) - { + if (customName != null) { return List.of(SpecificationItemId.createId(artifactType, customName, parseRevision(revision))); } final List result = new java.util.ArrayList<>(coveredIds.size()); - for (final SpecificationItemId coveredId : coveredIds) - { + for (final SpecificationItemId coveredId : coveredIds) { final String name = getItemName(lineNumber, lineMatchCount, coveredId, neededArtifactTypes); result.add(SpecificationItemId.createId(artifactType, name, parseRevision(revision))); } @@ -137,42 +118,30 @@ private List createItemIds(final Matcher matcher, final int } private void logItem(final int lineNumber, final List coveredIds, - final List neededArtifactTypes, final SpecificationItemId generatedId) - { - if (neededArtifactTypes.isEmpty()) - { - LOG.finest(() -> "File " + this.file + ":" + lineNumber + ": found '" + generatedId - + "' covering IDs " + coveredIds); - } - else - { - LOG.finest(() -> "File " + this.file + ":" + lineNumber + ": found '" + generatedId - + "' covering IDs " + coveredIds + ", needs artifact types " - + neededArtifactTypes); + final List neededArtifactTypes, final SpecificationItemId generatedId) { + if (neededArtifactTypes.isEmpty()) { + LOG.finest(() -> "File " + this.file + ":" + lineNumber + ": found '" + generatedId + "' covering IDs " + + coveredIds); + } else { + LOG.finest(() -> "File " + this.file + ":" + lineNumber + ": found '" + generatedId + "' covering IDs " + + coveredIds + ", needs artifact types " + neededArtifactTypes); } } - private static int parseRevision(final String revision) - { - return Optional.ofNullable(revision) - .map(Integer::parseInt) - .orElse(0); + private static int parseRevision(final String revision) { + return Optional.ofNullable(revision).map(Integer::parseInt).orElse(0); } // [impl->dsn~import.full-coverage-tag-with-needed-coverage-readable-names~1] private String getItemName(final int lineNumber, final int lineMatchCount, final SpecificationItemId coveredId, - final List neededArtifactTypes) - { - if (neededArtifactTypes.isEmpty()) - { + final List neededArtifactTypes) { + if (neededArtifactTypes.isEmpty()) { return generateUniqueName(coveredId, lineNumber, lineMatchCount); } return coveredId.getName(); } - private String generateUniqueName(final SpecificationItemId coveredId, final int lineNumber, - final int counter) - { + private String generateUniqueName(final SpecificationItemId coveredId, final int lineNumber, final int counter) { final String uniqueName = this.file.getPath() + lineNumber + counter + coveredId; final String checksum = Long.toString(ChecksumCalculator.calculateCrc32(uniqueName)); return coveredId.getName() + "-" + checksum; diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/ShortTagImportingLineConsumer.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/ShortTagImportingLineConsumer.java similarity index 70% rename from importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/ShortTagImportingLineConsumer.java rename to importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/ShortTagImportingLineConsumer.java index 428ab7dae..52c7877a9 100644 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/ShortTagImportingLineConsumer.java +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/ShortTagImportingLineConsumer.java @@ -1,4 +1,4 @@ -package org.itsallcode.openfasttrace.importer.tag; +package org.itsallcode.openfasttrace.importer.tag.common; import java.util.logging.Logger; import java.util.regex.Matcher; @@ -10,14 +10,13 @@ import org.itsallcode.openfasttrace.api.importer.tag.config.PathConfig; // [impl->dsn~import.short-coverage-tag~1] -class ShortTagImportingLineConsumer extends AbstractRegexLineConsumer -{ +class ShortTagImportingLineConsumer extends AbstractRegexLineConsumer { private static final Logger LOG = Logger.getLogger(ShortTagImportingLineConsumer.class.getName()); private static final String TAG_PREFIX = "\\[\\["; private static final String TAG_SUFFIX = "\\]\\]"; - private static final String TAG_REGEX = TAG_PREFIX // - + SpecificationItemId.ITEM_NAME_PATTERN // + private static final String TAG_REGEX = TAG_PREFIX + + SpecificationItemId.ITEM_NAME_PATTERN + ":" // + "(\\w+)" // + TAG_SUFFIX; @@ -27,8 +26,7 @@ class ShortTagImportingLineConsumer extends AbstractRegexLineConsumer private final InputFile file; ShortTagImportingLineConsumer(final PathConfig pathConfig, final InputFile file, - final ImportEventListener listener) - { + final ImportEventListener listener) { super(TAG_REGEX); this.pathConfig = pathConfig; this.file = file; @@ -36,26 +34,21 @@ class ShortTagImportingLineConsumer extends AbstractRegexLineConsumer } @Override - void processMatch(final Matcher matcher, final int lineNumber, final int lineMatchCount) - { + void processMatch(final Matcher matcher, final int lineNumber, final int lineMatchCount) { final String coveredItemName = matcher.group(1); final String coveredItemRevision = matcher.group(2); - final SpecificationItemId coveredId = createCoveredItem(coveredItemName, - coveredItemRevision); - + final SpecificationItemId coveredId = createCoveredItem(coveredItemName, coveredItemRevision); final String generatedName = generateName(coveredId, lineNumber, lineMatchCount); - final SpecificationItemId tagItemId = SpecificationItemId - .createId(this.pathConfig.getTagArtifactType(), generatedName); - - LOG.finest(() -> "File " + this.file + ":" + lineNumber + ": found '" + tagItemId - + "' covering id '" + coveredId + "'"); + final SpecificationItemId tagItemId = SpecificationItemId.createId(this.pathConfig.getTagArtifactType(), + generatedName); + LOG.finest(() -> "File " + this.file + ":" + lineNumber + ": found '" + tagItemId + "' covering id '" + + coveredId + "'"); addItem(lineNumber, coveredId, tagItemId); } private void addItem(final int lineNumber, final SpecificationItemId coveredId, - final SpecificationItemId tagItemId) - { + final SpecificationItemId tagItemId) { this.listener.beginSpecificationItem(); this.listener.setLocation(this.file.toString(), lineNumber); this.listener.setId(tagItemId); @@ -63,39 +56,28 @@ private void addItem(final int lineNumber, final SpecificationItemId coveredId, this.listener.endSpecificationItem(); } - private SpecificationItemId createCoveredItem(final String name, final String revision) - { + private SpecificationItemId createCoveredItem(final String name, final String revision) { final int parsedRevision = parseRevision(name, revision); final String nameWithPrefix = getCoveredItemNamePrefix() + name; - return SpecificationItemId.createId(this.pathConfig.getCoveredItemArtifactType(), - nameWithPrefix, parsedRevision); + return SpecificationItemId.createId(this.pathConfig.getCoveredItemArtifactType(), nameWithPrefix, + parsedRevision); } - private static int parseRevision(final String name, final String revision) - { - try - { + private static int parseRevision(final String name, final String revision) { + try { return Integer.parseInt(revision); - } - catch (final NumberFormatException e) - { - throw new ImporterException( - "Error parsing revision '" + revision + "' for item '" + name + "'.", e); + } catch (final NumberFormatException exception) { + throw new ImporterException("Error parsing revision '" + revision + "' for item '" + name + "'.", + exception); } } - private String getCoveredItemNamePrefix() - { - return this.pathConfig.getCoveredItemNamePrefix() != null - ? this.pathConfig.getCoveredItemNamePrefix() - : ""; + private String getCoveredItemNamePrefix() { + return this.pathConfig.getCoveredItemNamePrefix() != null ? this.pathConfig.getCoveredItemNamePrefix() : ""; } - private String generateName(final SpecificationItemId coveredId, final int lineNumber, - final int counter) - { - final String uniqueName = this.file.toString() + lineNumber + counter - + coveredId.toString(); + private String generateName(final SpecificationItemId coveredId, final int lineNumber, final int counter) { + final String uniqueName = this.file.toString() + lineNumber + counter + coveredId; final String checksum = Long.toString(ChecksumCalculator.calculateCrc32(uniqueName)); return coveredId.getName() + "-" + checksum; } diff --git a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestChecksumCalculator.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestChecksumCalculator.java similarity index 91% rename from importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestChecksumCalculator.java rename to importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestChecksumCalculator.java index baacdc036..07c8be79c 100644 --- a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestChecksumCalculator.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestChecksumCalculator.java @@ -1,7 +1,6 @@ -package org.itsallcode.openfasttrace.importer.tag; +package org.itsallcode.openfasttrace.importer.tag.common; import static org.hamcrest.MatcherAssert.assertThat; - import static org.hamcrest.Matchers.equalTo; import org.junit.jupiter.api.Test; diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestCoverageTagParser.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestCoverageTagParser.java new file mode 100644 index 000000000..3e77b197e --- /dev/null +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestCoverageTagParser.java @@ -0,0 +1,68 @@ +package org.itsallcode.openfasttrace.importer.tag.common; + +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.BufferedReader; +import java.io.StringReader; +import java.nio.file.Paths; + +import org.itsallcode.openfasttrace.api.core.SpecificationItemId; +import org.itsallcode.openfasttrace.api.importer.ImportEventListener; +import org.itsallcode.openfasttrace.api.importer.input.InputFile; +import org.itsallcode.openfasttrace.api.importer.tag.config.PathConfig; +import org.itsallcode.openfasttrace.importer.tag.common.LineReader.LineConsumer; +import org.itsallcode.openfasttrace.testutil.importer.input.StreamInput; +import org.junit.jupiter.api.Test; +import org.mockito.InOrder; + +class TestCoverageTagParser +{ + private static final String FILE = "source.file"; + + @Test + void testImportsFullTag() + { + final ImportEventListener listener = mock(ImportEventListener.class); + final LineConsumer parser = CoverageTagParser.create(null, inputFile(), listener); + + final String coverageTag = "[" + "impl~name~1->dsn~covered~2>>utest]"; + parser.readLine(3, coverageTag); + + final InOrder inOrder = inOrder(listener); + inOrder.verify(listener).beginSpecificationItem(); + inOrder.verify(listener).setLocation(FILE, 3); + inOrder.verify(listener).setId(SpecificationItemId.parseId("impl~name~1")); + inOrder.verify(listener).addCoveredId(SpecificationItemId.parseId("dsn~covered~2")); + inOrder.verify(listener).addNeededArtifactType("utest"); + inOrder.verify(listener).endSpecificationItem(); + inOrder.verifyNoMoreInteractions(); + } + + @Test + void testImportsConfiguredShortTag() + { + final PathConfig config = mock(PathConfig.class); + when(config.getCoveredItemArtifactType()).thenReturn("req"); + when(config.getCoveredItemNamePrefix()).thenReturn("prefix."); + when(config.getTagArtifactType()).thenReturn("utest"); + final ImportEventListener listener = mock(ImportEventListener.class); + final LineConsumer parser = CoverageTagParser.create(config, inputFile(), listener); + + parser.readLine(2, "[[covered:3]]"); + + final InOrder inOrder = inOrder(listener); + inOrder.verify(listener).beginSpecificationItem(); + inOrder.verify(listener).setLocation(FILE, 2); + inOrder.verify(listener).setId(SpecificationItemId.createId("utest", "prefix.covered-1743877134")); + inOrder.verify(listener).addCoveredId(SpecificationItemId.createId("req", "prefix.covered", 3)); + inOrder.verify(listener).endSpecificationItem(); + inOrder.verifyNoMoreInteractions(); + } + + private static InputFile inputFile() + { + return StreamInput.forReader(Paths.get(FILE), new BufferedReader(new StringReader(""))); + } +} diff --git a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestLineReader.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java similarity index 69% rename from importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestLineReader.java rename to importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java index 93fc02a84..33c629607 100644 --- a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestLineReader.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java @@ -1,17 +1,14 @@ -package org.itsallcode.openfasttrace.importer.tag; +package org.itsallcode.openfasttrace.importer.tag.common; + import static org.mockito.Mockito.inOrder; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.StringReader; +import java.io.*; import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; +import java.nio.file.*; import org.itsallcode.openfasttrace.api.importer.input.InputFile; import org.itsallcode.openfasttrace.api.importer.input.RealFileInput; -import org.itsallcode.openfasttrace.importer.tag.LineReader.LineConsumer; +import org.itsallcode.openfasttrace.importer.tag.common.LineReader.LineConsumer; import org.itsallcode.openfasttrace.testutil.importer.input.StreamInput; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -22,26 +19,21 @@ import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) -class TestLineReader -{ +class TestLineReader { private static final Path DUMMY_FILE = Paths.get("dummy"); private static final String TEST_CONTENT_LINE_1 = "testContent äöüß"; @Mock private LineConsumer consumerMock; - @Mock - private BufferedReader readerMock; private Path tempDir; @BeforeEach - void beforeEach(@TempDir final Path tempDir) - { + void beforeEach(@TempDir final Path tempDir) { this.tempDir = tempDir; } @Test - void testCreateForPathAndCharset() throws IOException - { + void testCreateForPathAndCharset() throws IOException { final Path tempFile = this.tempDir.resolve("test"); Files.write(tempFile, TEST_CONTENT_LINE_1.getBytes(StandardCharsets.UTF_8)); LineReader.create(RealFileInput.forPath(tempFile)).readLines(this.consumerMock); @@ -49,8 +41,7 @@ void testCreateForPathAndCharset() throws IOException } @Test - void testCreateForPathAndReaderReader() throws IOException - { + void testCreateForPathAndReaderReader() throws IOException { final Path tempFile = this.tempDir.resolve("test"); Files.write(tempFile, TEST_CONTENT_LINE_1.getBytes(StandardCharsets.UTF_8)); LineReader.create(StreamInput.forReader(DUMMY_FILE, Files.newBufferedReader(tempFile))) @@ -59,66 +50,55 @@ void testCreateForPathAndReaderReader() throws IOException } @Test - void testReadLinesEmptyFile() - { + void testReadLinesEmptyFile() { readContent(""); assertLinesRead(); } @Test - void testReadLinesSingleLine() - { + void testReadLinesSingleLine() { readContent("line1"); assertLinesRead("line1"); } @Test - void testReadLinesSingleLineWithTrailingNewline() - { + void testReadLinesSingleLineWithTrailingNewline() { readContent("line1\n"); assertLinesRead("line1"); } - // Using separate tests instead of parametrized tests to get readable test - // names @SuppressWarnings("java:S5976") @Test - void testReadLinesTwoLinesWithCR() - { + void testReadLinesTwoLinesWithCR() { readContent("line1\nline2"); assertLinesRead("line1", "line2"); } @Test - void testReadLinesTwoLinesWithLF() - { + void testReadLinesTwoLinesWithLF() { readContent("line1\rline2"); assertLinesRead("line1", "line2"); } @Test - void testReadLinesTwoLinesWithLFCR() - { + void testReadLinesTwoLinesWithLFCR() { readContent("line1\r\nline2"); assertLinesRead("line1", "line2"); } - private void readContent(final String content) - { + private void readContent(final String content) { final InputFile file = StreamInput.forReader(DUMMY_FILE, new BufferedReader(new StringReader(content))); LineReader.create(file).readLines(this.consumerMock); } - private void assertLinesRead(final String... expectedLines) - { + private void assertLinesRead(final String... expectedLines) { final InOrder inOrder = inOrder(this.consumerMock); int lineNumber = 1; - for (final String line : expectedLines) - { + for (final String line : expectedLines) { inOrder.verify(this.consumerMock).readLine(lineNumber, line); lineNumber++; } inOrder.verifyNoMoreInteractions(); } -} \ No newline at end of file +} diff --git a/importer/tag/pom.xml b/importer/tag/pom.xml index 165ed1187..f037464c1 100644 --- a/importer/tag/pom.xml +++ b/importer/tag/pom.xml @@ -18,6 +18,10 @@ org.itsallcode.openfasttrace openfasttrace-api + + org.itsallcode.openfasttrace + openfasttrace-importer-tag-importer-common + org.itsallcode.openfasttrace openfasttrace-testutil diff --git a/importer/tag/src/main/java/module-info.java b/importer/tag/src/main/java/module-info.java index dfe45b49f..1901ee56e 100644 --- a/importer/tag/src/main/java/module-info.java +++ b/importer/tag/src/main/java/module-info.java @@ -1,12 +1,12 @@ /** * This provides an importer for coverage tags. - * + * * @provides org.itsallcode.openfasttrace.api.importer.ImporterFactory */ module org.itsallcode.openfasttrace.importer.tag { - requires java.logging; requires transitive org.itsallcode.openfasttrace.api; + requires org.itsallcode.openfasttrace.importer.tag.common; provides org.itsallcode.openfasttrace.api.importer.ImporterFactory with org.itsallcode.openfasttrace.importer.tag.TagImporterFactory; diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/AbstractRegexLineConsumer.java b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/AbstractRegexLineConsumer.java deleted file mode 100644 index 1d2f4c9ce..000000000 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/AbstractRegexLineConsumer.java +++ /dev/null @@ -1,46 +0,0 @@ -package org.itsallcode.openfasttrace.importer.tag; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import org.itsallcode.openfasttrace.importer.tag.LineReader.LineConsumer; - -abstract class AbstractRegexLineConsumer implements LineConsumer -{ - private final Pattern pattern; - - AbstractRegexLineConsumer(final String patternRegex) - { - this(Pattern.compile(patternRegex)); - } - - private AbstractRegexLineConsumer(final Pattern pattern) - { - this.pattern = pattern; - } - - @Override - public void readLine(final int lineNumber, final String line) - { - final Matcher matcher = this.pattern.matcher(line); - int counter = 0; - while (matcher.find()) - { - this.processMatch(matcher, lineNumber, counter); - counter++; - } - } - - /** - * Process a match from an input line. - * - * @param matcher - * regular expression {@link Matcher}. - * @param lineNumber - * line number of the matched the input, starting with {@code 1} - * for the first line. - * @param lineMatchCount - * number of the current match in the context of the current - * line, starting with {@code 0} for the first match in a line. - */ - abstract void processMatch(Matcher matcher, int lineNumber, int lineMatchCount); -} diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LineReader.java b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LineReader.java deleted file mode 100644 index 0591511da..000000000 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/LineReader.java +++ /dev/null @@ -1,70 +0,0 @@ -package org.itsallcode.openfasttrace.importer.tag; - -import java.io.IOException; -import java.io.LineNumberReader; - -import org.itsallcode.openfasttrace.api.importer.ImporterException; -import org.itsallcode.openfasttrace.api.importer.input.InputFile; - -class LineReader -{ - private final InputFile file; - - LineReader(final InputFile file) - { - this.file = file; - } - - static LineReader create(final InputFile file) - { - return new LineReader(file); - } - - void readLines(final LineConsumer consumer) - { - int currentLineNumber = 0; - try (final LineNumberReader reader = new LineNumberReader(this.file.createReader())) - { - String line; - while ((line = reader.readLine()) != null) - { - currentLineNumber = reader.getLineNumber(); - processLine(consumer, currentLineNumber, line); - } - } - catch (final IOException exception) - { - throw new ImporterException( - "Error reading \"" + this.file + "\" at line " + currentLineNumber, exception); - } - } - - private void processLine(final LineConsumer consumer, final int currentLineNumber, - final String line) - { - try - { - consumer.readLine(currentLineNumber, line); - } - catch (final RuntimeException exception) - { - throw new ImporterException("Error processing line " + this.file.getPath() + ":" - + currentLineNumber + " '" + line + "': " + exception, exception); - } - } - - @FunctionalInterface - public interface LineConsumer - { - /** - * Process a single line from the input. - * - * @param line - * current line. - * @param lineNumber - * number of the current line, starting with {@code 1} for - * the first line. - */ - void readLine(int lineNumber, String line); - } -} diff --git a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporter.java b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporter.java index b48ef70bc..264d9757a 100644 --- a/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporter.java +++ b/importer/tag/src/main/java/org/itsallcode/openfasttrace/importer/tag/TagImporter.java @@ -1,13 +1,12 @@ package org.itsallcode.openfasttrace.importer.tag; -import java.util.ArrayList; -import java.util.List; - import org.itsallcode.openfasttrace.api.importer.ImportEventListener; import org.itsallcode.openfasttrace.api.importer.Importer; import org.itsallcode.openfasttrace.api.importer.input.InputFile; import org.itsallcode.openfasttrace.api.importer.tag.config.PathConfig; -import org.itsallcode.openfasttrace.importer.tag.LineReader.LineConsumer; +import org.itsallcode.openfasttrace.importer.tag.common.CoverageTagParser; +import org.itsallcode.openfasttrace.importer.tag.common.LineReader; +import org.itsallcode.openfasttrace.importer.tag.common.LineReader.LineConsumer; /** * {@link Importer} for tags in source code files. @@ -27,20 +26,7 @@ class TagImporter implements Importer static TagImporter create(final PathConfig config, final InputFile file, final ImportEventListener listener) { - final LineConsumer lineConsumer = createLineConsumer(config, file, listener); - return new TagImporter(lineConsumer, file); - } - - private static LineConsumer createLineConsumer(final PathConfig config, final InputFile file, - final ImportEventListener listener) - { - final List importers = new ArrayList<>(); - importers.add(new LongTagImportingLineConsumer(file, listener)); - if (config != null) - { - importers.add(new ShortTagImportingLineConsumer(config, file, listener)); - } - return new DelegatingLineConsumer(importers); + return new TagImporter(CoverageTagParser.create(config, file, listener), file); } @Override diff --git a/oft-self-trace.sh b/oft-self-trace.sh index 47b612c0a..d6b5b5471 100755 --- a/oft-self-trace.sh +++ b/oft-self-trace.sh @@ -21,6 +21,7 @@ if $oft_script trace \ "$base_dir/importer/restructuredtext/src" \ "$base_dir/importer/specobject/src" \ "$base_dir/importer/zip/src" \ + "$base_dir/importer/tag-importer-common/src" \ "$base_dir/importer/tag/src" \ "$base_dir/core/src/main" \ "$base_dir/core/src/test/java" \ diff --git a/parent/pom.xml b/parent/pom.xml index f611843ca..2648a3eff 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -147,6 +147,12 @@ ${revision} compile + + org.itsallcode.openfasttrace + openfasttrace-importer-tag-importer-common + ${revision} + compile + org.itsallcode.openfasttrace openfasttrace-importer-tag diff --git a/pom.xml b/pom.xml index 813a87ffb..3b7d3fca3 100644 --- a/pom.xml +++ b/pom.xml @@ -29,6 +29,7 @@ importer/xmlparser importer/restructuredtext importer/specobject + importer/tag-importer-common importer/tag importer/zip reporter/plaintext From edde843600242cf84d83e21c63b8facc32cfc166 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 09:26:51 +0200 Subject: [PATCH 05/14] Improve test coverage --- .../importer/tag/common/TestLineReader.java | 37 ++++++++++ .../TestLongTagImportingLineConsumer.java | 65 +++++++++++++++++ .../TestShortTagImportingLineConsumer.java | 70 +++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java create mode 100644 importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestShortTagImportingLineConsumer.java diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java index 33c629607..3ab0f7a1a 100644 --- a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java @@ -1,11 +1,20 @@ package org.itsallcode.openfasttrace.importer.tag.common; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.*; import java.nio.charset.StandardCharsets; import java.nio.file.*; +import org.itsallcode.openfasttrace.api.importer.ImporterException; import org.itsallcode.openfasttrace.api.importer.input.InputFile; import org.itsallcode.openfasttrace.api.importer.input.RealFileInput; import org.itsallcode.openfasttrace.importer.tag.common.LineReader.LineConsumer; @@ -86,6 +95,34 @@ void testReadLinesTwoLinesWithLFCR() { assertLinesRead("line1", "line2"); } + @Test + void wrapsConsumerFailureWithLineInformation() { + final RuntimeException cause = new IllegalArgumentException("invalid line"); + doThrow(cause).when(this.consumerMock).readLine(1, "line1"); + + final ImporterException exception = assertThrows(ImporterException.class, () -> readContent("line1")); + + assertAll( + () -> assertThat(exception.getMessage(), + equalTo("Error processing line dummy:1 'line1': " + cause)), + () -> assertThat(exception.getCause(), sameInstance(cause))); + } + + @Test + void wrapsReaderCreationFailure() throws IOException { + final InputFile file = mock(InputFile.class); + final IOException cause = new IOException("cannot read"); + when(file.createReader()).thenThrow(cause); + when(file.toString()).thenReturn("unreadable.file"); + + final ImporterException exception = assertThrows(ImporterException.class, + () -> LineReader.create(file).readLines(this.consumerMock)); + + assertAll( + () -> assertThat(exception.getMessage(), equalTo("Error reading \"unreadable.file\" at line 0")), + () -> assertThat(exception.getCause(), sameInstance(cause))); + } + private void readContent(final String content) { final InputFile file = StreamInput.forReader(DUMMY_FILE, new BufferedReader(new StringReader(content))); diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java new file mode 100644 index 000000000..19f796de9 --- /dev/null +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java @@ -0,0 +1,65 @@ +package org.itsallcode.openfasttrace.importer.tag.common; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +import java.io.BufferedReader; +import java.io.StringReader; +import java.nio.file.Paths; +import java.util.List; +import java.util.stream.Stream; + +import org.itsallcode.openfasttrace.api.core.SpecificationItem; +import org.itsallcode.openfasttrace.api.core.SpecificationItemId; +import org.itsallcode.openfasttrace.api.importer.SpecificationListBuilder; +import org.itsallcode.openfasttrace.api.importer.input.InputFile; +import org.itsallcode.openfasttrace.testutil.importer.input.StreamInput; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +class TestLongTagImportingLineConsumer { + private static final String FILE = "source.file"; + + static Stream tagImportingTests() { + return Stream.of( + // [utest->dsn~import.full-coverage-tag-with-name-and-revision~1] + // [utest->dsn~import.full-coverage-tag-with-needed-coverage~1] + // [utest->dsn~import.full-coverage-tag-multiple-needed-coverage~1] + Arguments.of(3, "[impl~tag~1 -> dsn~first~2, dsn~second~3 >> utest, itest" + "]", + List.of(item("impl~tag~1", 3, List.of("dsn~first~2", "dsn~second~3"), + List.of("utest", "itest")))), + // [utest->dsn~import.full-coverage-tag~1] + Arguments.of(4, "[impl -> dsn~covered~2" + "]", + List.of(item("impl~covered-3014110766~0", 4, List.of("dsn~covered~2"), List.of()))), + // [utest->dsn~import.full-coverage-tag-with-needed-coverage-readable-names~1] + Arguments.of(5, "[impl -> dsn~first~2, dsn~second~3 >> utest" + "]", + List.of(item("impl~first~0", 5, List.of("dsn~first~2"), List.of("utest")), + item("impl~second~0", 5, List.of("dsn~second~3"), List.of("utest"))))); + } + + @ParameterizedTest + @MethodSource("tagImportingTests") + void importsLongTag(final int lineNumber, final String tag, final List expectedItems) { + final SpecificationListBuilder listener = SpecificationListBuilder.create(); + final LongTagImportingLineConsumer consumer = new LongTagImportingLineConsumer(inputFile(), listener); + + consumer.readLine(lineNumber, tag); + + assertThat(listener.build(), equalTo(expectedItems)); + } + + private static SpecificationItem item(final String id, final int lineNumber, final List coveredIds, + final List neededArtifactTypes) { + final SpecificationItem.Builder builder = SpecificationItem.builder() + .id(SpecificationItemId.parseId(id)) + .location(FILE, lineNumber); + coveredIds.stream().map(SpecificationItemId::parseId).forEach(builder::addCoveredId); + neededArtifactTypes.forEach(builder::addNeedsArtifactType); + return builder.build(); + } + + private static InputFile inputFile() { + return StreamInput.forReader(Paths.get(FILE), new BufferedReader(new StringReader(""))); + } +} diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestShortTagImportingLineConsumer.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestShortTagImportingLineConsumer.java new file mode 100644 index 000000000..54a6e28ad --- /dev/null +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestShortTagImportingLineConsumer.java @@ -0,0 +1,70 @@ +package org.itsallcode.openfasttrace.importer.tag.common; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +import java.io.BufferedReader; +import java.io.StringReader; +import java.nio.file.Paths; +import java.util.List; +import java.util.stream.Stream; + +import org.itsallcode.openfasttrace.api.core.SpecificationItem; +import org.itsallcode.openfasttrace.api.core.SpecificationItemId; +import org.itsallcode.openfasttrace.api.importer.SpecificationListBuilder; +import org.itsallcode.openfasttrace.api.importer.input.InputFile; +import org.itsallcode.openfasttrace.api.importer.tag.config.PathConfig; +import org.itsallcode.openfasttrace.testutil.importer.input.StreamInput; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +// [utest->dsn~import.short-coverage-tag~1] +class TestShortTagImportingLineConsumer { + private static final String FILE = "source.file"; + + static Stream shortTagImportingTests() { + return Stream.of( + Arguments.of(3, "[[covered:2" + "]]", null, + List.of(item("covered-3798966306", 3, "req~covered~2"))), + Arguments.of(4, "[[covered:2" + "]]", "prefix.", + List.of(item("prefix.covered-1644633624", 4, "req~prefix.covered~2"))), + Arguments.of(5, "[[first:2" + "]]" + "[[second:3" + "]]", null, + List.of(item("first-969050621", 5, "req~first~2"), + item("second-2680780004", 5, "req~second~3")))); + } + + @ParameterizedTest + @MethodSource("shortTagImportingTests") + void importsShortTag(final int lineNumber, final String tag, final String coveredItemNamePrefix, + final List expectedItems) { + final SpecificationListBuilder listener = SpecificationListBuilder.create(); + final ShortTagImportingLineConsumer consumer = new ShortTagImportingLineConsumer( + pathConfig(coveredItemNamePrefix), inputFile(), listener); + + consumer.readLine(lineNumber, tag); + + assertThat(listener.build(), equalTo(expectedItems)); + } + + private static PathConfig pathConfig(final String coveredItemNamePrefix) { + return PathConfig.builder() + .patternPathMatcher("glob:**") + .coveredItemArtifactType("req") + .coveredItemNamePrefix(coveredItemNamePrefix) + .tagArtifactType("utest") + .build(); + } + + private static SpecificationItem item(final String tagItemName, final int lineNumber, final String coveredId) { + return SpecificationItem.builder() + .id(SpecificationItemId.createId("utest", tagItemName)) + .location(FILE, lineNumber) + .addCoveredId(SpecificationItemId.parseId(coveredId)) + .build(); + } + + private static InputFile inputFile() { + return StreamInput.forReader(Paths.get(FILE), new BufferedReader(new StringReader(""))); + } +} From cd50f83b74ae713cb1b289dd18cc62de723e9a24 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 09:28:19 +0200 Subject: [PATCH 06/14] Improve security by not persisting git credentials --- .github/workflows/broken_links_checker.yml | 2 ++ .github/workflows/build.yml | 3 +++ .github/workflows/codeql-analysis.yml | 2 ++ .github/workflows/gh-pages.yml | 2 ++ .github/workflows/release.yml | 2 ++ 5 files changed, 11 insertions(+) diff --git a/.github/workflows/broken_links_checker.yml b/.github/workflows/broken_links_checker.yml index 4944476c3..52706173b 100644 --- a/.github/workflows/broken_links_checker.yml +++ b/.github/workflows/broken_links_checker.yml @@ -12,6 +12,8 @@ jobs: contents: read steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - name: Configure broken links checker run: | mkdir -p ./target diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e58af4084..30ad864d3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,6 +20,8 @@ jobs: java: 21 - os: ubuntu-latest java: 25 + - os: ubuntu-latest + java: 26 concurrency: group: ${{ github.workflow }}-${{ github.ref }}-os-${{ matrix.os }}-java-${{ matrix.java }} @@ -38,6 +40,7 @@ jobs: - uses: actions/checkout@v7 with: fetch-depth: 0 + persist-credentials: false - uses: actions/setup-java@v5 name: Set up Java ${{ matrix.java }} diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 3a171bed6..220de3d75 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -23,6 +23,8 @@ jobs: steps: - name: Checkout repository uses: actions/checkout@v7 + with: + persist-credentials: false - uses: actions/setup-java@v5 with: diff --git a/.github/workflows/gh-pages.yml b/.github/workflows/gh-pages.yml index 4467d0050..c06aa3d3e 100644 --- a/.github/workflows/gh-pages.yml +++ b/.github/workflows/gh-pages.yml @@ -17,6 +17,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v7 + with: + persist-credentials: false - name: Setup Pages uses: actions/configure-pages@v6 - name: Build with Jekyll diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 13cdd2314..50d150eb7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -28,6 +28,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v7 + with: + persist-credentials: false - name: Fail if not running on main branch if: ${{ github.ref != 'refs/heads/main' }} From 2e3d4dca94465aa8c81b8d47a34c4e2a75900a1b Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 09:40:04 +0200 Subject: [PATCH 07/14] Fix sonar findings --- .../openfasttrace/importer/tag/common/LineReader.java | 2 +- .../openfasttrace/importer/tag/common/TestLineReader.java | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LineReader.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LineReader.java index cfb1dec4a..32bafc053 100644 --- a/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LineReader.java +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LineReader.java @@ -9,7 +9,7 @@ /** * Reads an input file line by line and forwards every line to a consumer. */ -public class LineReader { +public final class LineReader { private final InputFile file; private LineReader(final InputFile file) { diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java index 3ab0f7a1a..aebaceefa 100644 --- a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java @@ -5,10 +5,7 @@ import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import java.io.*; import java.nio.charset.StandardCharsets; @@ -114,9 +111,10 @@ void wrapsReaderCreationFailure() throws IOException { final IOException cause = new IOException("cannot read"); when(file.createReader()).thenThrow(cause); when(file.toString()).thenReturn("unreadable.file"); + final LineReader lineReader = LineReader.create(file); final ImporterException exception = assertThrows(ImporterException.class, - () -> LineReader.create(file).readLines(this.consumerMock)); + () -> lineReader.readLines(this.consumerMock)); assertAll( () -> assertThat(exception.getMessage(), equalTo("Error reading \"unreadable.file\" at line 0")), From 80989f49ae37d88b90b312b26226fdee391a2f60 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 09:46:59 +0200 Subject: [PATCH 08/14] Simplify unit test --- .../tag/common/TestCoverageTagParser.java | 70 ++++++++++--------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestCoverageTagParser.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestCoverageTagParser.java index 3e77b197e..e0086e9b9 100644 --- a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestCoverageTagParser.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestCoverageTagParser.java @@ -1,68 +1,70 @@ package org.itsallcode.openfasttrace.importer.tag.common; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import java.io.BufferedReader; import java.io.StringReader; import java.nio.file.Paths; +import java.util.List; +import org.itsallcode.openfasttrace.api.core.SpecificationItem; import org.itsallcode.openfasttrace.api.core.SpecificationItemId; -import org.itsallcode.openfasttrace.api.importer.ImportEventListener; +import org.itsallcode.openfasttrace.api.importer.SpecificationListBuilder; import org.itsallcode.openfasttrace.api.importer.input.InputFile; import org.itsallcode.openfasttrace.api.importer.tag.config.PathConfig; import org.itsallcode.openfasttrace.importer.tag.common.LineReader.LineConsumer; import org.itsallcode.openfasttrace.testutil.importer.input.StreamInput; import org.junit.jupiter.api.Test; -import org.mockito.InOrder; -class TestCoverageTagParser -{ +class TestCoverageTagParser { private static final String FILE = "source.file"; @Test - void testImportsFullTag() - { - final ImportEventListener listener = mock(ImportEventListener.class); + void importsFullTag() { + final SpecificationListBuilder listener = SpecificationListBuilder.create(); final LineConsumer parser = CoverageTagParser.create(null, inputFile(), listener); final String coverageTag = "[" + "impl~name~1->dsn~covered~2>>utest]"; parser.readLine(3, coverageTag); - final InOrder inOrder = inOrder(listener); - inOrder.verify(listener).beginSpecificationItem(); - inOrder.verify(listener).setLocation(FILE, 3); - inOrder.verify(listener).setId(SpecificationItemId.parseId("impl~name~1")); - inOrder.verify(listener).addCoveredId(SpecificationItemId.parseId("dsn~covered~2")); - inOrder.verify(listener).addNeededArtifactType("utest"); - inOrder.verify(listener).endSpecificationItem(); - inOrder.verifyNoMoreInteractions(); + assertThat(listener.build(), equalTo(List.of(item(SpecificationItemId.parseId("impl~name~1"), 3, + List.of("dsn~covered~2"), List.of("utest"))))); } @Test - void testImportsConfiguredShortTag() - { - final PathConfig config = mock(PathConfig.class); - when(config.getCoveredItemArtifactType()).thenReturn("req"); - when(config.getCoveredItemNamePrefix()).thenReturn("prefix."); - when(config.getTagArtifactType()).thenReturn("utest"); - final ImportEventListener listener = mock(ImportEventListener.class); + void importsConfiguredShortTag() { + final PathConfig config = pathConfig(); + final SpecificationListBuilder listener = SpecificationListBuilder.create(); final LineConsumer parser = CoverageTagParser.create(config, inputFile(), listener); parser.readLine(2, "[[covered:3]]"); - final InOrder inOrder = inOrder(listener); - inOrder.verify(listener).beginSpecificationItem(); - inOrder.verify(listener).setLocation(FILE, 2); - inOrder.verify(listener).setId(SpecificationItemId.createId("utest", "prefix.covered-1743877134")); - inOrder.verify(listener).addCoveredId(SpecificationItemId.createId("req", "prefix.covered", 3)); - inOrder.verify(listener).endSpecificationItem(); - inOrder.verifyNoMoreInteractions(); + assertThat(listener.build(), + equalTo(List.of(item(SpecificationItemId.createId("utest", "prefix.covered-1743877134"), + 2, List.of("req~prefix.covered~3"), List.of())))); } - private static InputFile inputFile() - { + private static PathConfig pathConfig() { + return PathConfig.builder() + .patternPathMatcher("glob:**") + .coveredItemArtifactType("req") + .coveredItemNamePrefix("prefix.") + .tagArtifactType("utest") + .build(); + } + + private static SpecificationItem item(final SpecificationItemId id, final int lineNumber, + final List coveredIds, final List neededArtifactTypes) { + final SpecificationItem.Builder builder = SpecificationItem.builder() + .id(id) + .location(FILE, lineNumber); + coveredIds.stream().map(SpecificationItemId::parseId).forEach(builder::addCoveredId); + neededArtifactTypes.forEach(builder::addNeedsArtifactType); + return builder.build(); + } + + private static InputFile inputFile() { return StreamInput.forReader(Paths.get(FILE), new BufferedReader(new StringReader(""))); } } From 2cd0905df2c982620e01fe29a11f651bbc9588a0 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 10:15:15 +0200 Subject: [PATCH 09/14] Update design for new module --- doc/spec/design.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/spec/design.md b/doc/spec/design.md index a96fd6e1d..48353ade3 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -98,6 +98,12 @@ The plugin loader discovers and loads available plugins. ## Importers For each specification artifact type OFT uses an importer. The importer uses the specification artifact as data source and reads specification items from it. +### Shared Coverage Tag Parser + +The `importer/tag-importer-common` module provides the reusable line scanning and coverage-tag parsing used by importers. Its public API consists of `LineReader`, which forwards input lines to a consumer, and `CoverageTagParser`, which recognizes full coverage tags and optionally configured short coverage tags. + +The tag importer remains responsible for selecting its input files and creating the shared parser. Parsing implementation classes remain encapsulated in the shared module so that future importers can reuse the same coverage-tag semantics without depending on tag-importer internals. + ## Import Event Listener Importers emit events if they find parts of a [specification item](#specification-item) in the artifact they are importing. From db2ccb0b7381f1d3dd9ff273e8e8d1b7b17e10f5 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 12:38:34 +0200 Subject: [PATCH 10/14] Add line length to quality requirements --- doc/spec/design/quality_requirements.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/spec/design/quality_requirements.md b/doc/spec/design/quality_requirements.md index 80972ae7e..04101bacf 100644 --- a/doc/spec/design/quality_requirements.md +++ b/doc/spec/design/quality_requirements.md @@ -110,6 +110,8 @@ The following rules are written for both human contributors and coding agents. T 3. Declare method parameters as `final`. 4. Output parameters are only allowed when required by external libraries. 5. Prefer explicit types over `var`. +6. Limit code lines to 120 characters. Do not wrap code earlier solely to fit a shorter line length. +7. Limit comment lines, including JavaDoc, to 80 characters. ### Java Test Rules From 1983c0c5315aab632ae44cf6660dfa5b084281da Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 12:42:16 +0200 Subject: [PATCH 11/14] Refer agents to use quality requirements --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 8ec0d70d5..be020b796 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,6 +31,7 @@ You are an expert Java developer specializing in requirement tracing and softwar - Review all changes with `./oft-self-trace.sh` to ensure tracing completeness. - Follow the branching strategy: `/_` (e.g., `feature/533_update_agents_md`). - Place coverage markers at the narrowest possible scope (method or class). + - Follow the quality requirements in `doc/spec/design/quality_requirements.md`. - **Ask First**: - Before adding new external dependencies to `pom.xml`. - Before changing existing architectural patterns in `openfasttrace-core`. From faad8bcf329d51589e15c4baadeb5692d5c9949c Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 13:05:01 +0200 Subject: [PATCH 12/14] Improve readability of unit tests --- .../TestLongTagImportingLineConsumer.java | 20 +++++++++++-------- .../TestShortTagImportingLineConsumer.java | 19 +++++++++++------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java index 19f796de9..e5cd1fae1 100644 --- a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java @@ -26,16 +26,20 @@ static Stream tagImportingTests() { // [utest->dsn~import.full-coverage-tag-with-name-and-revision~1] // [utest->dsn~import.full-coverage-tag-with-needed-coverage~1] // [utest->dsn~import.full-coverage-tag-multiple-needed-coverage~1] - Arguments.of(3, "[impl~tag~1 -> dsn~first~2, dsn~second~3 >> utest, itest" + "]", - List.of(item("impl~tag~1", 3, List.of("dsn~first~2", "dsn~second~3"), - List.of("utest", "itest")))), + testCase(3, "[impl~tag~1 -> dsn~first~2, dsn~second~3 >> utest, itest" + "]", + item("impl~tag~1", 3, List.of("dsn~first~2", "dsn~second~3"), + List.of("utest", "itest"))), // [utest->dsn~import.full-coverage-tag~1] - Arguments.of(4, "[impl -> dsn~covered~2" + "]", - List.of(item("impl~covered-3014110766~0", 4, List.of("dsn~covered~2"), List.of()))), + testCase(4, "[impl -> dsn~covered~2" + "]", + item("impl~covered-3014110766~0", 4, List.of("dsn~covered~2"), List.of())), // [utest->dsn~import.full-coverage-tag-with-needed-coverage-readable-names~1] - Arguments.of(5, "[impl -> dsn~first~2, dsn~second~3 >> utest" + "]", - List.of(item("impl~first~0", 5, List.of("dsn~first~2"), List.of("utest")), - item("impl~second~0", 5, List.of("dsn~second~3"), List.of("utest"))))); + testCase(5, "[impl -> dsn~first~2, dsn~second~3 >> utest" + "]", + item("impl~first~0", 5, List.of("dsn~first~2"), List.of("utest")), + item("impl~second~0", 5, List.of("dsn~second~3"), List.of("utest")))); + } + + static Arguments testCase(final int lineNumber, final String tag, final SpecificationItem... expectedItems) { + return Arguments.of(lineNumber, tag, List.of(expectedItems)); } @ParameterizedTest diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestShortTagImportingLineConsumer.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestShortTagImportingLineConsumer.java index 54a6e28ad..b1f155466 100644 --- a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestShortTagImportingLineConsumer.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestShortTagImportingLineConsumer.java @@ -25,13 +25,18 @@ class TestShortTagImportingLineConsumer { static Stream shortTagImportingTests() { return Stream.of( - Arguments.of(3, "[[covered:2" + "]]", null, - List.of(item("covered-3798966306", 3, "req~covered~2"))), - Arguments.of(4, "[[covered:2" + "]]", "prefix.", - List.of(item("prefix.covered-1644633624", 4, "req~prefix.covered~2"))), - Arguments.of(5, "[[first:2" + "]]" + "[[second:3" + "]]", null, - List.of(item("first-969050621", 5, "req~first~2"), - item("second-2680780004", 5, "req~second~3")))); + testCase(3, "[[covered:2" + "]]", null, + item("covered-3798966306", 3, "req~covered~2")), + testCase(4, "[[covered:2" + "]]", "prefix.", + item("prefix.covered-1644633624", 4, "req~prefix.covered~2")), + testCase(5, "[[first:2" + "]]" + "[[second:3" + "]]", null, + item("first-969050621", 5, "req~first~2"), + item("second-2680780004", 5, "req~second~3"))); + } + + private static Arguments testCase(final int lineNumber, final String tag, final String coveredItemNamePrefix, + final SpecificationItem... expectedItems) { + return Arguments.of(lineNumber, tag, coveredItemNamePrefix, List.of(expectedItems)); } @ParameterizedTest From 6c6c7627fcbf5c3073ce50bfe8d00bee53cbce35 Mon Sep 17 00:00:00 2001 From: Christoph Pirkl <4711730+kaklakariada@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:22:57 +0200 Subject: [PATCH 13/14] Update importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sebastian Bär --- .../importer/tag/common/TestLongTagImportingLineConsumer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java index e5cd1fae1..a5236ffca 100644 --- a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java @@ -47,9 +47,7 @@ static Arguments testCase(final int lineNumber, final String tag, final Specific void importsLongTag(final int lineNumber, final String tag, final List expectedItems) { final SpecificationListBuilder listener = SpecificationListBuilder.create(); final LongTagImportingLineConsumer consumer = new LongTagImportingLineConsumer(inputFile(), listener); - consumer.readLine(lineNumber, tag); - assertThat(listener.build(), equalTo(expectedItems)); } From 3e5a91a8228e71cb755566cd0ae51a372dd3c9dd Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Thu, 23 Jul 2026 13:23:09 +0200 Subject: [PATCH 14/14] Implement review findings --- .../importer/tag/common/TestLineReader.java | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java index aebaceefa..1da851f22 100644 --- a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLineReader.java @@ -2,8 +2,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.sameInstance; -import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.*; @@ -93,20 +91,16 @@ void testReadLinesTwoLinesWithLFCR() { } @Test - void wrapsConsumerFailureWithLineInformation() { + void testWrapsConsumerFailureWithLineInformation() { final RuntimeException cause = new IllegalArgumentException("invalid line"); doThrow(cause).when(this.consumerMock).readLine(1, "line1"); final ImporterException exception = assertThrows(ImporterException.class, () -> readContent("line1")); - - assertAll( - () -> assertThat(exception.getMessage(), - equalTo("Error processing line dummy:1 'line1': " + cause)), - () -> assertThat(exception.getCause(), sameInstance(cause))); + assertThat(exception.getMessage(), equalTo("Error processing line dummy:1 'line1': " + cause)); } @Test - void wrapsReaderCreationFailure() throws IOException { + void testWrapsReaderCreationFailure() throws IOException { final InputFile file = mock(InputFile.class); final IOException cause = new IOException("cannot read"); when(file.createReader()).thenThrow(cause); @@ -116,9 +110,7 @@ void wrapsReaderCreationFailure() throws IOException { final ImporterException exception = assertThrows(ImporterException.class, () -> lineReader.readLines(this.consumerMock)); - assertAll( - () -> assertThat(exception.getMessage(), equalTo("Error reading \"unreadable.file\" at line 0")), - () -> assertThat(exception.getCause(), sameInstance(cause))); + assertThat(exception.getMessage(), equalTo("Error reading \"unreadable.file\" at line 0")); } private void readContent(final String content) {