feat: add assertQueueEqualsFixture to TestingTrait - #279
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2292624c72
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| $properties = (new ReflectionClass($object))->getProperties(); | ||
|
|
||
| foreach ($properties as $property) { | ||
| $value = $property->getValue($object); |
There was a problem hiding this comment.
getObjectAttributes() reads every reflected property with getValue(), but this throws an Error for typed properties that were declared but never initialized (for example, queued jobs with public string $foo; and no constructor assignment). In that case, assertQueueEqualsFixture() fails before assertion output is produced, so queue-state fixtures cannot be used for valid job classes. Check ReflectionProperty::isInitialized($object) (or provide a fallback value) before calling getValue().
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c67464057
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| $actualData = []; | ||
|
|
||
| foreach (Queue::pushedJobs() as $namespace => $jobs) { | ||
| $actualData[$namespace] = Arr::map($jobs, fn ($job) => $this->getObjectAttributes($job['job'])); |
There was a problem hiding this comment.
assertQueueEqualsFixture() unconditionally passes $job['job'] into getObjectAttributes(object $object), but Laravel queue fakes can store pushed jobs as class strings (e.g., Queue::push('SomeJob', $data)) rather than instantiated objects. In that valid input path, this helper throws a TypeError before any fixture comparison runs, so tests that assert queue state for string-based jobs fail hard instead of producing assertion output.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71b15dd3c3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| $result[$property->getName()] = $value; | ||
| } | ||
|
|
||
| return json_decode(json_encode($result), true); |
There was a problem hiding this comment.
getObjectAttributes() converts reflected properties via json_encode/json_decode, but json_encode returns false for valid PHP values like invalid UTF-8 strings or resources inside job properties; in that case this expression returns null, violating the declared array return type and throwing a TypeError before fixture assertion runs. This makes queue fixture assertions crash for jobs carrying non-JSON-safe payloads instead of producing a comparison result.
Useful? React with 👍 / 👎.
|
@neellii, please, fix tests |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 714ac87d5e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| $actualData = []; | ||
|
|
||
| foreach (Queue::pushedJobs() as $namespace => $jobs) { | ||
| $actualData[$namespace] = Arr::map($jobs, fn ($job) => $this->getObjectAttributes($job['job'])); |
There was a problem hiding this comment.
assertQueueEqualsFixture() currently reduces each pushed entry to getObjectAttributes($job['job']), which drops the queue and data fields that QueueFake::push() stores alongside the job. In tests that dispatch the same job class to different queues (e.g. pushOn) or with different payload data, this helper will serialize both entries identically and can pass even when queue routing/payload behavior regresses. Include these fields in the compared fixture structure so queue-state assertions remain accurate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd6d2705b1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| { | ||
| $actualData = []; | ||
|
|
||
| foreach (Queue::pushedJobs() as $namespace => $jobs) { |
There was a problem hiding this comment.
When a test uses Queue::pushRaw(), Laravel's QueueFake stores that payload separately in rawPushes() while pushedJobs() remains unchanged. Because this assertion builds fixtures only from Queue::pushedJobs(), a test can pass against an empty or incomplete queue fixture even though raw payloads were enqueued, so regressions in code paths that produce raw queue jobs are silently missed.
Useful? React with 👍 / 👎.
@neellii @vitgrams we have a separate task for it #285 let's proceed work as part of this task |
4 participants
refs: RonasIT/laravel-media#85 (comment)