Skip to content

🐛 Fix flaky hub validation of synthesized discovery rulesets; run unit tests in CI#98

Merged
mguetta1 merged 3 commits into
konveyor:mainfrom
fabianvf:fix-hub-discovery-ruleset-race
Jul 20, 2026
Merged

🐛 Fix flaky hub validation of synthesized discovery rulesets; run unit tests in CI#98
mguetta1 merged 3 commits into
konveyor:mainfrom
fabianvf:fix-hub-discovery-ruleset-race

Conversation

@fabianvf

@fabianvf fabianvf commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the race reported by @mguetta1 where hub-target tests fail with Unexpected ruleset found: technology-usage depending on discovery-task timing (nerd-dinner, locally flaky while nightly stays green).

Root cause: the tackle-hub target synthesizes discovery-rules/technology-usage rulesets from the hub's discovery-task tags (no kantra equivalent, and tag contents are never compared for the hub target). koncur only waits on the analysis task, and empty rulesets are filtered before validation — so if tech-discovery finishes before the tag snapshot the synthesized ruleset survives with tags and trips the shared unexpected-ruleset check; if not, it's filtered and the test passes. Only C# is affected because the kantra-generated expected output for nerd-dinner has no technology-usage ruleset (all Java expected outputs do).

Fix: the unexpected-ruleset check now skips discovery-rules/technology-usage for tackle-hub when the ruleset contains nothing but tags. Rulesets with real content (violations/insights/errors) still fail, and the missing-ruleset direction is unchanged. Historical hub outputs show insight reporting for tagging rules has varied across hub versions, so tolerating the fabricated tag-only ruleset is the only behavior correct under all observed variants (as opposed to dropping the tag collection, which would break Java tests on hubs that don't report tagging insights).

Also: unit tests now run in CI

While verifying, we found no CI job runs go test — the unit tests in pkg/config and pkg/targets have been uncompilable since the CustomRule refactor (#44, ~4 months) without anyone noticing.

  • Updated rule-handling tests to the CustomRule model; rewrote prepareRules/prepareRulesForHub tests to exercise the real methods (including the hub's one-git-repo and no-mixed-rules error paths) instead of simulating removed logic
  • TestNewTackleHubTarget now stubs the login probe with httptest instead of retrying against localhost:8080 for 3 minutes
  • New unit-tests job (go vet ./... + go test ./...) in the PR CI workflow

Test plan

  • go vet ./... clean
  • go test ./... green (pkg/targets down from 180s of network retries to <1s)
  • New TestValidateFiles_HubSynthesizedDiscoveryRulesets covers the exact failure shape: tag-only technology-usage absent from expected passes for tackle-hub, still fails for kantra, and a synthesized-name ruleset with insights still fails
  • Hub nightly on this branch (validator change only relaxes the one fabricated-data check, so Java tests are unaffected by construction)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of discovered rulesets so tag-only synthesized rulesets are accepted for hub-based validation, while real content still triggers validation errors.
    • Made rule URL and path parsing more reliable for both Git-based and local file rules.
  • Tests

    • Expanded coverage for ruleset validation and rule parsing across hub and Kantra scenarios.

fabianvf and others added 2 commits July 8, 2026 11:57
The tackle-hub target synthesizes discovery-rules/technology-usage
rulesets from the hub's discovery-task tags, which have no kantra
equivalent and are never compared for the hub target. Whether those
rulesets survive output filtering depends on whether the discovery
tasks finish before koncur snapshots application tags, so tests whose
kantra-generated expected output lacks a technology-usage ruleset
(e.g. nerd-dinner) fail or pass depending on task timing.

Skip the unexpected-ruleset error for tackle-hub when the ruleset is
one of the synthesized names and contains nothing but tags. Rulesets
carrying real content (violations/insights/errors) still fail, and the
missing-ruleset direction is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Fabian von Feilitzsch <fabian@fabianism.us>
No CI job ran go test, so the unit tests in pkg/config and pkg/targets
have been uncompilable since the CustomRule refactor (konveyor#44) without
anyone noticing.

- Update rule-handling tests for the CustomRule model, and rewrite the
  prepareRules/prepareRulesForHub tests to exercise the real methods
  (including the hub's one-git-repo and no-mixed-rules error paths)
  instead of simulating removed logic
- Stub the hub login probe in TestNewTackleHubTarget with httptest
  instead of hitting localhost:8080 (3 minutes of retries)
- Add a unit-tests job (go vet + go test) to the PR CI workflow

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Fabian von Feilitzsch <fabian@fabianism.us>
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Test suites across pkg/config, pkg/targets, and pkg/validator are refactored to use structured config.CustomRule/GitRepoRule types instead of string-based analysis.Rules. validator.go adds logic to skip synthesized tag-only rulesets for tackle-hub targets. A new CI job runs go vet and go test.

Changes

CustomRule refactor, hub validation, and CI unit tests

Layer / File(s) Summary
GitRepoRule parsing tests
pkg/config/git_url_test.go
Re-scopes ParseGitURLs test table to application-only cases and adds TestGitRepoRule_GetCompents validating GitRepoRule component extraction.
Kantra rule preparation tests
pkg/targets/kantra_test.go
Reworks PrepareRules and GitURLIntegration tests to use config.CustomRule and call prepareRules/GetCompents directly, removing unused fmt import.
TackleHub target and rule prep tests
pkg/targets/tackle_hub_test.go
Uses httptest server with dynamic URL in place of hard-coded localhost, and refactors PrepareRulesForHub/GitURLIntegration tests to use config.CustomRule and inspect taskData.Rules directly.
Synthesized ruleset validation
pkg/validator/validator.go, pkg/validator/validator_test.go
Adds isSynthesizedTagRuleset helper and skips unexpected tag-only discovery-rules/technology-usage rulesets for tackle-hub targets in ValidateFiles, with new coverage test.
CI unit test job
.github/workflows/ci.yaml
Adds a unit-tests job running go vet and go test with Go 1.25.

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

Possibly related PRs

  • konveyor/koncur#25: Refactors tackle_hub_test.go to use structured config.CustomRule/Git components and asserts prepareRulesForHub outputs, targeting the same hub rule/Git preparation behavior.
  • konveyor/koncur#44: Builds on the config.CustomRule/GitRepoRule.GetCompents() refactor and prior validator.go tackle-hub ruleset handling, modifying the same git-rule parsing and validator code paths.

Suggested reviewers: jmle, shawn-hurley

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the two main changes: fixing hub validation for synthesized discovery rulesets and adding unit tests to CI.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@fabianvf

fabianvf commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yaml:
- Around line 10-25: The unit-tests workflow job is using overly broad default
permissions and is persisting the checkout token in git config. Update the
unit-tests job in the ci workflow to add a permissions block limited to
contents: read, and configure the actions/checkout step to disable credential
persistence with persist-credentials set to false. Use the unit-tests job and
the checkout step as the points to locate the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bbd4a684-3879-4e18-942e-8cd67bb80f9e

📥 Commits

Reviewing files that changed from the base of the PR and between ed0c1d3 and fc32f81.

📒 Files selected for processing (6)
  • .github/workflows/ci.yaml
  • pkg/config/git_url_test.go
  • pkg/targets/kantra_test.go
  • pkg/targets/tackle_hub_test.go
  • pkg/validator/validator.go
  • pkg/validator/validator_test.go

Comment thread .github/workflows/ci.yaml
Scope the job token to contents: read and skip credential persistence
on checkout; the job only runs go vet and go test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Fabian von Feilitzsch <fabian@fabianism.us>
@mguetta1
mguetta1 requested review from Copilot and mguetta1 and removed request for Copilot July 20, 2026 09:36
@mguetta1 mguetta1 closed this Jul 20, 2026
@mguetta1 mguetta1 reopened this Jul 20, 2026
@mguetta1 mguetta1 added the cherry-pick/release-0.10 This PR should be cherry-picked to release-0.10 branch label Jul 20, 2026
@mguetta1
mguetta1 enabled auto-merge (squash) July 20, 2026 09:42
@mguetta1
mguetta1 merged commit 70eee77 into konveyor:main Jul 20, 2026
23 checks passed
@konveyor-ci-bot

Copy link
Copy Markdown

PR cherry-picked to branch release-0.10. Backport PR: #102

mguetta1 pushed a commit that referenced this pull request Jul 21, 2026
…t tests in CI (#98) (#102)

* 🐛 Ignore hub-synthesized tag-only discovery rulesets in validation

The tackle-hub target synthesizes discovery-rules/technology-usage
rulesets from the hub's discovery-task tags, which have no kantra
equivalent and are never compared for the hub target. Whether those
rulesets survive output filtering depends on whether the discovery
tasks finish before koncur snapshots application tags, so tests whose
kantra-generated expected output lacks a technology-usage ruleset
(e.g. nerd-dinner) fail or pass depending on task timing.

Skip the unexpected-ruleset error for tackle-hub when the ruleset is
one of the synthesized names and contains nothing but tags. Rulesets
carrying real content (violations/insights/errors) still fail, and the
missing-ruleset direction is unchanged.




* 🌱 Fix stale unit tests and run them in CI

No CI job ran go test, so the unit tests in pkg/config and pkg/targets
have been uncompilable since the CustomRule refactor (#44) without
anyone noticing.

- Update rule-handling tests for the CustomRule model, and rewrite the
  prepareRules/prepareRulesForHub tests to exercise the real methods
  (including the hub's one-git-repo and no-mixed-rules error paths)
  instead of simulating removed logic
- Stub the hub login probe in TestNewTackleHubTarget with httptest
  instead of hitting localhost:8080 (3 minutes of retries)
- Add a unit-tests job (go vet + go test) to the PR CI workflow




* 🌱 Restrict unit-tests CI job permissions

Scope the job token to contents: read and skip credential persistence
on checkout; the job only runs go vet and go test.




---------

Signed-off-by: Fabian von Feilitzsch <fabian@fabianism.us>
Signed-off-by: Cherry Picker <noreply@github.com>
Co-authored-by: Fabian von Feilitzsch <fabian@fabianism.us>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick/release-0.10 This PR should be cherry-picked to release-0.10 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants