[QA] Add Kubernetes Connections report; split mesheryctl results dirs - #80
Conversation
Add a dedicated 'Kubernetes Connections' Allure report as a filtered view in allurerc.mjs: an @allurereport/plugin-awesome block whose filter selects results labelled epic="Kubernetes Connections" (falling back to componentUnderTest matching Kubernetes for results tagged before the epic convention), grouped by client (UI vs CLI) then suite/subSuite. It aggregates connection tests from both the Meshery (UI) and mesheryctl (CLI) pools without removing them from those reports. Fix the mesheryctl-results clobber (qa side): the BATS e2e and Go unit feeders both synced into mesheryctl-results/ via results-sync, which wipes its target, so whichever committed last erased the other. Add dedicated mesheryctl-bats-results- sync and mesheryctl-unit-results-sync targets writing to distinct dirs, merged at report-build; keep the legacy mesheryctl-results-sync as a back-compat alias so the change is order-independent with the meshery-side workflow change. Update the Pages workflow paths trigger and the README (report list + results-dir table + sparse-clone examples) for the new dirs. Signed-off-by: marblom007 <158522975+marblom007@users.noreply.github.com>
|
Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, you can find updates in the #github-notifications channel in the community Slack. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a Kubernetes Connections-focused Allure report view and prevents mesheryctl result sets from clobbering each other by splitting CLI results into dedicated directories.
Changes:
- Added a new “Kubernetes Connections” report in Allure config, filtered by
epicwith acomponentUnderTestfallback and grouped byclient. - Split
mesheryctlresults sync into separate BATS and unit-test targets, merging both duringreport-buildwhile keeping the old sync target as a deprecated alias. - Updated Pages workflow triggers and README documentation to reflect the new report and result directories.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| allurerc.mjs | Adds the Kubernetes Connections report and supporting label filter/grouping helpers |
| README.md | Documents published reports and new/split results directories |
| Makefile | Introduces split mesheryctl sync targets and merges new dirs at report build time |
| .github/workflows/publish-allure-report.yml | Triggers Pages publish when new results directories change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| singleFile: false, | ||
| reportLanguage: "en", | ||
| open: false, | ||
| logo: "https://raw.githubusercontent.com/meshery-extensions/qa/refs/heads/master/.github/assets/images/meshery/icon-only/meshery-light-icon.svg", |
There was a problem hiding this comment.
Leaving as-is here: every existing plugin in allurerc.mjs (meshery, mesheryctl, kanvas, layer5Cloud) uses this same refs/heads/master logo URL, so pinning only the new plugin to a commit SHA would diverge from the established convention. Converting all logos to pinned SHAs/tags is a worthwhile but separate, repo-wide change.
There was a problem hiding this comment.
Leaving as-is intentionally: every plugin in allurerc.mjs uses this same refs/heads/master logo URL, so pinning only the new plugin to a SHA would diverge from the file's convention. Converting all logos to pinned refs is a worthwhile separate, repo-wide change.
| // componentUnderTest values that denote Kubernetes connection behavior. Used as | ||
| // a fallback selector when a result predates the epic label. | ||
| const CONNECTION_COMPONENTS = /kubernetes/i; |
There was a problem hiding this comment.
Renamed CONNECTION_COMPONENTS to CONNECTION_COMPONENT_RE so the RegExp type is obvious at the use site. No behavior change.
| mesheryctl-bats-results-sync: | ||
| @echo "Syncing mesheryctl BATS e2e Test Results..." | ||
| $(call results-sync,MESHERYCTL_BATS_RESULTS_PATH,mesheryctl-bats-results) | ||
|
|
||
| ## Sync mesheryctl Go unit Test Results | ||
| mesheryctl-unit-results-sync: | ||
| @echo "Syncing mesheryctl Go unit Test Results..." | ||
| $(call results-sync,MESHERYCTL_UNIT_RESULTS_PATH,mesheryctl-unit-results) |
There was a problem hiding this comment.
Fixed - results-sync (and results-sync-path) now rm -rf the destination only after the source var is confirmed set and present, so a misconfigured or empty path var can no longer wipe committed results before the guard runs. This closes the data-loss window for every sync target, not just the new ones.
There was a problem hiding this comment.
This is addressed by the macro fix in the same commit: rm -rf/mkdir now sit inside the [ -n ... ] && [ -d ... ] guard, so an unset or missing path var no longer wipes the destination - it logs and leaves the committed results intact. That is the safe-skip behavior already used by the other named sync targets, so I kept it rather than adding a separate exit-1 per target. project-results-sync keeps its own exit-1 because it is the generic entrypoint.
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe PR separates Mesheryctl BATS and Go unit-test results, preserves results when sources are invalid, updates Allure publishing triggers and documentation, and adds a filtered Kubernetes Connections report grouped by client and suite. ChangesAllure publishing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ResultDirectories
participant AllureConfig
participant ConnectionsReport
ResultDirectories->>AllureConfig: Provide split BATS and unit-test results
AllureConfig->>ConnectionsReport: Filter Kubernetes Connections results
ConnectionsReport->>ConnectionsReport: Group results by client, suite, and sub-suite
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@allurerc.mjs`:
- Around line 26-33: Update isConnectionBehavior so the componentUnderTest
fallback is evaluated only when no epic label exists; results with a nonmatching
epic must be rejected. Add coverage for matching epic labels, legacy results
without an epic, and results with a different epic value.
In `@Makefile`:
- Around line 107-108: Update the report-build commands around the mesheryctl
result copies so the legacy mesheryctl-results artifacts are removed or isolated
from allure-results inputs; only the current split outputs from
mesheryctl-bats-results and mesheryctl-unit-results should be included,
preventing stale tracked results from being merged.
In `@README.md`:
- Around line 93-101: Update the Kubernetes Connections documentation in the
report table and the explanatory paragraph to include the legacy selection
fallback: older results without the epic label are included when
componentUnderTest matches Kubernetes. Keep the existing epic-based criterion
and tagged-test behavior unchanged.
🪄 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 Plus
Run ID: 0739f9d7-2c2e-44ff-9145-7cf0717d2cda
📒 Files selected for processing (4)
.github/workflows/publish-allure-report.ymlMakefileREADME.mdallurerc.mjs
… drop stale legacy dir - allurerc.mjs: the Connections filter's componentUnderTest fallback now applies ONLY when a result has no epic label at all. Previously a result with a different epic (e.g. epic="Other") plus componentUnderTest=Kubernetes was wrongly pulled into the report. Verified: matching epic, legacy no-epic fallback, and different-epic-excluded cases all behave correctly. - Makefile results-sync/results-sync-path: only rm -rf the destination after the source var is confirmed set and present. Wiping first meant a misconfigured or empty source var destroyed the committed destination results before the guard. - Makefile report-build: stop copying the frozen legacy mesheryctl-results/ dir; merging it with the split dirs would mix stale pre-split results with current ones. The split dirs repopulate on the next CI run of each feeder. - README: document the no-epic componentUnderTest fallback (and that a different epic is not pulled in by the component fallback). Signed-off-by: marblom007 <158522975+marblom007@users.noreply.github.com>
|
@coderabbitai review |
|
@copilot review |
✅ Action performedReview finished.
|
Review of the two commits (8f66ff6 and e7f3e1a): allurerc.mjs
Makefile
publish-allure-report.yml
README.md
No logic errors, security issues, or data-loss risks found. CI checks (DCO, triage) are passing. LGTM. |
There was a problem hiding this comment.
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 `@Makefile`:
- Around line 22-46: Update both results-sync and results-sync-path to copy the
validated source into a temporary sibling directory first, creating the parent
as needed, and only remove and replace the destination after the copy completes
successfully. Preserve the existing source validation and
destination-left-intact behavior when validation or copying fails, and clean up
temporary data on failure.
🪄 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 Plus
Run ID: 3baa8dd4-6242-4d67-bc06-112cb206e693
📒 Files selected for processing (3)
MakefileREADME.mdallurerc.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- allurerc.mjs
The constant is a RegExp; the plural name read like a collection. Rename it so the type is obvious at the use site. No behavior change. Signed-off-by: marblom007 <158522975+marblom007@users.noreply.github.com>
|
@coderabbitai review |
|
@copilot review |
✅ Action performedReview finished.
|
Review of all three commits (8f66ff6, e7f3e1a, 9d7e41): allurerc.mjs
Makefile
publish-allure-report.yml
README.md
CI: DCO and triage checks passing. No issues found. LGTM. |
…cess Previously the destination was rm -rf'd before cp completed, so a mid-copy I/O error, a disappearing source, or an empty glob could delete valid committed results and leave the destination empty. Copy the validated source into a temporary sibling dir first (cp -a src/. so it also succeeds on an empty source without glob expansion), then remove and swap in the destination only after the copy succeeds; on copy failure the destination is left intact and the target fails loudly. Applied to both results-sync and results-sync-path. Signed-off-by: marblom007 <158522975+marblom007@users.noreply.github.com>
|
@coderabbitai review |
|
@copilot review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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 `@Makefile`:
- Line 35: Quote the destination argument in both file-operation commands within
the Makefile sync rule: update the rm and mv uses of $(2) so paths containing
whitespace remain a single path, including the corresponding occurrence at the
other reported location.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
project-results-sync feeds $(PROJECT)-results into $(2); an unquoted rm -rf/mv would split a destination containing whitespace into multiple paths. Quote both uses in results-sync and results-sync-path. Verified with a PROJECT containing a space: a single correctly-named dir is created, no stray paths. Signed-off-by: marblom007 <158522975+marblom007@users.noreply.github.com>
|
@coderabbitai review |
|
@copilot review |
✅ Action performedReview finished.
|
Review of all four commits (8f66ff6, e7f3e1a, 9d7e41, db7d81c): allurerc.mjs — no changes since last review; still correct. Makefile (db7d81c — new since last review)
publish-allure-report.yml / README.md — unchanged; still consistent. CI: DCO and triage checks passing. No logic errors or safety issues found. LGTM. |
Review of all five commits (8f66ff6, e7f3e1a, 9d7e41, db7d81c, 471e287): allurerc.mjs — unchanged; still correct. Makefile (471e287 — new since last review)
publish-allure-report.yml / README.md — unchanged; still consistent. CI: DCO and triage checks passing. No issues found. LGTM. |
…ontract Point the two mesheryctl Allure feeders at dedicated qa result dirs so they stop clobbering each other. The BATS e2e workflow and the Go unit workflow both synced into qa's mesheryctl-results/ via results-sync, which wipes its target, so whichever committed last erased the other - and CLI Kubernetes Connection results (BATS) could be wiped by a later unit sync. - mesheryctl-e2e.yaml: make mesheryctl-bats-results-sync (MESHERYCTL_BATS_RESULTS_PATH) - go-testing-ci.yml: make mesheryctl-unit-results-sync (MESHERYCTL_UNIT_RESULTS_PATH) The matching qa targets/dirs (merged at report-build) are added in meshery/qa#80, which should merge first; qa keeps the legacy mesheryctl-results-sync as a back-compat alias so neither side hard-breaks on merge order. Document the QA dashboard (qa.meshery.io) and the shared Kubernetes Connections Allure label contract - epic, componentUnderTest, testId (TC-<n>), client - in the build-and-release contributing docs, so the UI and CLI test lanes tag connection tests consistently and the dedicated Connections report can filter them. Signed-off-by: marblom007 <158522975+marblom007@users.noreply.github.com>
What & why
Adds a dedicated Kubernetes Connections report to the QA dashboard and fixes a
results-clobber bug in the
mesheryctlpipeline. None of the existing reports focus onKubernetes Connections; this gives connection behavior its own cross-client lens.
Changes
1.
allurerc.mjs- new "Kubernetes Connections" report (filtered view)A new
@allurereport/plugin-awesomeblock whosefilterselects results labelledepic == "Kubernetes Connections", falling back tocomponentUnderTestmatchingKubernetes for any result tagged before the epic convention. Grouped by
client(UI vs CLI), then
suite/subSuite.epic, notproject, so it aggregates connection tests from boththe Meshery (UI,
project=Meshery) and Mesheryctl (CLI,project=mesheryctl) resultpools - and connection tests still appear in the Meshery / Mesheryctl reports. The
Connections report is an additional view, not a relocation.
groupBy: ["client","suite","subSuite"]is safe on the customclientlabel:preciseTreeLabels(plugin-api) keeps only label names present on at least one resultand drops absent ones, so results missing
clientfall back to suite grouping.Connection tests are tagged at their source (UI Playwright specs + CLI converters, in the
companion
meshery/mesheryPR) withepic="Kubernetes Connections",componentUnderTest,testId(TC-<n>), andclient(UI|CLI), sourced from the Meshery Test Plan.2. Fix the
mesheryctl-results/clobber (qa side)The BATS e2e feeder (
mesheryctl-e2e.yaml) and the Go unit feeder (go-testing-ci.yml)both synced into
mesheryctl-results/viaresults-sync, whichrm -rfs its target - sowhichever workflow committed last wiped the other's results. The Mesheryctl report
therefore never showed BATS + unit together, and CLI connection results (BATS) could be
erased by a later unit sync.
mesheryctl-bats-results-sync->mesheryctl-bats-results/andmesheryctl-unit-results-sync->mesheryctl-unit-results/targets; both are merged atreport-build.mesheryctl-results-syncas a deprecated back-compat alias so this change isorder-independent with the meshery-side workflow change (neither PR breaks
masterif itmerges first).
companion
meshery/mesheryPR.3.
publish-allure-report.yml+README.mdpathstrigger now includes the two new results dirs.and sparse-clone exclusion examples updated for the new dirs.
Verification
Ran
make report-buildlocally against synthetic results (allure 3 vianpm ci):connections.spec.ts) under aUIgroup and a CLI test (
007-connection) under aCLIgroup; the non-connection test wasexcluded. Confirms the filter +
clientgrouping.still present in its project report.
allurerc.mjsloads cleanly; filter returns true forepic, true forcomponentUnderTestfallback, false for a non-connection result.
Notes / follow-ups
categories.json(connection failure triage) was intentionally not added: Allure 3applies categories report-wide (
context.categories), so connection-specific categorieswould land on every report. Worth a separate, deliberate change owned across report owners.
allurerc.mjsmerges.mesheryctl-results/directory of already-committed data is left in place(still merged at
report-build); it can be pruned in a follow-up once both new dirs arepopulated by CI.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation