Skip to content

test: adapt PR #1078 markup regression to current reports - #1101

Merged
decyjphr merged 4 commits into
yadhav/fix-recent-issuesfrom
decyjphr-comment-markup-audit
Sep 30, 2026
Merged

decyjphr merged 4 commits into
yadhav/fix-recent-issuesfrom
decyjphr-comment-markup-audit

Conversation

@decyjphr

@decyjphr decyjphr commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Why

#1078 fixes malformed HTML in the former PR-comment diff table. The target already contains equivalent tag fixes (6ca72a7) and the later Markdown report replacement (16b9375), so applying the original production patch would restore obsolete markup. This PR adds the missing regression coverage without changing production behavior.

Directly calling Settings.handleResults does not exercise the running app or its webhook/configuration path. The original phase 28 has therefore been removed from the smoke harness and separated into offline integration coverage and one explicitly opt-in live rendering check.

Approach

  • Keep populated PR-comment/check-summary unit regressions for change/error/mixed/no-op results, warning/info sections, app subjects, escaping, pagination, oversized-section limits, and disabled comments.
  • Add native offline integration tests using installed Octokit with controlled HTTP responses and external network blocked. Cover outgoing report bodies, bounded pagination, comments-disabled mode, and API failures.
  • Add npm run integration:live:comment-markup for real GitHub body_html and completed-check readback. It uses synthetic mixed/no-op result rows, verifies the App and target-org installation before installation authentication, and limits requests to its owned fixture and verified installation.
  • The live command creates its own private repository, branch, PR, comments, and checks, refuses existing fixtures, checks repository identity before cleanup, and requires a 404 after deletion. It does not start safe-settings or run shared smoke setup/teardown.
  • Keep live execution outside test:* and default test discovery. Preserve and extend ownership/failure tests without VM source extraction.

smoke-test.js and README.md match baseline 337604d30c447554b8e21d88d5a4eb4bb219d052 exactly. The README remains focused on end users; this PR adds no integration-testing documentation there. No other smoke phases, production code, dependencies, or lockfile entries were changed.

Source audit

Both source commits were reviewed individually with the PR reviews/comments; neither is an ancestor of baseline 337604d30c447554b8e21d88d5a4eb4bb219d052.

Source Classification Result
19f048a142edd07311ab7d8e3956e82c6dd030e7 Already equivalent Ancestors 6ca72a7 and 16b9375 already fix and then replace the raw table renderer.
78e7182933e89d32db9e07a211458f60f2fa90c0 Adapted Current report-contract tests, offline integration coverage, and optional live rendering check.

No source commits or sibling patches were cherry-picked.

Current verification

Node 22.12.0, after extraction in d01205c (the subsequent 46651d9 changes only README.md):

  • Unit: 972 passed, 12 skipped. Native integration: 99 passed.
  • The complete optional live entrypoint is exercised offline with synthetic App credentials, intercepted HTTP, and fixture HTML; this is not a fresh live GitHub rendering run.
  • Changed test/helper files pass Standard/ESLint. Lockfile lint and diff checks pass.
  • No live organization requests, shared environment changes, or app-server startup during this refactor.

Historical live evidence

Before extraction, exact tree 500af948 passed exclusive smoke phases 0 -> 28 -> 9: 18 assertions, zero failures, with actual posted GitHub HTML/check readback, completed teardown, exit 0, and PONG. The actual historical pre-fix renderer reproduced two malformed row endings; the source fix and current renderer passed the populated row-ending contract.

Independent cleanup confirmed all seven owned resource endpoints returned 404, captured processes were absent, the port was free, and the private environment was restored byte-for-byte with mode 0600. Original settings/files/policy, 12 property definitions, 15 foreign admin refs, 18 repositories, 18 teams, and other-repository refs/open PRs were preserved. 6082e033 records that earlier audit; it is not evidence of a live run of the extracted command.

Scope: Only comment markup has an optional live integration command. Result rows are synthetic; when explicitly run live, Settings reporting, GitHub API calls, and HTML rendering are real. Neither offline fixtures nor HTML readback claim visual scrollbar verification, webhook delivery, plugin sync, unfiltered multi-organization coverage, or combined-branch compatibility.

decyjphr and others added 4 commits September 30, 2026 11:43
Audit baseline: 337604d (current yadhav/fix-recent-issues). Neither source commit is an ancestor; both were reviewed individually with PR reviews/comments and contain no merge commits.

ALREADY-EQUIVALENT: source 19f048a fixes row/table closings. Ancestor 6ca72a7 already corrected both paths; ancestor 16b9375 subsequently replaced raw diff tables with closed Markdown details sections. No production change or cherry-pick is needed.

ADAPT: source 78e7182 adds a table-specific regression. Assert populated current PR/check output, errors, info/warnings, no-op, app subjects, escaping, long-section pagination, truncation limits and comment defaults instead. Historical malformed handler reproduces two bad row endings; the source fix and current renderer pass the same populated regression contract.

Add isolated maintained phase 28 with ownership/failure tests: real Settings posts two reports/checks to an owned repo/PR, real GitHub body_html readback, and owned-repo cleanup. Result fixtures are injected; no plugin/webhook sync or visual-scrollbar claim. Live execution awaits the parent's serialized SECOND-slot grant after PR 1075 cleanup.

Node 22.12.0: focused baseline 141 passed; full unit 967 passed, 12 skipped; current native integration 88 passed. Changed tests pass Standard/ESLint; smoke harness retains exactly its 9 pre-existing lint findings. No sibling patches, production changes, dependency changes, push, PR, or base merge.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Live tested exact clean tree 500af94, based solely on 337604d. No code or test changes during the exclusive second smoke slot; this audit commit preserves that tree.

After independent PR 1075 release and explicit parent grant, Node 22.12.0 ran only smoke phases 0 -> 28 -> 9: 18 assertions passed, zero failures, phase/teardown completion, exit 0, actual Probot PONG. Synthetic mixed change/error and no-op result rows passed through real Settings.handleResults to real owned PR comments/checks. GitHub full-media readback verified populated details/code/list/error/no-op HTML and no malformed/phantom rows. This is not visual scrollbar, webhook delivery, plugin-sync or multi-org fanout coverage.

Seven offline proofs include actual installed Probot run/applyEnv/Server.load/ready/start/ping with synthetic credentials and blocked external network. Live inherited fetch/auth boundary rejected foreign scopes; controlled single-org listing prevented startup fanout. Actual post-dotenv ready verified all 12 execution flags, including comments enabled, CRON/proxy/enterprise/GHE settings blank, and port 31078.

Independent post-run snapshot exactly preserved original settings/config/file SHAs, organization policy, 12 property definitions, 15 foreign admin refs including phase17b/c/d, 18 repositories, 18 teams, 0 custom roles/org rulesets, 1 admin ruleset, and refs/open PRs of 17 other repos. Owned repo 1398416388, branch, PR 1, checks 109973531875/109973563887 and comments 5914880884/5914881903 independently returned HTTP 404. All four captured owned PIDs absent; ping absent; port 31078 bindable. Private .env restored exact original bytes/mode 0600; authorized parent .env unchanged. Exclusive slot explicitly released to parent only after these proofs.

Source classification unchanged: 19f048a ALREADY-EQUIVALENT through 6ca72a7 then renderer replacement 16b9375; 78e7182 ADAPT to test-only 500af94. No source cherry-picks, production edits, base merges, sibling patches, dependency changes, push or PR. Local unit 967 passed/12 skipped; native integration 88 passed; exact lint baseline unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Restore the smoke harness to its original contents. Keep existing report formatting regressions and add native offline integration coverage through installed Octokit for report bodies, pagination, disabled comments and API failures.

Extract phase 28 into an explicit integration:live:comment-markup command outside test:* and default test discovery. It authenticates only the verified test-org installation, uses fixture-scoped requests, creates and removes its own repository/PR/comments/checks, and never starts the app or invokes shared smoke setup/teardown. Preserve ownership and HTML regression tests without VM source extraction; verify repository identity before cleanup and require deletion readback.

Node 22.12.0: 972 unit tests passed, 12 skipped; 99 native integration tests passed, including the complete optional entrypoint with synthetic credentials and mocked GitHub HTML. Changed files pass Standard/ESLint, lockfile lint and diff checks. No live organization requests or server startup during this refactor; prior live evidence applies to the pre-extraction phase, not a fresh execution of the new command.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove this branch's integration-testing documentation additions and restore README.md to the audit baseline. Test code and commands are unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@decyjphr
decyjphr merged commit a3dcf7f into yadhav/fix-recent-issues Sep 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant