-
-
Notifications
You must be signed in to change notification settings - Fork 34
Register schedule commands #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Register schedule commands #218
Conversation
|
@LukeTowers I can't figure out why the “Code Analysis” test fails. I haven't made any changes to the concerned file, and locally the test passes without error. |
|
@bennothommo can you take a look at this? Not sure what is going on here. |
|
@LukeTowers @bennothommo Any news?? |
WalkthroughThis PR registers three new Artisan schedule commands (ScheduleList, ScheduleTest, ScheduleWork) in the service provider and adds comprehensive test coverage for the ScheduleListCommand and ScheduleTestCommand functionality across various scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tests/Scheduling/ScheduleListCommandTest.php`:
- Around line 100-105: In ScheduleListCommandTest::tearDown() add cleanup to
reset Carbon's mocked time by calling Carbon::setTestNow(null) (or
Carbon::setTestNow()) before calling parent::tearDown(), so the frozen time set
in setUp() doesn't leak into other tests; keep the existing
putenv('SHELL_VERBOSITY') call and ensure you import/use Carbon\Carbon or
reference it fully when adding the reset.
- Around line 119-123: PHPMD flags the constructor parameter in class
FooParamJob as unused; add a PHPMD suppression docblock to the constructor to
silence the warning. Locate the FooParamJob class and its __construct($param)
method and add a docblock like /**
`@SuppressWarnings`(PHPMD.UnusedFormalParameter) */ immediately above the
__construct definition so the unused-parameter warning is suppressed for this
test stub.
| protected function tearDown(): void | ||
| { | ||
| putenv('SHELL_VERBOSITY'); | ||
|
|
||
| parent::tearDown(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reset Carbon test time to avoid cross-test leakage.
setUp() freezes time; without clearing it, other tests can inherit the mocked clock.
✅ Add cleanup in tearDown()
protected function tearDown(): void
{
+ Carbon::setTestNow(null);
putenv('SHELL_VERBOSITY');
parent::tearDown();
}🤖 Prompt for AI Agents
In `@tests/Scheduling/ScheduleListCommandTest.php` around lines 100 - 105, In
ScheduleListCommandTest::tearDown() add cleanup to reset Carbon's mocked time by
calling Carbon::setTestNow(null) (or Carbon::setTestNow()) before calling
parent::tearDown(), so the frozen time set in setUp() doesn't leak into other
tests; keep the existing putenv('SHELL_VERBOSITY') call and ensure you
import/use Carbon\Carbon or reference it fully when adding the reset.
| class FooParamJob | ||
| { | ||
| public function __construct($param) | ||
| { | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suppress PHPMD unused-parameter warning for the stub.
PHPMD flags $param as unused; this can fail code analysis if tests are included.
🔧 Suppress the unused-parameter warning
class FooParamJob
{
+ /**
+ * `@SuppressWarnings`(PHPMD.UnusedFormalParameter)
+ */
public function __construct($param)
{
}
}🧰 Tools
🪛 PHPMD (2.15.0)
121-121: Avoid unused parameters such as '$param'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In `@tests/Scheduling/ScheduleListCommandTest.php` around lines 119 - 123, PHPMD
flags the constructor parameter in class FooParamJob as unused; add a PHPMD
suppression docblock to the constructor to silence the warning. Locate the
FooParamJob class and its __construct($param) method and add a docblock like /**
`@SuppressWarnings`(PHPMD.UnusedFormalParameter) */ immediately above the
__construct definition so the unused-parameter warning is suppressed for this
test stub.
|
Thanks a lot @LukeTowers !! |
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.