Add integration tests for DevPullCommand - #19
Merged
Merged
Conversation
Merged
Co-authored-by: sak0a <24781653+sak0a@users.noreply.github.com>
Co-authored-by: sak0a <24781653+sak0a@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
[WIP] WIP Address feedback on feature/dev pull command PR
Add integration tests for DevPullCommand
Feb 7, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds integration test coverage for the notur:dev:pull Artisan command and introduces a small DI hook so HTTP calls can be mocked during tests.
Changes:
- Added
DevPullCommandTest.phpwith integration tests covering dry-run behavior, commit info display, error handling, and custom repo config. - Updated
DevPullCommandto accept an optionalGuzzleHttp\Clientvia constructor for testability.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/Integration/Console/DevPullCommandTest.php |
New integration tests for notur:dev:pull, including mocked GitHub API responses. |
src/Console/Commands/DevPullCommand.php |
Adds optional constructor injection for Client and uses it when present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+33
to
+37
| protected function setUp(): void | ||
| { | ||
| parent::setUp(); | ||
| $this->loadMigrationsFrom(__DIR__ . '/../../../database/migrations'); | ||
| } |
There was a problem hiding this comment.
These tests invoke notur:dev:pull, but the command exits early unless base_path('vendor/notur/notur') exists and is writable. The test setup doesn’t currently create that directory, so the tests will fail before hitting the mocked HTTP client. Create the directory (and ensure it’s writable) in setUp() and clean it up in tearDown() (or use a temporary base path override if your testbench setup supports it).
Comment on lines
+71
to
+75
| $this->artisan('notur:dev:pull', ['--dry-run' => true]) | ||
| ->expectsOutput('[DRY RUN] Would download and extract commit abc123de to ' . base_path('vendor/notur/notur')) | ||
| ->expectsOutput('[DRY RUN] Would rebuild frontend bridge') | ||
| ->expectsOutput('[DRY RUN] Would copy bridge.js and tailwind.css to public/notur/') | ||
| ->assertExitCode(0); |
There was a problem hiding this comment.
expectsOutput() is order-sensitive in Laravel/Orchestra console testing; this command prints several lines ("Fetching commit info…", commit details, warnings) before the [DRY RUN] … lines. As written, the first expectsOutput('[DRY RUN] …') is likely to fail because it won’t be the first output line. Prefer expectsOutputToContain() for these assertions, or assert the full ordered output including the preceding lines.
Comment on lines
+236
to
+240
| ->expectsOutput(' Branch: develop') | ||
| ->expectsOutput(' Commit: abc123de') | ||
| ->expectsOutput(' Author: Jane Developer') | ||
| ->expectsOutput(' Date: 2024-01-15T10:30:00Z') | ||
| ->expectsOutput(' Message: Add new feature') |
There was a problem hiding this comment.
Similar to the dry-run assertions: the command outputs additional lines before the commit info block (e.g. "Fetching commit info…") and also outputs warnings after it. If expectsOutput() is used here, ensure the expected lines match the actual ordered output, or switch these to expectsOutputToContain() to avoid brittle ordering failures.
Suggested change
| ->expectsOutput(' Branch: develop') | |
| ->expectsOutput(' Commit: abc123de') | |
| ->expectsOutput(' Author: Jane Developer') | |
| ->expectsOutput(' Date: 2024-01-15T10:30:00Z') | |
| ->expectsOutput(' Message: Add new feature') | |
| ->expectsOutputToContain(' Branch: develop') | |
| ->expectsOutputToContain(' Commit: abc123de') | |
| ->expectsOutputToContain(' Author: Jane Developer') | |
| ->expectsOutputToContain(' Date: 2024-01-15T10:30:00Z') | |
| ->expectsOutputToContain(' Message: Add new feature') |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
notur:dev:pullcommand lacked test coverage despite existing integration tests for other Artisan commands.Changes
Added
DevPullCommandTest.phpwith 8 integration tests covering:--dry-runexecution path (with/without--no-rebuild)Modified
DevPullCommandto support dependency injection:Clientconstructor parameter for test mockingImplementation
Tests use Mockery to mock GitHub API responses, avoiding real HTTP calls:
Follows patterns from existing integration tests in
tests/Integration/Console/.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.