Skip to content
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

[5.x] Throw an error when running static:clear when static caching is disabled #11193

Merged
merged 7 commits into from
Dec 3, 2024

Conversation

duncanmcclean
Copy link
Member

Currently, when you run php please static:clear with static caching disabled, it won't do anything but it'll say the static cache has been cleared, which is misleading if you've just disabled static caching, but still have files in public/static since you'll continue to be served the cached pages.

This pull request mirrors the php please static:warm command by showing a warning if you try to run the command while static caching is disabled.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests no longer test anything. The expectations were there intentionally.

They were checking that the command did (or did not) run. It doesn't have to be static:clear, but it should be something.

Maybe the rtfm command would be better than flat:camp. There's no randomness. You could assert on the output.

Asserting the output of please commands gets complicated.

We need to duplicate Laravel's `PendingCommand` class to swap out the kernel, amoungst other things.
@duncanmcclean
Copy link
Member Author

They were checking that the command did (or did not) run. It doesn't have to be static:clear, but it should be something.

I've updated the test to call stache:clear instead, since we can do the same kind of assertion with it.

I investigated asserting the output of the rtfm command, however, it'd mean we would need to copy Laravel's PendingCommand class in order to swap out the Kernel import. I was also running into other issues with ->expectsOutput() that I couldn't figure out.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the exit code to 0 to keep it successful. If someone was running static:clear with caching disabled during a deployment for whatever reason, changing to a failure might cause the deployment to fail.

We can change it to a failure in v6.

@jasonvarga jasonvarga merged commit 9493598 into 5.x Dec 3, 2024
19 checks passed
@jasonvarga jasonvarga deleted the static-clear-warn-when-caching-is-disabled branch December 3, 2024 16:51
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.

2 participants