Skip to content

fix(idris-dependents): repair the test crate (red on main since #628) - #649

Merged
hyperpolymath merged 2 commits into
mainfrom
claude/vigilant-turing-94eajv-idris-dependents-mock
Nov 5, 2026
Merged

hyperpolymath merged 2 commits into
mainfrom
claude/vigilant-turing-94eajv-idris-dependents-mock

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

Summary

build · test · clippy (idris-dependents) has failed on main since #628, and on every PR since (for example #645).

  1. Compile error. ready_mock() in tests/scan_tests.rs predates the responses field on Mock, so the test crate failed with E0063 before any test ran. The field is now initialised empty, as the file's other two Mock literals already do.
  2. Two tests the compile error had hidden. In tests/scan_edge_tests.rs, both tests encoded behaviour from before raw files returning HTTP 404/410/451 became skipped URLs. That contract is specified in the README, in the get_file doc comment, and in scan_tests::unavailable_files_are_logged_reported_and_skipped_in_both_raw_fetch_paths.
    • raw_content_fetches_retry_rate_limits_and_propagate_hard_failures expected a raw 404 to fail the run. Its hard-failure case now uses HTTP 500, which still propagates as HTTP 500 for <url>. The 404 skip stays covered by scan_tests.
    • report_json_keeps_public_field_names_and_confidence_spelling now includes the public skippedUrls field.

Only test code changes; the product behaviour is untouched.

Validation

  • I reproduced E0063 on a clean checkout of main.
  • All 79 idris-dependents tests pass across every test binary.
  • cargo clippy --all-targets -D warnings and cargo fmt --check are clean.
  • The first commit's message says "all tests pass". That was wrong when written: the two edge-test failures had been cut from the output I checked. The second commit fixes them and says so.

Generated by Claude Code

`ready_mock()` in tests/scan_tests.rs predates the `responses` field on
`Mock`, so the test crate failed to compile (E0063) and the
`build · test · clippy (idris-dependents)` job has been red on main
since #628, and on every PR since.

The field is initialised empty, as the other two Mock literals in the
file already do. All idris-dependents tests pass; clippy -D warnings and
fmt are clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpQVLrCbURpoXtrdvy8V3B
…tract

With scan_tests compiling again, two tests in scan_edge_tests.rs fail.
(The previous commit said all tests pass; that was wrong: those two were
cut from the output I checked.) Both encode behaviour from before raw
files returning HTTP 404/410/451 became skipped URLs, which the README,
the `get_file` doc comment and scan_tests'
`unavailable_files_are_logged_reported_and_skipped_in_both_raw_fetch_paths`
all specify:

- raw_content_fetches_retry_rate_limits_and_propagate_hard_failures
  expected a raw 404 to fail the run. A 404 is now skipped, so the hard
  failure case uses HTTP 500, which still propagates. The 404 skip stays
  covered by scan_tests.
- report_json_keeps_public_field_names_and_confidence_spelling now
  includes the public `skippedUrls` field.

All 79 idris-dependents tests across every test binary pass; clippy
-D warnings and fmt are clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpQVLrCbURpoXtrdvy8V3B
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T00:12:59.265222Z 49e4d56 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@sonarqubecloud

sonarqubecloud Bot commented Nov 5, 2026

Copy link
Copy Markdown

Copy link
Copy Markdown
Owner Author

build · test · clippy (idris-dependents) is now green, the check this PR exists to fix. SonarCloud passes (0 new issues), Codex found nothing, and every other job is green.

Three governance checks fail here, and they fail on main and every PR. This PR changes only idris-dependents/tests/:

Squash-merging per the owner's merge-when-green instruction; these three checks are already red on main and documented here.


Generated by Claude Code

@hyperpolymath
hyperpolymath merged commit 8a91dd9 into main Nov 5, 2026
59 of 62 checks passed
@hyperpolymath
hyperpolymath deleted the claude/vigilant-turing-94eajv-idris-dependents-mock branch November 5, 2026 01:20
hyperpolymath pushed a commit that referenced this pull request Nov 5, 2026
#649 landed the same three test-fixture repairs as this branch's first
commit. Conflicts resolved to main's ready_mock and this branch's
scan_edge_tests (a superset: it also asserts the 404 skip and the HTTP
error body), so against main this PR now only adds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J4QvDfb1gn2SD218jMg9jk
hyperpolymath added a commit that referenced this pull request Nov 5, 2026
…644)

