Skip to content

Test outgoing HTTP requests and IssueCommandBase::initialize() - #387

Open
mglaman wants to merge 4 commits into
mainfrom
382-http-client-and-initialize-tests
Open

mglaman wants to merge 4 commits into
mainfrom
382-http-client-and-initialize-tests

Conversation

@mglaman

@mglaman mglaman commented Sep 20, 2026

Copy link
Copy Markdown
Owner

Closes #382

What changed

Both HTTP clients accept an optional HandlerStack, so tests can wrap a MockHandler with Guzzle's history middleware and assert on the requests the client actually sends. IssueCommandBase::initRepo() throws a RuntimeException instead of calling exit(1), and checks the git rev-parse result before opening the repository.

Why

#381 changed the GitLab auth header and nothing in the suite noticed. The existing Drupal.org client test swapped in a bare Guzzle client, so it could only test response parsing. #385 fixed a crash in initialize() that the one placeholder test could not have caught.

While making the no-repository path testable, it turned out the old catch never fired: realpath('') returns the current directory, so a failed git rev-parse opened a non-repository and failed later with an unrelated GitException. The exit code seen by users is still 1.

New coverage

GitLab client (tests/src/GitLab/ClientTest.php)

  • Authorization: Bearer <token> from DRUPALORG_GITLAB_TOKEN, and no auth header when the variable is unset and glab is off PATH
  • User-Agent and Accept
  • project/drupal encoded as project%2Fdrupal for getProject(), getIssue(), and postIssueNote()
  • getIssueNotes() requests page 2 only after a full page of 100
  • getIssuesByIid() maps a 404 and malformed JSON to null per iid
  • JSON body of a slash command note
  • 503 followed by 200 succeeds through the retry middleware

Drupal.org client (tests/src/ClientTest.php)

  • Default headers, and Cache-Control / Pragma only with --no-cache
  • Cookie persisted across two requests
  • Request URLs for nodes and project lookups
  • 503 followed by 200 succeeds through the retry middleware

IssueCommandBase (tests/src/Command/Issue/IssueCommandBaseTest.php)

  • Bare NID, project#nid, and work item URL all open the repository
  • requiresRepository = false never touches git
  • NID read from a 3617735-fix_js branch
  • "Argument nid not provided" on main, "No repository found" outside a repository

Testing

Each new header and initialize test was checked by reintroducing the original regression (PRIVATE-TOKEN header, and the early return from #385) and confirming the suite fails.

vendor/bin/phpunit        OK (262 tests, 814 assertions)
vendor/bin/phpcs src      clean
vendor/bin/phpstan analyse src   No errors

The 503 retry tests send Retry-After: 0, so the retry middleware does not sleep.

🤖 Generated with Claude Code

mglaman and others added 4 commits September 20, 2026 10:30
Both HTTP clients accept an optional HandlerStack. When one is passed
it still gets the retry middleware, the default headers, and the
base_uri, so a test can build the stack around a MockHandler plus
Guzzle's history middleware and inspect what was actually sent.

The new GitLab client test covers the gap #381 exposed: the Bearer
header, the User-Agent and Accept headers, the unauthenticated case,
project path encoding, note pagination, the concurrent
getIssuesByIid() fan-out, the JSON body of a slash command note, and a
503 recovered by the retry middleware.

Refs #382

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Replace the anonymous subclass that swapped in a bare Guzzle client
with the HandlerStack the constructor now accepts. The client under
test keeps its headers, base_uri, cookie jar, and retry middleware, so
the test can cover the default headers, the --no-cache headers, cookie
persistence across requests, the request URLs, and a 503 recovered by
the retry middleware.

Refs #382

Co-Authored-By: Claude Fable 5.1 <[email protected]>
initRepo() called exit(1) from inside the command, which no test can
observe. It also never reached that branch: when git rev-parse fails
its output is empty, and realpath('') is the current directory, so
GitRepository opened a non-repository without complaint and the
failure surfaced later as an unrelated GitException.

Check the git process result and throw a RuntimeException. The console
application renders the message and exits 1, so the user-facing exit
code is unchanged.

Refs #382

Co-Authored-By: Claude Fable 5.1 <[email protected]>
…sitory

Each test runs a throwaway command inside a fresh temporary directory,
with or without a git repository in it, and asserts on the nid, the
work item reference, and whether the repository was opened.

This covers the regression fixed in #385, where a project#nid or work
item URL argument returned before initRepo() ran, along with reading
the nid from the branch name and both failure messages.

Refs #382

Co-Authored-By: Claude Fable 5.1 <[email protected]>
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.

Test outgoing HTTP requests with Guzzle MockHandler and history middleware

1 participant