Skip to content

docs: add Run 2 section to GA test report #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shreemaan-abhishek wants to merge 1 commit into
base: master
Choose a base branch
from
+50 −0
Open

docs: add Run 2 section to GA test report #38

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions docs/ga-test-report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,53 @@ A declarative file with a top-level `service_templates:` section validated as "C
## Follow-ups

- **Task #2** (remove the `plugin-config` standalone command) is still outstanding. Once done, Phase C also expects the declarative `plugin_configs` top-level section to be rejected — the `service_templates` rejection added here is the template for that change.

---

# Run 2 (post-#31, post-#34)
Comment on lines 87 to +93

## Environment

| Item | Value |
|---|---|
| Date | 2026-05-23 |
| API7 EE version | **3.9.12** (`/api/version` → `v3.9.12`) |
| `a7 version` | `ac31b9d` (master after PR #34 merged) |
| Admin URL | `https://localhost:7443` |
| Gateway group | `default` |
| Deviations | Data-plane traffic tests not run (out of scope per plan). |

## Summary

Re-run after the Run 1 fixes (PR #31) and the `plugin-config` removal (PR #34) merged to master. **All 6 targeted regression checks hold.** Each of the 13 core resources round-trips. **4 new findings surfaced (3 bugs + 1 cosmetic)** — none block GA, but the 3 bugs each warrant a sub-issue and a test-before-fix.

## Regression checks (all pass)

| Check | Source | Result |
|---|---|---|
| `ssl update` without `--cert/--key` → clean error | PR #31 BUG-1 | ✅ |
| `plugin-metadata get -o yaml` → YAML map | PR #31 BUG-2 | ✅ |
| `plugin get -o yaml` → YAML (not JSON) | PR #31 BUG-3 | ✅ |
| `stream-route create` without `--name` → error; `-f -o yaml` → YAML map | PR #31 BUG-4 + review fixup | ✅ |
| `config validate` rejects all 4 unsupported sections (incl. empty `[]`) | PR #31 BUG-5 + PR #34 | ✅ |
| `a7 plugin-config` / `upstream` / `consumer-group` / `service-template` → `unknown command` | PR #34 | ✅ |

## New findings

| # | Severity | Resource | Finding | Disposition |
|---|---|---|---|---|
| R2-1 | 🟡 Bug | route | README documents `a7 route update <id> --desc "..."` but neither `route create` nor `route update` exposes a `--desc` flag. Description is only settable through `-f`. | Sub-issue + E2E |
| R2-2 | 🟡 UX | route | `a7 route list -g default` errors with `--service-id is required by API7 EE`. The e2e helper iterates services to aggregate routes; the CLI doesn't. | Sub-issue (post-GA candidate) |
| R2-3 | 🟡 Bug | credential | `a7 credential create smoke-cred-X --consumer Y ...` returned a server-assigned UUID, ignoring the positional id. (Run 1 noted this as "intended" via `TestCredential_CreateWithPositionalID` — needs re-confirmation; if intended, drop the misleading `[id]` from the help.) | Sub-issue + decision |
| R2-4 | 🟡 Bug | global-rule | `a7 global-rule create --id X --plugins-json '{"cors":...}'` ignores `--id`; resource is created at `id=cors`. Run 1 noted "value is ignored by EE" as a minor; treat as a real bug: the CLI shouldn't accept a flag it will silently override. | Sub-issue + E2E |
| R2-5 | 🟢 Cosmetic | stream-route | `-o yaml` renders `created_at: 1.779521636e+09` in scientific notation. Should be plain integer. | Note only |
Comment on lines +106 to +129
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Inconsistency between summary count and findings table.

Line 108 states "4 new findings surfaced (3 bugs + 1 cosmetic)" but the New findings table (lines 121-129) contains 5 entries:

  • R2-1, R2-3, R2-4: 3 bugs ✓
  • R2-5: 1 cosmetic ✓
  • R2-2: 1 UX issue (not accounted for in the summary)

Additionally, the PR description lists four findings (#35, #36, #37, + cosmetic) which correspond to R2-1, R2-3, R2-4, and R2-5, but does not mention R2-2 (route list requiring service-id).

Please either:

  1. Update the summary to "5 new findings surfaced (3 bugs + 1 UX + 1 cosmetic)" and add R2-2 to the PR description, or
  2. Remove R2-2 from the table if it's not considered a new finding for this run, or
  3. Clarify why R2-2 is excluded from the count (e.g., if "post-GA candidate" disposition means it's tracked separately).
🤖 Prompt for 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.

In `@docs/ga-test-report.md` around lines 106 - 129, The summary count is
inconsistent with the New findings table: the summary sentence ("4 new findings
surfaced (3 bugs + 1 cosmetic)") must be reconciled with the table entries
(R2-1..R2-5) — either update the summary to "5 new findings surfaced (3 bugs + 1
UX + 1 cosmetic)" and add R2-2 to the PR description (alongside the existing
references to R2-1/R2-3/R2-4/R2-5), or remove R2-2 from the table, or add a
clarifying note why R2-2 is excluded (e.g., "tracked separately / post-GA
candidate"); locate the summary sentence and the New findings rows (R2-2) in the
doc and make the corresponding change so the numeric totals and PR description
(`#35/`#36/#37 mapping) are consistent.


## Exit criteria (Run 2)

| Criterion | Status |
|---|---|
| API7 EE version pinned to 3.9.12 in writing | ✅ |
| All Run 1 fixes hold against current master | ✅ 6/6 regression checks pass |
| Phase C unsupported commands gone (including `plugin-config`) | ✅ all 4 return `unknown command` |
| Phase D `config dump/validate` work on a clean instance | ✅ (no Run 2 dirty-state diff test, but Run 1 covered diff/sync convergence) |
| Each new bug gets a tracked sub-issue with test-before-fix protocol | ⏳ pending sub-issue creation
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix table formatting: missing trailing pipe.

Line 139 is missing the trailing | character to properly close the markdown table row.

📝 Proposed fix 
-| Each new bug gets a tracked sub-issue with test-before-fix protocol | ⏳ pending sub-issue creation
+| Each new bug gets a tracked sub-issue with test-before-fix protocol | ⏳ pending sub-issue creation |

As per coding guidelines, the static analysis tool flagged this as MD055 table-pipe-style violation.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| Each new bug gets a tracked sub-issue with test-before-fix protocol | ⏳ pending sub-issue creation
| Each new bug gets a tracked sub-issue with test-before-fix protocol | ⏳ pending sub-issue creation |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 139-139: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe

(MD055, table-pipe-style)

🤖 Prompt for 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.

In `@docs/ga-test-report.md` at line 139, The markdown table row missing a closing
pipe should be fixed by appending a trailing '|' to the row string "Each new bug
gets a tracked sub-issue with test-before-fix protocol | ⏳ pending sub-issue
creation" so the row becomes properly terminated with a final pipe character to
satisfy the MD055 table-pipe-style rule.

Loading