> **Update (2026-11-05):** `main`'s red CI was fixed independently by
#649, which merged the same three test repairs as this PR's first
commit. `main` has been merged in (`50a5b5f`, conflicts resolved to
`main`'s `ready_mock` plus this branch's superset `scan_edge_tests`).
Against `main`, this PR now **only adds** the review fixes below and
their tests:
> - Codex/CodeRabbit on #628: unterminated strings, GitHub-as-host,
case-insensitive `.gitmodules` keys, relative submodule URLs, HTTP error
bodies
> - Codex/CodeRabbit on this PR: relative URLs that climb into the host
(git-verified), owner pages with `?`/`#`, authority ending at `?`/`#`
(host spoofing)
>
> 87 tests pass. #642 is now fully superseded by #649.

## Why

Follow-up to #628. That PR merged while CodeRabbit's bot-generated tests
were failing, so **`build · test · clippy (idris-dependents)` has been
red on `main` since 2026-10-27**. This PR turns it green and closes the
review findings that were still open when #628 merged.

## Commit 1: repair the tests that broke `main`

All three are the defects CodeRabbit itself flagged in its last review
of #628:
- `scan_tests::ready_mock` was missing the new `responses` field (E0063,
so the target did not compile). It now takes the remaining fields from
`..Mock::default()`.
- The report JSON contract test lacked `skippedUrls`.
- The "hard failure" test used 404, which #628's own change now skips by
design. The hard failure uses 500, and the 404 skip is asserted
separately: empty rows, with the URL listed in `skipped_urls`.

This commit is green on its own: 79 tests, fmt and clippy, checked in an
isolated worktree.

## Commit 2: review findings from #628

| Finding | Source | Fix |
|---|---|---|
| `brief = "depends = proven` reported `proven` | Codex | An
unterminated string runs to EOF, so its contents are never read as
package syntax |
| `https://example.com/mirrors/github.com/<owner>/x` counted as HIGH |
Codex | GitHub must be the URL's **host**. Handles ports, userinfo,
`git+https` and scp form |
| `URL = …` in `.gitmodules` was ignored | Codex | Keys matched
case-insensitively, as git config does |
| `url = ../../<owner>/repo.git` was ignored | Codex | Relative URLs
resolved against the dependent repository, as git does. Climbing past
the host returns nothing |
| HTTP errors said only `HTTP 401` | CodeRabbit | A bounded body excerpt
(≤300 chars) is appended, e.g. "Bad credentials" |

Each fix has a regression test, and **all 5 fail on the previous
source** (checked by running them against `main`'s `src/`).

## Verification

- `cargo build --locked --all-targets` ✅ · `cargo test --locked`: **84
passed** ✅ · `cargo clippy -D warnings` ✅ · `cargo fmt --check` ✅
- Idris2 oracle, re-run because the lexer changed: `idris2 agrees on all
1003 cases` (Idris2 `0.8.0-d87ae51c3`, 3 fixtures and 1,000 generated
files)

## Not in this PR

- A live scan against GitHub still needs a token with code-search
access. This session's token is repo-scoped.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01J4QvDfb1gn2SD218jMg9jk)_

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Bug Fixes**
* GitHub references are now matched by host and repository owner,
avoiding false matches from unrelated URLs. Owner matching is
case-insensitive, and query strings and fragments are excluded from
repository paths.
* Relative submodule URLs are resolved against their GitHub superproject
when checking references.
  * Unterminated quoted strings are consumed through the end of input.
* Failed content requests now include a trimmed portion of the response
body in error details. HTTP 404 responses are skipped without failing
the scan.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->


---
_Generated by [Claude Code](https://claude.ai/code)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

3 participants