Skip to content

Restore Flyway command coverage - #4016

Merged
thomhurst merged 3 commits into
mainfrom
issue-3993-command-coverage
Aug 24, 2026
Merged

Restore Flyway command coverage#4016
thomhurst merged 3 commits into
mainfrom
issue-3993-command-coverage

Conversation

@thomhurst

@thomhurst thomhurst commented Aug 23, 2026

Copy link
Copy Markdown
Owner

Fixes #3993
Part of #3996

Summary

  • parse Flyway command rows annotated with (preview)
  • retain description-only and flag-only Flyway commands
  • stop discovery before current configuration-section headings
  • extract the installed Flyway version from noisy -v output
  • regenerate all 19 pinned Flyway 10.20.1 commands, including seven missing APIs

Validation

  • OptionsGenerator tests: 818 passed
  • OptionsGenerator Release build: 0 warnings, 0 errors
  • Flyway Release build: 0 warnings, 0 errors
  • Flyway tests: 1 passed
  • exact pinned Flyway 10.20.1 generation: 19 commands, 0 errors

Summary by CodeRabbit

  • New Features

    • Expanded Flyway command documentation with additional commands and options.
    • Added clearer installation guidance, including the required external Flyway executable.
  • Bug Fixes

    • Improved recognition of Flyway versions, commands, options, and configuration sections.
    • Enhanced compatibility with noisy output, legacy help formats, and varied command formatting.
  • Tests

    • Added coverage for version extraction, command parsing, option detection, and command completeness.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 856cb0e7-d39f-426a-a95a-f18d891b9c92

📥 Commits

Reviewing files that changed from the base of the PR and between 13fd094 and f52788a.

⛔ Files ignored due to path filters (11)
  • src/ModularPipelines.Flyway/Extensions/FlywayExtensions.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Generated/Flyway.CommandCoverage.json is excluded by !**/generated/**
  • src/ModularPipelines.Flyway/Options/FlywayAddOptions.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Options/FlywayAuthOptions.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Options/FlywayDiffApplyOptions.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Options/FlywayDiffOptions.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Options/FlywayDiffTextOptions.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Options/FlywayGenerateOptions.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Options/FlywayUndoOptions.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Services/Flyway.Generated.cs is excluded by !**/*.generated.*
  • src/ModularPipelines.Flyway/Services/IFlyway.Generated.cs is excluded by !**/*.generated.*
📒 Files selected for processing (3)
  • docs/docs/mp-packages/cli/flyway.md
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/FlywayCommandCoverageTests.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/FlywayCliScraper.cs

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Flyway CLI parsing now handles edition-formatted versions, command-table boundaries, legacy configuration help, and expanded option syntax. New tests validate command coverage and parsing cases. Flyway package documentation now separates executable installation and package usage.

Changes

Flyway CLI support

Layer / File(s) Summary
Flyway scraper parsing
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/FlywayCliScraper.cs
The scraper extracts Flyway edition versions, parses commands until the first non-command line, and recognizes additional option formats.
Coverage validation tests
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/FlywayCommandCoverageTests.cs
Tests cover noisy version output, legacy configuration help, reworded section headings, and a pinned set of 19 commands.
Flyway package documentation
docs/docs/mp-packages/cli/flyway.md
The documentation separates executable installation from package installation, shows service resolution, and lists additional Flyway commands.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to f5278

The Flyway command coverage changes have passing tests and successful release builds, but the documentation build remains unverified after a TLS certificate error during dependency installation. The PR is mergeable with explicit owner awareness or follow-up to complete that validation.

Sequence Diagram(s)

sequenceDiagram
  participant CoverageTests
  participant FlywayExecutor
  participant FlywayCliScraper
  participant CommandCoverageGuard
  CoverageTests->>FlywayExecutor: Request Flyway fixtures
  FlywayExecutor->>FlywayCliScraper: Return version and help output
  FlywayCliScraper->>CommandCoverageGuard: Submit extracted commands
  CommandCoverageGuard-->>CoverageTests: Return coverage result
Loading

Poem

I’m a rabbit with a CLI map,
Parsing commands without a trap.
Versions, options, rows of green,
Nineteen commands now are seen.
Documentation hops in line—
Flyway’s paths are clear and fine.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: restoring Flyway command coverage.
Linked Issues check ✅ Passed The scraper changes and regression tests address the missing Flyway command coverage described in issue #3993.
Out of Scope Changes check ✅ Passed The documentation, scraper, and tests are related to restoring Flyway command extraction and coverage.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-3993-command-coverage

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Greptile Summary

The PR restores the Flyway 10.20.1 command surface by improving help/version parsing and regenerating the typed integration.

  • Recognizes preview-annotated, description-only, and flag-only Flyway commands.
  • Restores seven generated command APIs for a total of 19.
  • Adds regression coverage for noisy version output, command-table boundaries, legacy configuration help, and pinned command completeness.
  • Updates the Flyway package documentation and command-coverage manifest.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/FlywayCliScraper.cs Expands Flyway command and version parsing while terminating discovery at the next non-command section.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/FlywayCommandCoverageTests.cs Adds focused regression tests for version extraction, legacy help, section boundaries, and exact 19-command coverage.
src/ModularPipelines.Flyway/Services/IFlyway.Generated.cs Regenerates the public service interface with all 19 Flyway commands and compatibility defaults.
src/ModularPipelines.Flyway/Services/Flyway.Generated.cs Adds concrete wrappers for the seven restored Flyway commands.
src/ModularPipelines.Flyway/Generated/Flyway.CommandCoverage.json Updates the pinned Flyway 10.20.1 coverage baseline from 12 to 19 commands.
docs/docs/mp-packages/cli/flyway.md Documents the executable prerequisite and complete restored command surface.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Flyway help and version output] --> B[FlywayCliScraper]
    B --> C[Command definitions]
    C --> D[Command coverage guard]
    D --> E[Options records]
    D --> F[IFlyway service API]
    D --> G[Flyway service implementation]
    E --> H[Typed Flyway package]
    F --> H
    G --> H
Loading

Reviews (6): Last reviewed commit: "fix(flyway): stop at command table bound..." | Re-trigger Greptile

@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown

Code Review

I reviewed the diff for this PR (Flyway command-coverage restoration: new (preview)-aware regexes, HasOptions/ParseVersionOutput overrides in FlywayCliScraper, the 7 newly-generated option classes/methods, and the updated coverage JSON/docs).

Overall: this is a solid, well-scoped fix. The regenerated surface is backed by the PR's own test additions and a real validation run against Flyway 10.20.1, the generator-driven method ordering (Diff/DiffApply/DiffText) is correctly produced by the existing OrderBy(c => c.ClassName) in ServiceInterfaceGenerator.cs (not a regression), and I didn't find any crash-level or high-severity correctness bugs while tracing removed-behavior, cross-file callers, regex correctness, and the JSON schema changes.

One medium-confidence finding worth a look

tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/FlywayCliScraper.cs:274

The rewritten HasOptions override drops the old Flyway-specific Configuration/-url= detection without an equivalent replacement. A Flyway subcommand whose --help output has a genuine Configuration section with single-dash options (e.g. -url= : Jdbc url) but no Description: heading and no double-dash flags would previously be picked up via Contains("Configuration")/Contains("-url="). Now HasOptions falls through to the base implementation, whose OptionLinePattern requires --, so it returns false, ShouldSkipCommand skips the command, and it silently disappears from generation entirely — the opposite failure mode from the parsing gap this PR is fixing.

This doesn't manifest against the currently-pinned Flyway 10.20.1 output (every subcommand's help includes a Description: section there), so it's not blocking. But since the whole point of this PR is restoring dropped command coverage, it'd be worth either:

  • keeping the old Configuration/-url= fallback alongside the new logic (an ||), or
  • adding a regression test with help text shaped like the old single-dash-only format, so a future Flyway version change doesn't silently regress coverage again without anyone noticing (since a "command disappeared" failure mode won't fail loudly — it'll just be missing).

Nice work overall — the test coverage and the validation-against-real-CLI-output approach here is exactly the right way to harden a scraper like this.

@thomhurst

Copy link
Copy Markdown
Owner Author

Addressed the legacy-help finding in 76b89db. Flyway HasOptions now retains the Configuration-section and -url= compatibility signals alongside Description/base detection. Added an end-to-end legacy configuration-only scrape regression proving flyway migrate and its -url option remain generated. Validation: OptionsGenerator tests 819/819; Release build 0 warnings, 0 errors.

@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown

Code Review

Re-reviewed after the latest push (commit 76b89dbbde). This is a solid, well-tested fix for the Flyway command-coverage regression — 7 restored commands, a version-parsing fix for noisy -v output, and good regression coverage including a real pinned-help-text test (PinnedFlywayHelp_SatisfiesCommandCoveragePolicy) and a sentinel test that actively fails if coverage regresses.

Previous finding: addressed ✅

The earlier review flagged that the rewritten HasOptions override dropped the old loose Contains("Configuration") / Contains("-url=") detection, which could silently drop Flyway subcommands using the legacy single-dash Configuration section format. This has been fixed in 76b89dbbde: HasOptions (FlywayCliScraper.cs:274) now retains ConfigurationSectionPattern() and -url= checks alongside Description: and base.HasOptions, backed by the new LegacyConfigurationOnlyHelp_PreservesCommandAndOptions regression test. Good — that's exactly the kind of "silent disappearance" failure mode that's hard to catch without a targeted test.

New finding worth considering: NextSectionPattern's fallback is a hardcoded heading list (medium confidence)

FlywayCliScraper.cs:300-303:

[GeneratedRegex(
    @"(?:\r?\n[ \t]*[A-Za-z][\w ]*[ \t]*\r?\n[ \t]*[-=]+[ \t]*\r?\n)|(?:^[ \t]*(?:Configuration parameters(?:[ \t]*\([^\r\n]*\))?|Flags|Flyway Usage Example)[ \t]*\r?$)",
    RegexOptions.IgnoreCase | RegexOptions.Multiline)]
private static partial Regex NextSectionPattern();

The first alternative (underlined headings) is generic and robust. The second alternative — which exists specifically to bound the Commands section when it's followed by Configuration parameters (Format: -key=value) (not underlined with dashes, per the pinned 10.20.1 output) — only recognizes three literal heading strings: Configuration parameters (...), Flags, Flyway Usage Example.

If a future Flyway release rewords that heading again (e.g. Configuration Options instead of Configuration parameters), NextSectionPattern won't match, sectionEnd falls back to helpText.Length, and the command scan bleeds past the intended boundary into the options/configuration text — FlywayCommandLinePattern would then start matching colon-formatted option lines (-user : User to use to connect...) as spurious commands. That's the exact same class of format-drift breakage this PR exists to fix, just moved one section over.

Suggested approach: rather than enumerating literal heading text, bound the Commands section structurally — e.g. stop at the first line after the section start that doesn't match FlywayCommandLinePattern and isn't blank/continuation, or stop at the first line that starts with a non-indented word followed by a parenthetical/format description (a shape distinct from name : description command rows). That ties the boundary to the shape of the content rather than specific vendor wording, so it survives the next rewording instead of needing another hardcoded string added reactively. Given this PR is explicitly about hardening against Flyway's help-text churn, closing this second, structurally-identical gap now would be cheap insurance — otherwise the next Flyway release risks reopening issue #3993 in a new spot.

Not blocking — the current 10.20.1 output is covered and there's good test coverage overall — but flagging it since it's the same fragility pattern this PR was written to eliminate.

@thomhurst
thomhurst force-pushed the issue-3993-command-coverage branch from 76b89db to 5064ae1 Compare August 23, 2026 20:29
@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown

Code Review

Re-checked this PR against the current HEAD (5064ae12) — no new commits since the last review, and the diff is unchanged from what was already covered in the two previous passes on this thread.

Status of prior findings:

  • Legacy Configuration/-url= detection — fixed. HasOptions (FlywayCliScraper.cs:274) retains the loose Configuration/-url= checks alongside the new Description: and structural detection, backed by LegacyConfigurationOnlyHelp_PreservesCommandAndOptions.

  • ⚠️ NextSectionPattern's hardcoded heading fallback (FlywayCliScraper.cs:300-303) — still open, still non-blocking. The second alternative in that regex only recognizes three literal strings (Configuration parameters (...), Flags, Flyway Usage Example) to bound the Commands section. If a future Flyway release rewords that heading, the scan bleeds past the intended boundary and FlywayCommandLinePattern starts matching option lines as spurious commands — the same class of drift-breakage this PR exists to fix, just one section over.

    Since this PR's whole premise is hardening the scraper against Flyway's help-text churn, it'd be worth closing this second instance of the same fragility pattern now rather than reactively, e.g. bound the Commands section by shape (stop at the first non-blank line that doesn't match FlywayCommandLinePattern) instead of by an enumerated list of heading strings. That way the boundary logic survives the next rewording instead of needing another literal added after the fact. Not a blocker for merging against the pinned 10.20.1 output, which is fully covered by tests.

No new issues found in this pass. Nice regression coverage overall (noisy version output, legacy single-dash help, and a sentinel test pinning the full 19-command surface).

@thomhurst

Copy link
Copy Markdown
Owner Author

Addressed the remaining section-boundary note in 937e844. Flyway command extraction now stops by table shape at the first non-command row after command discovery, rather than enumerating future section headings. Alias rows such as �ersion, -v, --version are recognized without truncating the table. Added a reworded Runtime settings regression. Validation: OptionsGenerator tests 820/820; Release build 0 warnings/errors.

@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown

Code Review

Reviewed the scraper/parsing logic changes in FlywayCliScraper.cs, the new test fixtures, and the regenerated Flyway package files (options records, IFlyway/Flyway service methods, coverage JSON, docs).

Summary

This is a well-scoped, self-contained fix. Highlights:

  • ExtractSubcommands rewrite replaces the old fixed-lookahead NextSectionPattern heuristic with a line-by-line scan that skips blank lines and stops at the first non-matching, non-blank line after a command has already been found (foundCommand flag). I traced this against the "Configuration parameters (Format: -key=value)" heading used in the pinned fixture — it correctly fails to match FlywayCommandLinePattern (no run of 2+ spaces or : immediately after Configuration), so the loop terminates there instead of misparsing url as a command. This is more robust than the previous regex-based section-boundary approach, since it no longer depends on the next heading having a specific underline format.
  • FlywayCommandLinePattern extension to support (preview) suffixes and comma-separated aliases (version, -v, --version) is precise — switching from \s to [ \t] for the separator prevents the pattern from spanning newlines, which is safer under RegexOptions.Multiline.
  • ParseVersionOutput override correctly pulls the edition version out of noisy -v output (with a WARNING: banner and plugin table) via a targeted regex, falling back to the base implementation if it doesn't match — no regression for other output shapes.
  • HasOptions gaining a Description: check is a reasonable heuristic given ShouldSkipCommand in CliScraperBase treats !HasOptions && !HasOperandTokens as "drop this command" — this is what restores the description-only/flag-only commands (add, auth, undo, etc.) that were previously being silently dropped. Confirmed this mirrors intent from the PR description rather than accidentally suppressing genuine option-less detection elsewhere (it's only read in one place).
  • The bulk of the IFlyway.Generated.cs/Flyway.Generated.cs/FlywayExtensions.Generated.cs diff (default NotSupportedException interface bodies, "for compatibility" doc wording) is just the generator's existing standard template being applied on a fresh full regeneration — I compared against IDocker.Generated.cs on main and it already uses the same pattern, so this isn't new/introduced by this PR, just Flyway catching up.
  • New tests (FlywayVersion_ExtractsEditionVersionFromNoisy Output, LegacyConfigurationOnlyHelp_PreservesCommandAndOptions, RewordedSectionHeading_TerminatesCommandTable, PinnedFlywayHelp_SatisfiesCommandCoveragePolicy) directly exercise the new branches and pin the full 19-command real-world help output, including the existing FlywaySentinels_RejectTheKnownPartialCommandSurface test (unmodified) which still correctly asserts the guard fails on a partial 12-command surface. Good regression coverage for exactly the bug being fixed.

Minor / non-blocking observations

  • HasOptions is being used as a dual-purpose "should this command be retained" signal rather than a literal "does it have flags" check (per its shared name/doc in CliScraperBase). Not a bug — it's consistent with how the base class already uses it — but a short comment on the Flyway override explaining why Description: counts (i.e. "every real Flyway command help output has this heading, so its presence distinguishes a genuine leaf command from an empty/error response") would help the next person who touches this scraper avoid re-litigating the heuristic.
  • No architectural concerns — the change stays within the existing scraper/base-class extension points (ExtractSubcommands, ParseVersionOutput, HasOptions) rather than introducing new abstractions, which fits the surrounding pattern used by the other ~20 CLI scrapers in this tool.

No blocking issues found. Nice, targeted fix with solid test coverage for the exact regression it addresses.

@thomhurst
thomhurst force-pushed the issue-3993-command-coverage branch from 937e844 to f52788a Compare August 23, 2026 20:53
@thomhurst

Copy link
Copy Markdown
Owner Author

Rebased onto current main (13fd094). Validation: Flyway command-coverage tests 5/5; ModularPipelines.Flyway.slnx Release build succeeded with 0 warnings/errors.

@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown

Code Review

Re-checked this PR at the current head (f52788a9). Per thomhurst's note, this update is a rebase onto current main (13fd094b55) — the diff content is byte-for-byte the same three commits already covered in the prior review rounds on this thread (restore Flyway commandsretain legacy option helpstop at command table boundary), just replayed on a newer base with new SHAs.

Status of prior findings — both still hold:

  • Legacy Configuration/-url= detectionHasOptions (FlywayCliScraper.cs:274) retains the loose Configuration-section/-url= checks alongside Description: and base.HasOptions, backed by LegacyConfigurationOnlyHelp_PreservesCommandAndOptions.
  • Command-table boundary fragilityExtractSubcommands now stops by table shape (first non-blank, non-command line after a command has been found) instead of an enumerated list of heading strings, backed by RewordedSectionHeading_TerminatesCommandTable. This closes the drift-fragility gap raised earlier without depending on Flyway's exact heading wording.

Confirmed the file set (14 files) and diff hunks in tools/.../FlywayCliScraper.cs, the options generator tests, and the regenerated Flyway package files match what was already reviewed and approved — no new logic, no new surface area, nothing to re-flag.

No new issues found. This is a well-tested, well-scoped fix and looks good to merge.

Parse preview annotations and description-only help so pinned Flyway coverage discovers all 19 commands.
@thomhurst
thomhurst force-pushed the issue-3993-command-coverage branch from f52788a to e2306e8 Compare August 23, 2026 21:59
@thomhurst

Copy link
Copy Markdown
Owner Author

Rebased onto current main after #4023/#3985 and force-pushed with an exact lease. Validation: FlywayCommandCoverageTests passed 5/5; Flyway Release solution build succeeded with 0 warnings/errors; diff checks clean. Verified remote head: e2306e8.

@thomhurst
thomhurst merged commit 1a4a47b into main Aug 24, 2026
16 checks passed
@thomhurst
thomhurst deleted the issue-3993-command-coverage branch August 24, 2026 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant