Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"approvals": [],
"reopenings": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
id: accept-windows-refused-connection-wording-in-the-isolation-test
state: draft
type: bug_fix
base_commit: c65af7f55d832475c877fb9032bc89da4bb685f3
---

# Accept Windows' refused-connection wording in the isolation test

## Intent

Accept Windows' refused-connection wording in the isolation test

## Affected Canonical Specs

- None

## Acceptance Criteria

- default_temp_env_points_github_at_a_dead_port in tests/isolation.rs passes on Linux, macOS and Windows: it still asserts the default TempEnv GitHub base starts with http://127.0.0.1: and that the spawned templates search fails, and it recognizes the refusal as 'connection refused' (Linux, macOS) or Windows' WSAECONNREFUSED wording ('actively refused' / 'os error 10061'), so a genuine api.github.com failure still does not satisfy it; cargo test --locked is green on ubuntu-latest, macos-latest and windows-latest with both the ureq 3.3.0 lockfile on main and the ureq 3.4.2 lockfile the v1.8.0 release carries

## No-spec Rationale

Only tests/isolation.rs changes, and tests/ is outside source_dirs (src, templates). The behavior under test is unchanged: the default TempEnv GitHub base is loopback and the spawned search fails with a refused connection. The test now also recognizes the Windows wording of that refusal, which ureq 3.4 passes through from the OS. specs/github/testing.md already describes the test in wording-neutral terms.
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
change: accept-windows-refused-connection-wording-in-the-isolation-test
artifact: context
---

# Context

## What led here

The v1.8.0 release PR (#532) went red on exactly one cell: `test (windows-latest)`,
in `tests/isolation.rs::default_temp_env_points_github_at_a_dead_port`:

```text
expected a refused local connection, got: error: searching github for template repos:
github api request failed: io: no connection could be made because the target machine
actively refused it. (os error 10061)
```

The same test is green on Windows on `main`. The difference is the lockfile: the release
bump moves `ureq` from 3.3.0 to 3.4.2. In 3.3.0, when every resolved address refused,
`ureq`'s TCP connector threw the OS error away and returned a synthesized
`io::Error::new(ConnectionRefused, "Connection refused")`, so every platform printed the
same English words. 3.4 keeps the last per-address error instead (`last_err` in
`unversioned/transport/tcp.rs`), so the message is now the OS's own:

| Platform | stderr on ureq 3.4.2 |
|---|---|
| Linux | `io: Connection refused (os error 111)` |
| macOS | `io: Connection refused (os error 61)` (observed locally) |
| Windows | `io: No connection could be made because the target machine actively refused it. (os error 10061)` (from the CI log) |

The test's two alternatives were `connection refused` and `127.0.0.1`. The Windows text
has neither, and `ureq` never puts the address in the message, so the test failed.
Nothing in fledge's behavior changed: the request was refused locally, as intended.

## Decisions

**Widen the wording, keep both halves of the proof.** The test exists to rule out a real
request to api.github.com. It does that in two halves, and both are kept unchanged in
strength: the base handed to the child starts with `http://127.0.0.1:`, and the failure is
a refusal. Only the set of words that count as "a refusal" grows, by the two stable parts
of WSAECONNREFUSED's message: `actively refused` and `os error 10061`. A generic
`contains("failed")` stays excluded, because a genuine api.github.com failure satisfies it.

**Match the message, not the error kind.** The test sees a spawned binary's stderr, not
an `io::Error`, so `ErrorKind::ConnectionRefused` is not observable here. Matching the OS
text is the only option at this boundary.

**Land on `main` first.** `main` is green today only because it still pins ureq 3.3.0.
Any lockfile refresh, the release's included, brings in 3.4 and the failure. Fixing it
on `main` lets the release PR rebase onto a test that is already correct, instead of
carrying a test change inside a release commit.

## Out of scope

- Dropping or skipping the Windows matrix cell. corvid-agent offered it as an
alternative. It would hide the signal instead of fixing a test that is wrong.
- The `127.0.0.1` alternative in the stderr assertion. It predates this change and is
unreachable with ureq, but removing it is a separate tightening.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"schema_version": 1,
"workflow_version": 2,
"workflow_origin_version": 2,
"id": "accept-windows-refused-connection-wording-in-the-isolation-test",
"slug": "accept-windows-refused-connection-wording-in-the-isolation-test",
"title": "Accept Windows' refused-connection wording in the isolation test",
"description": "Accept Windows' refused-connection wording in the isolation test",
"kind": "bug_fix",
"state": "draft",
"base_commit": "c65af7f55d832475c877fb9032bc89da4bb685f3",
"created_at": 1790441579,
"updated_at": 1790441589,
"affected_specs": [],
"affected_paths": [
"tests/isolation.rs"
],
"no_spec_change": true,
"no_spec_change_rationale": "Only tests/isolation.rs changes, and tests/ is outside source_dirs (src, templates). The behavior under test is unchanged: the default TempEnv GitHub base is loopback and the spawned search fails with a refused connection. The test now also recognizes the Windows wording of that refusal, which ureq 3.4 passes through from the OS. specs/github/testing.md already describes the test in wording-neutral terms.",
"acceptance_criteria": [
"default_temp_env_points_github_at_a_dead_port in tests/isolation.rs passes on Linux, macOS and Windows: it still asserts the default TempEnv GitHub base starts with http://127.0.0.1: and that the spawned templates search fails, and it recognizes the refusal as 'connection refused' (Linux, macOS) or Windows' WSAECONNREFUSED wording ('actively refused' / 'os error 10061'), so a genuine api.github.com failure still does not satisfy it; cargo test --locked is green on ubuntu-latest, macos-latest and windows-latest with both the ureq 3.3.0 lockfile on main and the ureq 3.4.2 lockfile the v1.8.0 release carries"
],
"selected_artifacts": [
"context",
"testing",
"tasks"
],
"dependencies": [],
"answers": {
"architecture_risk": "no",
"public_contract": "no"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
change: accept-windows-refused-connection-wording-in-the-isolation-test
artifact: tasks
---

# Tasks

- [x] Read the failing `test (windows-latest)` log on #532 and record the exact stderr
- [x] Confirm `main`'s Windows cell is green, and find why: ureq 3.3.0 on `main` vs 3.4.2
in the release lockfile
- [x] Confirm the cause in ureq's source: 3.3.0 synthesizes "Connection refused", 3.4
returns the OS error
- [x] Accept `actively refused` / `os error 10061` alongside `connection refused`, keeping
the loopback-base assertion and excluding a bare `failed`
- [x] Run the isolation tests on both lockfiles locally (macOS)
- [ ] `test (windows-latest)` green on this PR (ureq 3.3.0)
- [ ] `test (windows-latest)` green on the rebased #532 (ureq 3.4.2)
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
change: accept-windows-refused-connection-wording-in-the-isolation-test
artifact: testing
---

# Testing

## Automated

`tests/isolation.rs::default_temp_env_points_github_at_a_dead_port` is the only test in
scope. It runs in CI's `cargo test --verbose --locked` on `ubuntu-latest`,
`macos-latest` and `windows-latest`.

## What was verified

| Check | Result |
|---|---|
| `fledge run test -- --test isolation` on `main`'s lockfile (ureq 3.3.0), macOS | 4 passed |
| Same, on the v1.8.0 release lockfile (ureq 3.4.2), macOS | 4 passed |
| Spawned binary stderr, ureq 3.3.0, macOS | `io: Connection refused` (synthesized by ureq) |
| Spawned binary stderr, ureq 3.4.2, macOS | `io: Connection refused (os error 61)` (the OS's text) |
| Windows stderr, ureq 3.4.2 (#532 CI log) | contains `actively refused` and `os error 10061`, now accepted |
| `windows-latest` on this PR | runs with `main`'s ureq 3.3.0, must stay green |
| `windows-latest` on the rebased #532 | runs with ureq 3.4.2, the case this change fixes |

## Acceptance signals

- `test (windows-latest)` is green on the rebased release PR, which carries ureq 3.4.2.
- `test (ubuntu-latest)`, `test (macos-latest)` and `test (windows-latest)` stay green on
`main`'s ureq 3.3.0 lockfile.

## Rejection signals

- A run that reached api.github.com and failed there must still fail the test. The
loopback assertion on `env.github_api_base()` is unchanged, and no accepted phrase
(`connection refused`, `actively refused`, `os error 10061`) appears in a GitHub HTTP
error or a DNS failure.
- A generic "the command failed" must not be enough. No new alternative matches
`failed` or `error` alone.
11 changes: 10 additions & 1 deletion tests/isolation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,18 @@ fn default_temp_env_points_github_at_a_dead_port() {
"default GitHub base must be loopback, got: {}",
env.github_api_base()
);
//
// The refusal is worded by the OS, not by ureq: since ureq 3.4 the socket
// error passes through (3.3 replaced it with a bare "Connection refused").
// Linux and macOS say "Connection refused (os error 111/61)"; Windows says
// WSAECONNREFUSED's "No connection could be made because the target
// machine actively refused it. (os error 10061)". Same event, both accepted.
let stderr = String::from_utf8_lossy(&output.stderr).to_lowercase();
let refused = stderr.contains("connection refused")
|| stderr.contains("actively refused")
|| stderr.contains("os error 10061");
assert!(
stderr.contains("connection refused") || stderr.contains("127.0.0.1"),
refused || stderr.contains("127.0.0.1"),
"expected a refused local connection, got: {stderr}"
);
}
Expand Down
Loading